Skip to content

Commit 7b7fd34

Browse files
authored
Fix ZipBinaryProvider unpack reporting + parallelize more (#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
1 parent 67fcf69 commit 7b7fd34

File tree

11 files changed

+140
-140
lines changed

11 files changed

+140
-140
lines changed

include/vcpkg/archives.h

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -66,6 +66,4 @@ namespace vcpkg
6666
Optional<Path> seven_zip;
6767
#endif
6868
};
69-
70-
std::vector<ExpectedL<Unit>> decompress_in_parallel(View<Command> jobs);
7169
}

include/vcpkg/base/files.h

Lines changed: 2 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -153,7 +153,7 @@ namespace vcpkg
153153

154154
virtual std::vector<Path> get_files_recursive(const Path& dir, std::error_code& ec) const = 0;
155155
std::vector<Path> get_files_recursive(const Path& dir, LineInfo li) const;
156-
ExpectedL<std::vector<Path>> try_get_files_recursive(const Path& dir) const;
156+
Optional<std::vector<Path>> try_get_files_recursive(DiagnosticContext& context, const Path& dir) const;
157157

158158
virtual std::vector<Path> get_files_recursive_lexically_proximate(const Path& dir,
159159
std::error_code& ec) const = 0;
@@ -325,8 +325,7 @@ namespace vcpkg
325325
virtual int64_t last_write_time(const Path& target, std::error_code& ec) const = 0;
326326
int64_t last_write_time(const Path& target, LineInfo li) const noexcept;
327327

328-
virtual void last_write_time(const Path& target, int64_t new_time, std::error_code& ec) const = 0;
329-
void last_write_time(const Path& target, int64_t new_time, LineInfo li) const noexcept;
328+
virtual bool last_write_time(DiagnosticContext& context, const Path& target, int64_t new_time) const = 0;
330329

