Fix ZipBinaryProvider unpack reporting + parallelize more (microsoft#1805)

* Fixes the incorrect tracking bug introduced here: https://github.com/microsoft/vcpkg-tool/pull/1715/files#r2414987620
* Gets clean_prepare_dir and the last_write_time fixup into the parallelized region

Related to microsoft/vcpkg#43309

* Plumbs error handling through to one place
* Deletes decompress_in_parallel / cmd_execute_and_capture_output_parallel

Author: BillyONeal

include/vcpkg/archives.h

@@ -66,6 +66,4 @@ namespace vcpkg
         Optional<Path> seven_zip;
 #endif
     };
-
-    std::vector<ExpectedL<Unit>> decompress_in_parallel(View<Command> jobs);
 }

include/vcpkg/base/files.h

@@ -155,7 +155,7 @@ namespace vcpkg
 
         virtual std::vector<Path> get_files_recursive(const Path& dir, std::error_code& ec) const = 0;
         std::vector<Path> get_files_recursive(const Path& dir, LineInfo li) const;
-        ExpectedL<std::vector<Path>> try_get_files_recursive(const Path& dir) const;
+        Optional<std::vector<Path>> try_get_files_recursive(DiagnosticContext& context, const Path& dir) const;
 
         virtual std::vector<Path> get_files_recursive_lexically_proximate(const Path& dir,
                                                                           std::error_code& ec) const = 0;
@@ -327,8 +327,7 @@ namespace vcpkg
         virtual int64_t last_write_time(const Path& target, std::error_code& ec) const = 0;
         int64_t last_write_time(const Path& target, LineInfo li) const noexcept;
 
-        virtual void last_write_time(const Path& target, int64_t new_time, std::error_code& ec) const = 0;
-        void last_write_time(const Path& target, int64_t new_time, LineInfo li) const noexcept;
+        virtual bool last_write_time(DiagnosticContext& context, const Path& target, int64_t new_time) const = 0;
 
         using ReadOnlyFilesystem::current_path;
         virtual void current_path(const Path& new_current_path, std::error_code&) const = 0;

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

