Deduplicate 'already installed' / HEAD message. (#1683)

Improve console output when installing packages by deduplicating 'already installed' message, --head warning, and merging the 'green success' message into the total elapsed time part.

Cherry picked from #1514

Co-authored-by: Robert Schumacher <[email protected]>

Author: BillyONeal

azure-pipelines/end-to-end-tests-dir/build-test-ports.ps1

@@ -64,9 +64,10 @@ The following packages are already installed:
 
 $output = Run-VcpkgAndCaptureOutput @commonArgs --overlay-ports="$PSScriptRoot/../e2e-ports" install vcpkg-internal-e2e-test-port3 --head
 Throw-IfFailed
-if ($output -notmatch 'vcpkg-internal-e2e-test-port3:[^ ]+ is already installed -- not building from HEAD') {
-    throw 'Wrong already installed message for --head'
-}
+Throw-IfNonContains -Expected @"
+The following packages are already installed, but were requested at --head version. Their installed contents will not be changed. To get updated versions, remove these packages first:
+    vcpkg-internal-e2e-test-port3:$Triplet@1.0.0
+"@ -Actual $output
 
 Refresh-TestRoot
 $output = Run-VcpkgAndCaptureOutput @commonArgs --x-builtin-ports-root="$PSScriptRoot/../e2e-ports" install vcpkg-bad-spdx-license

include/vcpkg/base/message-data.inc.h

@@ -130,13 +130,9 @@ DECLARE_MESSAGE(AllFormatArgsUnbalancedBraces,
                 (msg::value),
                 "example of {value} is 'foo bar {'",
                 "unbalanced brace in format string \"{value}\"")
-DECLARE_MESSAGE(AllPackagesAreUpdated, (), "", "All installed packages are up-to-date.")
+DECLARE_MESSAGE(AllPackagesAreUpdated, (), "", "No action taken because all installed packages are up-to-date.")
 DECLARE_MESSAGE(AllShasValid, (), "sha = sha512 of url", "All checked sha's are valid.")
 DECLARE_MESSAGE(AlreadyInstalled, (msg::spec), "", "{spec} is already installed")
-DECLARE_MESSAGE(AlreadyInstalledNotHead,
-                (msg::spec),
-                "'HEAD' means the most recent version of source code",
-                "{spec} is already installed -- not building from HEAD")
 DECLARE_MESSAGE(AManifest, (), "", "a manifest")
 DECLARE_MESSAGE(AMaximumOfOneAssetReadUrlCanBeSpecified, (), "", "a maximum of one asset read url can be specified.")
 DECLARE_MESSAGE(AMaximumOfOneAssetWriteUrlCanBeSpecified, (), "", "a maximum of one asset write url can be specified.")
@@ -1787,6 +1783,11 @@ DECLARE_MESSAGE(InstallCopiedFile,
                 "{path_source} -> {path_destination} done")
 DECLARE_MESSAGE(InstalledBy, (msg::path), "", "Installed by {path}")
 DECLARE_MESSAGE(InstalledPackages, (), "", "The following packages are already installed:")
+DECLARE_MESSAGE(InstalledPackagesHead,
+                (),
+                "",
+                "The following packages are already installed, but were requested at --head version. Their installed "
+                "contents will not be changed. To get updated versions, remove these packages first:")
 DECLARE_MESSAGE(InstalledRequestedPackages, (), "", "All requested packages are currently installed.")
 DECLARE_MESSAGE(InstallFailed, (msg::path, msg::error_msg), "", "failed: {path}: {error_msg}")
 DECLARE_MESSAGE(InstallingFromFilesystemRegistry, (), "", "installing from filesystem registry here")
@@ -2812,6 +2813,10 @@ DECLARE_MESSAGE(ToRemovePackages,
                 "",
                 "To only remove outdated packages, run\n{command_name} remove --outdated")
 DECLARE_MESSAGE(TotalInstallTime, (msg::elapsed), "", "Total install time: {elapsed}")
+DECLARE_MESSAGE(TotalInstallTimeSuccess,
+                (msg::elapsed),
+                "",
+                "All requested installations completed successfully in: {elapsed}")
 DECLARE_MESSAGE(ToUpdatePackages,
                 (msg::command_name),
                 "",

include/vcpkg/commands.install.h

@@ -49,11 +49,11 @@ namespace vcpkg
     {
         std::vector<SpecSummary> results;
         ElapsedTime elapsed;
+        bool failed = false;
 
-        LocalizedString format() const;
+        LocalizedString format_results() const;
         void print_failed() const;
-        std::string xunit_results() const;
-        bool failed() const;
+        void print_complete_message() const;
     };
 
     struct InstallDir

include/vcpkg/dependencies.h

@@ -208,7 +208,10 @@ namespace vcpkg
     struct FormattedPlan
     {
         bool has_removals = false;
-        LocalizedString text;
+        LocalizedString warning_text;
+        LocalizedString normal_text;
+
+        LocalizedString all_text() const;
     };
 
     FormattedPlan format_plan(const ActionPlan& action_plan);

locales/messages.json

@@ -114,13 +114,11 @@
   "_AllFormatArgsRawArgument.comment": "example of {value} is 'foo {} bar'",
   "AllFormatArgsUnbalancedBraces": "unbalanced brace in format string \"{value}\"",
   "_AllFormatArgsUnbalancedBraces.comment": "example of {value} is 'foo bar {'",
-  "AllPackagesAreUpdated": "All installed packages are up-to-date.",
+  "AllPackagesAreUpdated": "No action taken because all installed packages are up-to-date.",
   "AllShasValid": "All checked sha's are valid.",
   "_AllShasValid.comment": "sha = sha512 of url",
   "AlreadyInstalled": "{spec} is already installed",
   "_AlreadyInstalled.comment": "An example of {spec} is zlib:x64-windows.",
-  "AlreadyInstalledNotHead": "{spec} is already installed -- not building from HEAD",
-  "_AlreadyInstalledNotHead.comment": "'HEAD' means the most recent version of source code An example of {spec} is zlib:x64-windows.",
   "AmbiguousConfigDeleteConfigFile": "Ambiguous vcpkg configuration provided by both manifest and configuration file.\n-- Delete configuration file {path}",
   "_AmbiguousConfigDeleteConfigFile.comment": "An example of {path} is /foo/bar.",
   "AnArrayOfDefaultFeatures": "an array of default features",
@@ -1003,6 +1001,7 @@
   "InstalledBy": "Installed by {path}",
   "_InstalledBy.comment": "An example of {path} is /foo/bar.",
   "InstalledPackages": "The following packages are already installed:",
+  "InstalledPackagesHead": "The following packages are already installed, but were requested at --head version. Their installed contents will not be changed. To get updated versions, remove these packages first:",
   "InstalledRequestedPackages": "All requested packages are currently installed.",
   "InstallingFromFilesystemRegistry": "installing from filesystem registry here",
   "InstallingFromGitRegistry": "installing from git registry",
@@ -1486,6 +1485,8 @@
   "_ToolOfVersionXNotFound.comment": "An example of {tool_name} is signtool. An example of {version} is 1.3.8.",
   "TotalInstallTime": "Total install time: {elapsed}",
   "_TotalInstallTime.comment": "An example of {elapsed} is 3.532 min.",
+  "TotalInstallTimeSuccess": "All requested installations completed successfully in: {elapsed}",
+  "_TotalInstallTimeSuccess.comment": "An example of {elapsed} is 3.532 min.",
   "TrailingCommaInArray": "Trailing comma in array",
   "TrailingCommaInObj": "Trailing comma in an object",
   "TripletFileNotFound": "Triplet file {triplet}.cmake not found",

src/vcpkg-test/dependencies.cpp

@@ -2444,41 +2444,41 @@ TEST_CASE ("formatting plan 1", "[dependencies]")
     {
         auto formatted = format_plan(plan);
         CHECK_FALSE(formatted.has_removals);
-        CHECK(formatted.text == "All requested packages are currently installed.\n");
+        CHECK(formatted.all_text() == "All requested packages are currently installed.\n");
     }
 
     plan.remove_actions.push_back(remove_b);
     {
         auto formatted = format_plan(plan);
         CHECK(formatted.has_removals);
-        CHECK(formatted.text == "The following packages will be removed:\n"
-                                "    b:x64-osx\n");
+        CHECK(formatted.all_text() == "The following packages will be removed:\n"
+                                      "    b:x64-osx\n");
     }
 
     plan.remove_actions.push_back(remove_a);
-    REQUIRE_LINES(format_plan(plan).text,
+    REQUIRE_LINES(format_plan(plan).all_text(),
                   "The following packages will be removed:\n"
                   "    a:x64-osx\n"
                   "    b:x64-osx\n");
 
     plan.install_actions.push_back(std::move(install_c));
-    REQUIRE_LINES(format_plan(plan).text,
+    REQUIRE_LINES(format_plan(plan).all_text(),
                   "The following packages will be removed:\n"
                   "    a:x64-osx\n"
                   "    b:x64-osx\n"
                   "The following packages will be built and installed:\n"
                   "    c:x64-osx@1 -- c\n");
 
     plan.remove_actions.push_back(remove_c);
-    REQUIRE_LINES(format_plan(plan).text,
+    REQUIRE_LINES(format_plan(plan).all_text(),
                   "The following packages will be removed:\n"
                   "    a:x64-osx\n"
                   "    b:x64-osx\n"
                   "The following packages will be rebuilt:\n"
                   "    c:x64-osx@1 -- c\n");
 
     plan.install_actions.push_back(std::move(install_b));
-    REQUIRE_LINES(format_plan(plan).text,
+    REQUIRE_LINES(format_plan(plan).all_text(),
                   "The following packages will be removed:\n"
                   "    a:x64-osx\n"
                   "The following packages will be rebuilt:\n"
@@ -2492,7 +2492,7 @@ TEST_CASE ("formatting plan 1", "[dependencies]")
     {
         auto formatted = format_plan(plan);
         CHECK(formatted.has_removals);
-        REQUIRE_LINES(formatted.text,
+        REQUIRE_LINES(formatted.all_text(),
                       "The following packages are already installed:\n"
                       "  * d:x86-windows@1\n"
                       "    e:x86-windows@1\n"
@@ -2504,7 +2504,7 @@ TEST_CASE ("formatting plan 1", "[dependencies]")
     }
 
     plan.install_actions.push_back(std::move(install_f));
-    REQUIRE_LINES(format_plan(plan).text,
+    REQUIRE_LINES(format_plan(plan).all_text(),
                   "The following packages are excluded:\n"
                   "    f:x64-osx@1 -- git+https://github.com/microsoft/vcpkg:f54a99d43e600ceea175205850560f6dd37ea6cf\n"
                   "The following packages are already installed:\n"

src/vcpkg/commands.ci.cpp

@@ -513,7 +513,7 @@ namespace vcpkg
                            .append_raw(' ')
                            .append_raw(target_triplet)
                            .append_raw('\n')
-                           .append(summary.format()));
+                           .append(summary.format_results()));
             const bool any_regressions = print_regressions(summary.results,
                                                            split_specs->known,
                                                            cidata,

src/vcpkg/commands.install.cpp

@@ -330,10 +330,6 @@ namespace vcpkg
         const InstallPlanType& plan_type = action.plan_type;
         if (plan_type == InstallPlanType::ALREADY_INSTALLED)
         {
-            if (action.use_head_version == UseHeadVersion::Yes)
-                msg::println(Color::warning, msgAlreadyInstalledNotHead, msg::spec = action.spec);
-            else
-                msg::println(Color::success, msgAlreadyInstalled, msg::spec = action.spec);
             return ExtendedBuildResult{BuildResult::Succeeded};
         }
 
@@ -438,7 +434,7 @@ namespace vcpkg
             .append_raw(result.timing.to_string());
     }
 
-    LocalizedString InstallSummary::format() const
+    LocalizedString InstallSummary::format_results() const
     {
         LocalizedString to_print;
         to_print.append(msgResultsHeader).append_raw('\n');
@@ -478,26 +474,16 @@ namespace vcpkg
         msg::print(output);
     }
 
-    bool InstallSummary::failed() const
+    void InstallSummary::print_complete_message() const
     {
-        for (const auto& result : this->results)
+        if (failed)
         {
-            switch (result.build_result.value_or_exit(VCPKG_LINE_INFO).code)
-            {
-                case BuildResult::Succeeded:
-                case BuildResult::Removed:
-                case BuildResult::Downloaded:
-                case BuildResult::Excluded: continue;
-                case BuildResult::BuildFailed:
-                case BuildResult::PostBuildChecksFailed:
-                case BuildResult::FileConflicts:
-                case BuildResult::CascadedDueToMissingDependencies:
-                case BuildResult::CacheMissing: return true;
-                default: Checks::unreachable(VCPKG_LINE_INFO);
-            }
+            msg::println(msgTotalInstallTime, msg::elapsed = elapsed);
+        }
+        else
+        {
+            msg::println(Color::success, msgTotalInstallTimeSuccess, msg::elapsed = elapsed);
         }
-
-        return false;
     }
 
     struct TrackedPackageInstallGuard
@@ -580,29 +566,29 @@ namespace vcpkg
                                         const IBuildLogsRecorder& build_logs_recorder,
                                         bool include_manifest_in_github_issue)
     {
-        const ElapsedTimer timer;
-        std::vector<SpecSummary> results;
+        ElapsedTimer timer;
+        InstallSummary summary;
         const size_t action_count = action_plan.remove_actions.size() + action_plan.install_actions.size();
         size_t action_index = 1;
 
         auto& fs = paths.get_filesystem();
         for (auto&& action : action_plan.remove_actions)
         {
-            TrackedPackageInstallGuard this_install(action_index++, action_count, results, action);
+            TrackedPackageInstallGuard this_install(action_index++, action_count, summary.results, action);
             remove_package(fs, paths.installed(), action.spec, status_db);
-            results.back().build_result.emplace(BuildResult::Removed);
+            summary.results.back().build_result.emplace(BuildResult::Removed);
         }
 
         for (auto&& action : action_plan.already_installed)
         {
-            results.emplace_back(action).build_result.emplace(perform_install_plan_action(
+            summary.results.emplace_back(action).build_result.emplace(perform_install_plan_action(
                 args, paths, host_triplet, build_options, action, status_db, binary_cache, build_logs_recorder));
         }
 
         for (auto&& action : action_plan.install_actions)
         {
             binary_cache.print_updates();
-            TrackedPackageInstallGuard this_install(action_index++, action_count, results, action);
+            TrackedPackageInstallGuard this_install(action_index++, action_count, summary.results, action);
             auto result = perform_install_plan_action(
                 args, paths, host_triplet, build_options, action, status_db, binary_cache, build_logs_recorder);
             if (result.code != BuildResult::Succeeded && build_options.keep_going == KeepGoing::No)
@@ -625,11 +611,25 @@ namespace vcpkg
                 Checks::exit_fail(VCPKG_LINE_INFO);
             }
 
+            switch (result.code)
+            {
+                case BuildResult::Succeeded:
+                case BuildResult::Removed:
+                case BuildResult::Downloaded:
+                case BuildResult::Excluded: break;
+                case BuildResult::BuildFailed:
+                case BuildResult::PostBuildChecksFailed:
+                case BuildResult::FileConflicts:
+                case BuildResult::CascadedDueToMissingDependencies:
+                case BuildResult::CacheMissing: summary.failed = true; break;
+                default: Checks::unreachable(VCPKG_LINE_INFO);
+            }
             this_install.current_summary.build_result.emplace(std::move(result));
         }
 
         database_load_collapse(fs, paths.installed());
-        return InstallSummary{std::move(results), timer.elapsed()};
+        summary.elapsed = timer.elapsed();
+        return summary;
     }
 
     static constexpr CommandSwitch INSTALL_SWITCHES[] = {
@@ -1384,7 +1384,7 @@ namespace vcpkg
         // success.
         if (keep_going == KeepGoing::Yes)
         {
-            msg::print(summary.format());
+            msg::print(summary.format_results());
         }
 
         auto it_xunit = options.settings.find(SwitchXXUnit);
@@ -1419,7 +1419,8 @@ namespace vcpkg
             }
         }
         binary_cache.wait_for_async_complete_and_join();
-        Checks::exit_with_code(VCPKG_LINE_INFO, summary.failed());
+        summary.print_complete_message();
+        Checks::exit_with_code(VCPKG_LINE_INFO, summary.failed);
     }
 
     SpecSummary::SpecSummary(const InstallPlanAction& action)

src/vcpkg/commands.set-installed.cpp

@@ -168,9 +168,19 @@ namespace vcpkg
             specs_installed.erase(action.spec);
         }
 
-        Util::erase_remove_if(action_plan.install_actions, [&](const InstallPlanAction& ipa) {
-            return Util::Sets::contains(specs_installed, ipa.spec);
+        Util::erase_remove_if(action_plan.install_actions, [&](InstallPlanAction& ipa) {
+            if (Util::Sets::contains(specs_installed, ipa.spec))
+            {
+                // convert the 'to install' entry to an already installed entry
+                ipa.installed_package = status_db.get_installed_package_view(ipa.spec);
+                ipa.plan_type = InstallPlanType::ALREADY_INSTALLED;
+                action_plan.already_installed.push_back(std::move(ipa));
+                return true;
+            }
+
+            return false;
         });
+
         return specs_installed;
     }
 
@@ -269,8 +279,7 @@ namespace vcpkg
                                                   null_build_logs_recorder,
                                                   include_manifest_in_github_issue);
 
-        msg::println(msgTotalInstallTime, msg::elapsed = summary.elapsed);
-        if (build_options.keep_going == KeepGoing::Yes && summary.failed())
+        if (build_options.keep_going == KeepGoing::Yes && summary.failed)
         {
             summary.print_failed();
             if (build_options.only_downloads == OnlyDownloads::No)
@@ -308,6 +317,7 @@ namespace vcpkg
         }
 
         binary_cache.wait_for_async_complete_and_join();
+        summary.print_complete_message();
         Checks::exit_success(VCPKG_LINE_INFO);
     }
 

src/vcpkg/commands.upgrade.cpp

@@ -216,10 +216,11 @@ namespace vcpkg
         msg::println(msgTotalInstallTime, msg::elapsed = summary.elapsed);
         if (keep_going == KeepGoing::Yes)
         {
-            msg::print(summary.format());
+            msg::print(summary.format_results());
         }
 
         binary_cache.wait_for_async_complete_and_join();
+        summary.print_complete_message();
         Checks::exit_success(VCPKG_LINE_INFO);
     }
 } // namespace vcpkg