331330
using ReadOnlyFilesystem::current_path;
332331
virtual void current_path(const Path& new_current_path, std::error_code&) const = 0;

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

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1268,6 +1268,7 @@ DECLARE_MESSAGE(ExportedZipArchive, (msg::path), "", "Zip archive exported at: {
12681268
DECLARE_MESSAGE(ExportingAlreadyBuiltPackages, (), "", "The following packages are already built and will be exported:")
12691269
DECLARE_MESSAGE(ExportingPackage, (msg::package_name), "", "Exporting {package_name}...")
12701270
DECLARE_MESSAGE(ExtendedDocumentationAtUrl, (msg::url), "", "Extended documentation available at '{url}'.")
1271+
DECLARE_MESSAGE(ExtractedInto, (msg::path), "", "extracted into {path}")
12711272
DECLARE_MESSAGE(ExtractHelp, (), "", "Extracts an archive.")
12721273
DECLARE_MESSAGE(ExtractingTool, (msg::tool_name), "", "Extracting {tool_name}...")
12731274
DECLARE_MESSAGE(FailedPostBuildChecks,
@@ -2849,7 +2850,6 @@ DECLARE_MESSAGE(TwoFeatureFlagsSpecified,
28492850
(msg::value),
28502851
"'{value}' is a feature flag.",
28512852
"Both '{value}' and -'{value}' were specified as feature flags.")
2852-
DECLARE_MESSAGE(UnableToClearPath, (msg::path), "", "unable to delete {path}")
28532853
DECLARE_MESSAGE(UnableToReadAppDatas, (), "", "both %LOCALAPPDATA% and %APPDATA% were unreadable")
28542854
DECLARE_MESSAGE(UnableToReadEnvironmentVariable, (msg::env_var), "", "unable to read {env_var}")
28552855
DECLARE_MESSAGE(UndeterminedToolChainForTriplet,
@@ -3348,11 +3348,13 @@ DECLARE_MESSAGE(WaitUntilPackagesUploaded,
33483348
"",
33493349
"Waiting for {count} remaining binary cache submissions...")
33503350
DECLARE_MESSAGE(WarningsTreatedAsErrors, (), "", "previous warnings being interpreted as errors")
3351+
DECLARE_MESSAGE(WhileClearingThis, (), "", "while clearing this directory")
33513352
DECLARE_MESSAGE(WhileCheckingOutBaseline, (msg::commit_sha), "", "while checking out baseline {commit_sha}")
33523353
DECLARE_MESSAGE(WhileCheckingOutPortTreeIsh,
33533354
(msg::package_name, msg::git_tree_sha),
33543355
"",
33553356
"while checking out port {package_name} with git tree {git_tree_sha}")
3357+
DECLARE_MESSAGE(WhileExtractingThisArchive, (), "", "while extracting this archive")
33563358
DECLARE_MESSAGE(WhileGettingLocalTreeIshObjectsForPorts, (), "", "while getting local treeish objects for ports")
33573359
DECLARE_MESSAGE(WhileLookingForSpec, (msg::spec), "", "while looking for {spec}:")
33583360
DECLARE_MESSAGE(WhileLoadingBaselineVersionForPort,

include/vcpkg/base/parallel-algorithms.h

Lines changed: 0 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -161,10 +161,4 @@ namespace vcpkg
161161
{
162162
execute_in_parallel(c.size(), [&](size_t offset) { cb(c[offset]); });
163163
}
164-
165-
template<class Container, class RanItTarget, class F>
166-
void parallel_transform(const Container& c, RanItTarget out_begin, F cb) noexcept
167-
{
168-
execute_in_parallel(c.size(), [&](size_t offset) { out_begin[offset] = cb(c[offset]); });
169-
}
170164
}

include/vcpkg/base/system.process.h

Lines changed: 0 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -166,10 +166,6 @@ namespace vcpkg
166166
settings);
167167
}
168168

169-
std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(View<Command> commands);
170-
std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(
171-
View<Command> commands, const RedirectedProcessLaunchSettings& settings);
172-
173169
Optional<ExitCodeIntegral> cmd_execute_and_stream_lines(DiagnosticContext& context,
174170
const Command& cmd,
175171
const std::function<void(StringView)>& per_line_cb);

locales/messages.json

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -761,6 +761,8 @@
761761
"ExtendedDocumentationAtUrl": "Extended documentation available at '{url}'.",
762762
"_ExtendedDocumentationAtUrl.comment": "An example of {url} is https://github.com/microsoft/vcpkg.",
763763
"ExtractHelp": "Extracts an archive.",
764+
"ExtractedInto": "extracted into {path}",
765+
"_ExtractedInto.comment": "An example of {path} is /foo/bar.",
764766
"ExtractingTool": "Extracting {tool_name}...",
765767
"_ExtractingTool.comment": "An example of {tool_name} is signtool.",
766768
"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.",
@@ -1499,8 +1501,6 @@
14991501
"TripletLabel": "Triplet:",
15001502
"TwoFeatureFlagsSpecified": "Both '{value}' and -'{value}' were specified as feature flags.",
15011503
"_TwoFeatureFlagsSpecified.comment": "'{value}' is a feature flag.",
1502-
"UnableToClearPath": "unable to delete {path}",
1503-
"_UnableToClearPath.comment": "An example of {path} is /foo/bar.",
15041504
"UnableToReadAppDatas": "both %LOCALAPPDATA% and %APPDATA% were unreadable",
15051505
"UnableToReadEnvironmentVariable": "unable to read {env_var}",
15061506
"_UnableToReadEnvironmentVariable.comment": "An example of {env_var} is VCPKG_DEFAULT_TRIPLET.",
@@ -1755,6 +1755,8 @@
17551755
"_WhileCheckingOutBaseline.comment": "An example of {commit_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949.",
17561756
"WhileCheckingOutPortTreeIsh": "while checking out port {package_name} with git tree {git_tree_sha}",
17571757
"_WhileCheckingOutPortTreeIsh.comment": "An example of {package_name} is zlib. An example of {git_tree_sha} is 7cfad47ae9f68b183983090afd6337cd60fd4949.",
1758+
"WhileClearingThis": "while clearing this directory",
1759+
"WhileExtractingThisArchive": "while extracting this archive",
17581760
"WhileGettingLocalTreeIshObjectsForPorts": "while getting local treeish objects for ports",
17591761
"WhileLoadingBaselineVersionForPort": "while loading baseline version for {package_name}",
17601762
"_WhileLoadingBaselineVersionForPort.comment": "An example of {package_name} is zlib.",

src/vcpkg-test/system.cpp

Lines changed: 0 additions & 28 deletions
Original file line numberDiff line numberDiff line change
@@ -134,34 +134,6 @@ TEST_CASE ("cmdlinebuilder", "[system]")
134134
#endif
135135
}
136136