@@ -1270,6 +1270,7 @@ DECLARE_MESSAGE(ExportedZipArchive, (msg::path), "", "Zip archive exported at: {
 DECLARE_MESSAGE(ExportingAlreadyBuiltPackages, (), "", "The following packages are already built and will be exported:")
 DECLARE_MESSAGE(ExportingPackage, (msg::package_name), "", "Exporting {package_name}...")
 DECLARE_MESSAGE(ExtendedDocumentationAtUrl, (msg::url), "", "Extended documentation available at '{url}'.")
+DECLARE_MESSAGE(ExtractedInto, (msg::path), "", "extracted into {path}")
 DECLARE_MESSAGE(ExtractHelp, (), "", "Extracts an archive.")
 DECLARE_MESSAGE(ExtractingTool, (msg::tool_name), "", "Extracting {tool_name}...")
 DECLARE_MESSAGE(FailedPostBuildChecks,
@@ -2851,7 +2852,6 @@ DECLARE_MESSAGE(TwoFeatureFlagsSpecified,
                 (msg::value),
                 "'{value}' is a feature flag.",
                 "Both '{value}' and -'{value}' were specified as feature flags.")
-DECLARE_MESSAGE(UnableToClearPath, (msg::path), "", "unable to delete {path}")
 DECLARE_MESSAGE(UnableToReadAppDatas, (), "", "both %LOCALAPPDATA% and %APPDATA% were unreadable")
 DECLARE_MESSAGE(UnableToReadEnvironmentVariable, (msg::env_var), "", "unable to read {env_var}")
 DECLARE_MESSAGE(UndeterminedToolChainForTriplet,
@@ -3350,11 +3350,13 @@ DECLARE_MESSAGE(WaitUntilPackagesUploaded,
                 "",
                 "Waiting for {count} remaining binary cache submissions...")
 DECLARE_MESSAGE(WarningsTreatedAsErrors, (), "", "previous warnings being interpreted as errors")
+DECLARE_MESSAGE(WhileClearingThis, (), "", "while clearing this directory")
 DECLARE_MESSAGE(WhileCheckingOutBaseline, (msg::commit_sha), "", "while checking out baseline {commit_sha}")
 DECLARE_MESSAGE(WhileCheckingOutPortTreeIsh,
                 (msg::package_name, msg::git_tree_sha),
                 "",
                 "while checking out port {package_name} with git tree {git_tree_sha}")
+DECLARE_MESSAGE(WhileExtractingThisArchive, (), "", "while extracting this archive")
 DECLARE_MESSAGE(WhileGettingLocalTreeIshObjectsForPorts, (), "", "while getting local treeish objects for ports")
 DECLARE_MESSAGE(WhileLookingForSpec, (msg::spec), "", "while looking for {spec}:")
 DECLARE_MESSAGE(WhileLoadingBaselineVersionForPort,

include/vcpkg/base/parallel-algorithms.h

@@ -161,10 +161,4 @@ namespace vcpkg
     {
         execute_in_parallel(c.size(), [&](size_t offset) { cb(c[offset]); });
     }
-
-    template<class Container, class RanItTarget, class F>
-    void parallel_transform(const Container& c, RanItTarget out_begin, F cb) noexcept
-    {
-        execute_in_parallel(c.size(), [&](size_t offset) { out_begin[offset] = cb(c[offset]); });
-    }
 }

include/vcpkg/base/system.process.h

@@ -166,10 +166,6 @@ namespace vcpkg
             settings);
     }
 
-    std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(View<Command> commands);
-    std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(
-        View<Command> commands, const RedirectedProcessLaunchSettings& settings);
-
     Optional<ExitCodeIntegral> cmd_execute_and_stream_lines(DiagnosticContext& context,
                                                             const Command& cmd,
                                                             const std::function<void(StringView)>& per_line_cb);

locales/messages.json

@@ -764,6 +764,8 @@
   "ExtendedDocumentationAtUrl": "Extended documentation available at '{url}'.",
   "_ExtendedDocumentationAtUrl.comment": "An example of {url} is https://github.com/microsoft/vcpkg.",
   "ExtractHelp": "Extracts an archive.",
+  "ExtractedInto": "extracted into {path}",
+  "_ExtractedInto.comment": "An example of {path} is /foo/bar.",
   "ExtractingTool": "Extracting {tool_name}...",
   "_ExtractingTool.comment": "An example of {tool_name} is signtool.",
   "FailedPostBuildChecks": "Found {count} post-build check problem(s). These are usually caused by bugs in portfile.cmake or the upstream build system. Please correct these before submitting this port to the curated registry.",
@@ -1502,8 +1504,6 @@
   "TripletLabel": "Triplet:",
   "TwoFeatureFlagsSpecified": "Both '{value}' and -'{value}' were specified as feature flags.",
   "_TwoFeatureFlagsSpecified.comment": "'{value}' is a feature flag.",
-  "UnableToClearPath": "unable to delete {path}",
-  "_UnableToClearPath.comment": "An example of {path} is /foo/bar.",
   "UnableToReadAppDatas": "both %LOCALAPPDATA% and %APPDATA% were unreadable",
   "UnableToReadEnvironmentVariable": "unable to read {env_var}",
   "_UnableToReadEnvironmentVariable.comment": "An example of {env_var} is VCPKG_DEFAULT_TRIPLET.",
@@ -1758,6 +1758,8 @@
   "_WhileCheckingOutBaseline.comment": "An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949.",
   "WhileCheckingOutPortTreeIsh": "while checking out port {package_name} with git tree {git_tree_sha}",
   "_WhileCheckingOutPortTreeIsh.comment": "An example of {package_name} is zlib. An example of {git_tree_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949.",
+  "WhileClearingThis": "while clearing this directory",
+  "WhileExtractingThisArchive": "while extracting this archive",
   "WhileGettingLocalTreeIshObjectsForPorts": "while getting local treeish objects for ports",
   "WhileLoadingBaselineVersionForPort": "while loading baseline version for {package_name}",
   "_WhileLoadingBaselineVersionForPort.comment": "An example of {package_name} is zlib.",

src/vcpkg-test/system.cpp

@@ -134,34 +134,6 @@ TEST_CASE ("cmdlinebuilder", "[system]")
 #endif
 }
 