137-
TEST_CASE ("cmd_execute_and_capture_output_parallel", "[system]")
138-
{
139-
std::vector<Command> vec;
140-
for (size_t i = 0; i < 50; ++i)
141-
{
142-
#if defined(_WIN32)
143-
vec.push_back(Command("cmd.exe").string_arg("/d").string_arg("/c").string_arg(fmt::format("echo {}", i)));
144-
#else
145-
vec.push_back(Command("echo").string_arg(std::string(i, 'a')));
146-
#endif
147-
}
148-
149-
auto res = cmd_execute_and_capture_output_parallel(vec);
150-
151-
for (size_t i = 0; i != res.size(); ++i)
152-
{
153-
auto out = res[i].get();
154-
REQUIRE(out != nullptr);
155-
REQUIRE(out->exit_code == 0);
156-
157-
#if defined(_WIN32)
158-
REQUIRE(out->output == (fmt::format("{}\r\n", i)));
159-
#else
160-
REQUIRE(out->output == (std::string(i, 'a') + "\n"));
161-
#endif
162-
}
163-
}
164-
165137
TEST_CASE ("append_shell_escaped", "[system]")
166138
{
167139
Command cmd;

src/vcpkg/archives.cpp

Lines changed: 0 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -368,19 +368,4 @@ namespace vcpkg
368368
#endif
369369
return cmd;
370370
}
371-
372-
std::vector<ExpectedL<Unit>> decompress_in_parallel(View<Command> jobs)
373-
{
374-
RedirectedProcessLaunchSettings settings;
375-
settings.environment = get_clean_environment();
376-
auto results = cmd_execute_and_capture_output_parallel(jobs, settings);
377-
std::vector<ExpectedL<Unit>> filtered_results;
378-
filtered_results.reserve(jobs.size());
379-
for (std::size_t idx = 0; idx < jobs.size(); ++idx)
380-
{
381-
filtered_results.push_back(flatten(results[idx], jobs[idx].command_line()));
382-
}
383-
384-
return filtered_results;
385-
}
386371
}

src/vcpkg/base/files.cpp

Lines changed: 40 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -1664,16 +1664,23 @@ namespace vcpkg
16641664

16651665
std::vector<Path> ReadOnlyFilesystem::get_files_recursive(const Path& dir, LineInfo li) const
16661666
{
1667-
return this->try_get_files_recursive(dir).value_or_exit(li);
1667+
return this->try_get_files_recursive(console_diagnostic_context, dir).value_or_exit(li);
16681668
}
16691669

1670-
ExpectedL<std::vector<Path>> ReadOnlyFilesystem::try_get_files_recursive(const Path& dir) const
1670+
Optional<std::vector<Path>> ReadOnlyFilesystem::try_get_files_recursive(DiagnosticContext& context,
1671+
const Path& dir) const
16711672
{
16721673
std::error_code ec;
16731674
auto maybe_files = this->get_files_recursive(dir, ec);
16741675
if (ec)
16751676
{
1676-
return format_filesystem_call_error(ec, __func__, {dir});
1677+
context.report(DiagnosticLine{DiagKind::Error,
1678+
dir,
1679+
msg::format(msgSystemApiErrorMessage,
1680+
msg::system_api = "get_files_recursive",
1681+
msg::exit_code = ec.value(),
1682+
msg::error_msg = ec.message())});
1683+
return nullopt;
16771684
}
16781685

16791686
return maybe_files;
@@ -2279,16 +2286,6 @@ namespace vcpkg
22792286
return result;
22802287
}
22812288