-TEST_CASE ("cmd_execute_and_capture_output_parallel", "[system]")
-{
-    std::vector<Command> vec;
-    for (size_t i = 0; i < 50; ++i)
-    {
-#if defined(_WIN32)
-        vec.push_back(Command("cmd.exe").string_arg("/d").string_arg("/c").string_arg(fmt::format("echo {}", i)));
-#else
-        vec.push_back(Command("echo").string_arg(std::string(i, 'a')));
-#endif
-    }
-
-    auto res = cmd_execute_and_capture_output_parallel(vec);
-
-    for (size_t i = 0; i != res.size(); ++i)
-    {
-        auto out = res[i].get();
-        REQUIRE(out != nullptr);
-        REQUIRE(out->exit_code == 0);
-
-#if defined(_WIN32)
-        REQUIRE(out->output == (fmt::format("{}\r\n", i)));
-#else
-        REQUIRE(out->output == (std::string(i, 'a') + "\n"));
-#endif
-    }
-}
-
 TEST_CASE ("append_shell_escaped", "[system]")
 {
     Command cmd;

src/vcpkg/archives.cpp

@@ -368,19 +368,4 @@ namespace vcpkg
 #endif
         return cmd;
     }
-
-    std::vector<ExpectedL<Unit>> decompress_in_parallel(View<Command> jobs)
-    {
-        RedirectedProcessLaunchSettings settings;
-        settings.environment = get_clean_environment();
-        auto results = cmd_execute_and_capture_output_parallel(jobs, settings);
-        std::vector<ExpectedL<Unit>> filtered_results;
-        filtered_results.reserve(jobs.size());
-        for (std::size_t idx = 0; idx < jobs.size(); ++idx)
-        {
-            filtered_results.push_back(flatten(results[idx], jobs[idx].command_line()));
-        }
-
-        return filtered_results;
-    }
 }

src/vcpkg/base/files.cpp

@@ -1693,16 +1693,23 @@ namespace vcpkg
 
     std::vector<Path> ReadOnlyFilesystem::get_files_recursive(const Path& dir, LineInfo li) const
     {
-        return this->try_get_files_recursive(dir).value_or_exit(li);
+        return this->try_get_files_recursive(console_diagnostic_context, dir).value_or_exit(li);
     }
 
-    ExpectedL<std::vector<Path>> ReadOnlyFilesystem::try_get_files_recursive(const Path& dir) const
+    Optional<std::vector<Path>> ReadOnlyFilesystem::try_get_files_recursive(DiagnosticContext& context,
+                                                                            const Path& dir) const
     {
         std::error_code ec;
         auto maybe_files = this->get_files_recursive(dir, ec);
         if (ec)
         {
-            return format_filesystem_call_error(ec, __func__, {dir});
+            context.report(DiagnosticLine{DiagKind::Error,
+                                          dir,
+                                          msg::format(msgSystemApiErrorMessage,
+                                                      msg::system_api = "get_files_recursive",
+                                                      msg::exit_code = ec.value(),
+                                                      msg::error_msg = ec.message())});
+            return nullopt;
         }
 
         return maybe_files;
@@ -2308,16 +2315,6 @@ namespace vcpkg
         return result;
     }
 