2282-
void Filesystem::last_write_time(const Path& target, int64_t new_time, vcpkg::LineInfo li) const noexcept
2283-
{
2284-
std::error_code ec;
2285-
this->last_write_time(target, new_time, ec);
2286-
if (ec)
2287-
{
2288-
exit_filesystem_call_error(li, ec, __func__, {target});
2289-
}
2290-
}
2291-
22922289
void Filesystem::write_lines(const Path& file_path, const std::vector<std::string>& lines, LineInfo li) const
22932290
{
22942291
std::error_code ec;
@@ -3926,29 +3923,56 @@ namespace vcpkg
39263923
#endif // ^^^ !_WIN32
39273924
}
39283925

3929-
void last_write_time(const Path& target, int64_t new_time, std::error_code& ec) const override
3926+
virtual bool last_write_time(DiagnosticContext& context, const Path& target, int64_t new_time) const override
39303927
{
3928+
std::error_code ec;
39313929
#if defined(_WIN32)
39323930
stdfs::last_write_time(to_stdfs_path(target),
39333931
stdfs::file_time_type::time_point{stdfs::file_time_type::time_point::duration {
39343932
new_time
39353933
}},
39363934
ec);
3935+
if (ec)
3936+
{
3937+
context.report(DiagnosticLine{DiagKind::Error,
3938+
target,
3939+
msg::format(msgSystemApiErrorMessage,
3940+
msg::system_api = "std::fs::last_write_time",
3941+
msg::exit_code = ec.value(),
3942+
msg::error_msg = ec.message())});
3943+
return false;
3944+
}
39373945

3946+
return true;
39383947
#else // ^^^ _WIN32 // !_WIN32 vvv
39393948
PosixFd fd(target.c_str(), O_WRONLY, ec);
39403949
if (ec)
39413950
{
3942-
return;
3951+
context.report(DiagnosticLine{DiagKind::Error,
3952+
target,
3953+
msg::format(msgSystemApiErrorMessage,
3954+
msg::system_api = "open",
3955+
msg::exit_code = ec.value(),
3956+
msg::error_msg = ec.message())});
3957+
return false;
39433958
}
39443959
timespec times[2]; // last access and modification time
39453960
times[0].tv_nsec = UTIME_OMIT;
39463961
times[1].tv_nsec = new_time % 1'000'000'000;
39473962
times[1].tv_sec = new_time / 1'000'000'000;
39483963
if (futimens(fd.get(), times))
39493964
{
3950-
ec.assign(errno, std::system_category());
3965+
auto local_errno = errno;
3966+
context.report(
3967+
DiagnosticLine{DiagKind::Error,
3968+
target,
3969+
msg::format(msgSystemApiErrorMessage,
3970+
msg::system_api = "futimens",
3971+
msg::exit_code = local_errno,
3972+
msg::error_msg = std::system_category().message(local_errno))});
39513973
}
3974+
3975+
return true;
39523976
#endif // ^^^ !_WIN32
39533977
}
39543978

src/vcpkg/base/system.process.cpp

Lines changed: 0 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,7 +4,6 @@
44
#include <vcpkg/base/chrono.h>
55
#include <vcpkg/base/contractual-constants.h>
66
#include <vcpkg/base/files.h>
7-
#include <vcpkg/base/parallel-algorithms.h>
87
#include <vcpkg/base/parse.h>
98
#include <vcpkg/base/strings.h>
109
#include <vcpkg/base/system.debug.h>
@@ -746,23 +745,6 @@ namespace vcpkg
746745
static const Environment clean_env = get_modified_clean_environment({});
747746
return clean_env;
748747
}
749-
750-
std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(View<Command> commands)
751-
{
752-
RedirectedProcessLaunchSettings default_redirected_process_launch_settings;
753-
return cmd_execute_and_capture_output_parallel(commands, default_redirected_process_launch_settings);
754-
}
755-
756-
std::vector<ExpectedL<ExitCodeAndOutput>> cmd_execute_and_capture_output_parallel(
757-
View<Command> commands, const RedirectedProcessLaunchSettings& settings)
758-
{
759-
std::vector<ExpectedL<ExitCodeAndOutput>> res(commands.size(), LocalizedString{});
760-
761-
parallel_transform(
762-
commands, res.begin(), [&](const Command& cmd) { return cmd_execute_and_capture_output(cmd, settings); });
763-
764-
return res;
765-
}
766748
} // namespace vcpkg
767749

768750
namespace

0 commit comments

Comments
 (0)