-    void Filesystem::last_write_time(const Path& target, int64_t new_time, vcpkg::LineInfo li) const noexcept
-    {
-        std::error_code ec;
-        this->last_write_time(target, new_time, ec);
-        if (ec)
-        {
-            exit_filesystem_call_error(li, ec, __func__, {target});
-        }
-    }
-
     void Filesystem::write_lines(const Path& file_path, const std::vector<std::string>& lines, LineInfo li) const
     {
         std::error_code ec;
@@ -3955,29 +3952,56 @@ namespace vcpkg
 #endif // ^^^ !_WIN32
         }
 
-        void last_write_time(const Path& target, int64_t new_time, std::error_code& ec) const override
+        virtual bool last_write_time(DiagnosticContext& context, const Path& target, int64_t new_time) const override
         {
+            std::error_code ec;
 #if defined(_WIN32)
             stdfs::last_write_time(to_stdfs_path(target),
                                    stdfs::file_time_type::time_point{stdfs::file_time_type::time_point::duration {
                                        new_time
                                    }},
                                    ec);
+            if (ec)
+            {
+                context.report(DiagnosticLine{DiagKind::Error,
+                                              target,
+                                              msg::format(msgSystemApiErrorMessage,
+                                                          msg::system_api = "std::fs::last_write_time",
+                                                          msg::exit_code = ec.value(),
+                                                          msg::error_msg = ec.message())});
+                return false;
+            }
 
+            return true;
 #else  // ^^^ _WIN32 // !_WIN32 vvv
             PosixFd fd(target.c_str(), O_WRONLY, ec);
             if (ec)
             {
-                return;
+                context.report(DiagnosticLine{DiagKind::Error,
+                                              target,
+                                              msg::format(msgSystemApiErrorMessage,
+                                                          msg::system_api = "open",
+                                                          msg::exit_code = ec.value(),
+                                                          msg::error_msg = ec.message())});
+                return false;
             }
             timespec times[2]; // last access and modification time
             times[0].tv_nsec = UTIME_OMIT;
             times[1].tv_nsec = new_time % 1'000'000'000;
             times[1].tv_sec = new_time / 1'000'000'000;
             if (futimens(fd.get(), times))
             {
-                ec.assign(errno, std::system_category());
+                auto local_errno = errno;
+                context.report(
+                    DiagnosticLine{DiagKind::Error,
+                                   target,
+                                   msg::format(msgSystemApiErrorMessage,
+                                               msg::system_api = "futimens",
+                                               msg::exit_code = local_errno,
+                                               msg::error_msg = std::system_category().message(local_errno))});
             }
+
+            return true;
 #endif // ^^^ !_WIN32
         }
 

src/vcpkg/base/system.process.cpp

@@ -4,7 +4,6 @@
 #include <vcpkg/base/chrono.h>
 #include <vcpkg/base/contractual-constants.h>
 #include <vcpkg/base/files.h>
-#include <vcpkg/base/parallel-algorithms.h>
 #include <vcpkg/base/parse.h>
 #include <vcpkg/base/strings.h>
 #include <vcpkg/base/system.debug.h>
@@ -746,23 +745,6 @@ namespace vcpkg
         static const Environment clean_env = get_modified_clean_environment({});
         return clean_env;
     }
-
-    std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(View<Command> commands)
-    {
-        RedirectedProcessLaunchSettings default_redirected_process_launch_settings;
-        return cmd_execute_and_capture_output_parallel(commands, default_redirected_process_launch_settings);
-    }
-
-    std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(
-        View<Command> commands, const RedirectedProcessLaunchSettings& settings)
-    {
-        std::vector<ExpectedL<ExitCodeAndOutput>> res(commands.size(), LocalizedString{});
-
-        parallel_transform(
-            commands, res.begin(), [&](const Command& cmd) { return cmd_execute_and_capture_output(cmd, settings); });
-
-        return res;
-    }
 } // namespace vcpkg
 
 namespace