Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Avoid leaking doskey macros from internal vcvarsall runs. #1295

Merged
merged 6 commits into from
Jan 10, 2024
Merged

Avoid leaking doskey macros from internal vcvarsall runs. #1295

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
ae96f48
Turn the blizzard of parameters for process launches into structs.
BillyONeal Dec 1, 2023
473e586
Fix POSIX build.
BillyONeal Dec 1, 2023
95065c7
Attempt to resolve https://github.com/microsoft/vcpkg/issues/32641 by…
BillyONeal Dec 1, 2023
e4ddb8b
Fix macos warnings.
BillyONeal Dec 1, 2023
78ee5bf
Merge remote-tracking branch 'origin/main' into create_new_console
BillyONeal Dec 12, 2023
1a5893f
Apply code review fixes from @ras0219-msft .
BillyONeal Dec 13, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion include/vcpkg/base/fwd/system.process.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,15 @@ namespace vcpkg
Utf16
};

enum class CreateNewConsole
{
No,
Yes
};

struct CMakeVariable;
struct Command;
struct CommandLess;
struct ExitCodeAndOutput;
struct Environment;
struct WorkingDirectory;
}
68 changes: 35 additions & 33 deletions include/vcpkg/base/system.process.h
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -109,51 +109,53 @@ namespace vcpkg
Environment get_modified_clean_environment(const std::unordered_map<std::string, std::string>& extra_env,
StringView prepend_to_path = {});

struct WorkingDirectory
struct ProcessLaunchSettings
{
Path working_directory;
Optional<Path> working_directory;
Optional<Environment> environment;
};

extern const WorkingDirectory default_working_directory;
extern const Environment default_environment;
struct RedirectedProcessLaunchSettings
{
Optional<Path> working_directory;
Optional<Environment> environment;

#if defined(_WIN32)
// the encoding to use for standard streams of the child
Encoding encoding = Encoding::Utf8;
CreateNewConsole create_new_console = CreateNewConsole::No;
#endif // ^^^ _WIN32
// whether to echo all read content to the enclosing terminal;
EchoInDebug echo_in_debug = EchoInDebug::Hide;
std::string stdin_content;
};

ExpectedL<int> cmd_execute(const Command& cmd_line,
const WorkingDirectory& wd = default_working_directory,
const Environment& env = default_environment);
ExpectedL<int> cmd_execute_clean(const Command& cmd_line, const WorkingDirectory& wd = default_working_directory);
ExpectedL<int> cmd_execute(const Command& cmd);
ExpectedL<int> cmd_execute(const Command& cmd, const ProcessLaunchSettings& settings);

#if defined(_WIN32)
Environment cmd_execute_and_capture_environment(const Command& cmd_line,
const Environment& env = default_environment);
Environment cmd_execute_and_capture_environment(const Command& cmd, const Environment& env);
#endif

void cmd_execute_background(const Command& cmd_line);

ExpectedL<ExitCodeAndOutput> cmd_execute_and_capture_output(const Command& cmd_line,
const WorkingDirectory& wd = default_working_directory,
const Environment& env = default_environment,
Encoding encoding = Encoding::Utf8,
EchoInDebug echo_in_debug = EchoInDebug::Hide,
StringView stdin_content = {});
ExpectedL<ExitCodeAndOutput> cmd_execute_and_capture_output(const Command& cmd);
ExpectedL<ExitCodeAndOutput> cmd_execute_and_capture_output(const Command& cmd,
const RedirectedProcessLaunchSettings& 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> cmd_lines,
const WorkingDirectory& wd = default_working_directory,
const Environment& env = default_environment);

ExpectedL<int> cmd_execute_and_stream_lines(const Command& cmd_line,
const std::function<void(StringView)>& per_line_cb,
const WorkingDirectory& wd = default_working_directory,
const Environment& env = default_environment,
Encoding encoding = Encoding::Utf8,
StringView stdin_content = {});

ExpectedL<int> cmd_execute_and_stream_data(const Command& cmd_line,
const std::function<void(StringView)>& data_cb,
const WorkingDirectory& wd = default_working_directory,
const Environment& env = default_environment,
Encoding encoding = Encoding::Utf8,
StringView stdin_content = {});
View<Command> commands, const RedirectedProcessLaunchSettings& settings);

ExpectedL<int> cmd_execute_and_stream_lines(const Command& cmd, const std::function<void(StringView)>& per_line_cb);
ExpectedL<int> cmd_execute_and_stream_lines(const Command& cmd,
const RedirectedProcessLaunchSettings& settings,
const std::function<void(StringView)>& per_line_cb);

ExpectedL<int> cmd_execute_and_stream_data(const Command& cmd, const std::function<void(StringView)>& data_cb);
ExpectedL<int> cmd_execute_and_stream_data(const Command& cmd,
const RedirectedProcessLaunchSettings& settings,
const std::function<void(StringView)>& data_cb);

uint64_t get_subproccess_stats();

Expand Down
28 changes: 5 additions & 23 deletions src/vcpkg-test/system.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,15 +9,7 @@

#include <string>

using vcpkg::CPUArchitecture;
using vcpkg::get_environment_variable;
using vcpkg::guess_visual_studio_prompt_target_architecture;
using vcpkg::nullopt;
using vcpkg::Optional;
using vcpkg::set_environment_variable;
using vcpkg::StringView;
using vcpkg::to_cpu_architecture;
using vcpkg::ZStringView;
using namespace vcpkg;

namespace
{
Expand Down Expand Up @@ -123,8 +115,6 @@ TEST_CASE ("guess_visual_studio_prompt", "[system]")

TEST_CASE ("cmdlinebuilder", "[system]")
{
using vcpkg::Command;

Command cmd;
cmd.string_arg("relative/path.exe");
cmd.string_arg("abc");
Expand All @@ -146,23 +136,17 @@ TEST_CASE ("cmdlinebuilder", "[system]")

TEST_CASE ("cmd_execute_and_capture_output_parallel", "[system]")
{
std::vector<vcpkg::Command> vec;
std::vector<Command> vec;
for (size_t i = 0; i < 50; ++i)
{
#if defined(_WIN32)
vcpkg::Command cmd("cmd.exe");
cmd.string_arg("/d");
cmd.string_arg("/c");
cmd.string_arg(fmt::format("echo {}", i));
vec.push_back(Command("cmd.exe").string_arg("/d").string_arg("/c").string_arg(fmt::format("echo {}", i)));
#else
vcpkg::Command cmd("echo");
const auto cmd_str = std::string(i, 'a');
cmd.string_arg(cmd_str);
vec.push_back(Command("echo").string_arg(std::string(i, 'a')));
#endif
vec.emplace_back(std::move(cmd));
}

auto res = vcpkg::cmd_execute_and_capture_output_parallel(vcpkg::View<vcpkg::Command>(vec));
auto res = cmd_execute_and_capture_output_parallel(vec);

for (size_t i = 0; i != res.size(); ++i)
{
Expand All @@ -180,8 +164,6 @@ TEST_CASE ("cmd_execute_and_capture_output_parallel", "[system]")

TEST_CASE ("append_shell_escaped", "[system]")
{
using vcpkg::Command;

Command cmd;

cmd.clear();
Expand Down
33 changes: 10 additions & 23 deletions src/vcpkg-test/system.process.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -7,16 +7,15 @@ using namespace vcpkg;
TEST_CASE ("captures-output", "[system.process]")
{
auto test_program = Path(get_exe_path_of_current_process().parent_path()) / "reads-stdin";
Command cmd{test_program};
cmd.string_arg("this is printed when something is read");
auto cmd = Command{test_program}.string_arg("this is printed when something is read");
static constexpr std::size_t minimum_size = 1'000'000; // to exceed OS pipe buffer size
constexpr StringLiteral example = "example";
constexpr auto examples = (minimum_size / example.size()) + 1;
std::string input;
constexpr auto input_size = examples * example.size();
RedirectedProcessLaunchSettings settings;
for (std::size_t idx = 0; idx < examples; ++idx)
{
input.append(example.data(), example.size());
settings.stdin_content.append(example.data(), example.size());
}

std::string expected;
Expand All @@ -37,39 +36,27 @@ TEST_CASE ("captures-output", "[system.process]")
#endif // ^^^ _WIN32
expected.push_back('\n');

auto run = cmd_execute_and_capture_output(
cmd, default_working_directory, default_environment, Encoding::Utf8, EchoInDebug::Hide, input)
.value_or_exit(VCPKG_LINE_INFO);
auto run = cmd_execute_and_capture_output(cmd, settings).value_or_exit(VCPKG_LINE_INFO);
REQUIRE(run.exit_code == 0);
REQUIRE(run.output == expected);
}

TEST_CASE ("no closes-stdin crash", "[system.process]")
{
auto test_program = Path(get_exe_path_of_current_process().parent_path()) / "closes-stdin";
Command cmd{test_program};
auto run = cmd_execute_and_capture_output(cmd,
default_working_directory,
default_environment,
Encoding::Utf8,
EchoInDebug::Hide,
"this is some input that will be intentionally not read")
.value_or_exit(VCPKG_LINE_INFO);
RedirectedProcessLaunchSettings settings;
settings.stdin_content = "this is some input that will be intentionally not read";
auto run = cmd_execute_and_capture_output(Command{test_program}, settings).value_or_exit(VCPKG_LINE_INFO);
REQUIRE(run.exit_code == 0);
REQUIRE(run.output.empty());
}

TEST_CASE ("no closes-stdout crash", "[system.process]")
{
auto test_program = Path(get_exe_path_of_current_process().parent_path()) / "closes-stdout";
Command cmd{test_program};
auto run = cmd_execute_and_capture_output(cmd,
default_working_directory,
default_environment,
Encoding::Utf8,
EchoInDebug::Hide,
"this is some input that will be read")
.value_or_exit(VCPKG_LINE_INFO);
RedirectedProcessLaunchSettings settings;
settings.stdin_content = "this is some input that will be intentionally not read";
auto run = cmd_execute_and_capture_output(Command{test_program}, settings).value_or_exit(VCPKG_LINE_INFO);
REQUIRE(run.exit_code == 0);
REQUIRE(run.output == "hello world");
}
99 changes: 51 additions & 48 deletions src/vcpkg/archives.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -32,23 +32,23 @@ namespace
StringView nugetid{stem.begin(), dot_after_name};
StringView version{dot_after_name + 1, stem.end()};

Command nuget_command{nuget_exe};
nuget_command.string_arg("install")
.string_arg(nugetid)
.string_arg("-Version")
.string_arg(version)
.string_arg("-OutputDirectory")
.string_arg(to_path)
.string_arg("-Source")
.string_arg(archive.parent_path())
.string_arg("-nocache")
.string_arg("-DirectDownload")
.string_arg("-NonInteractive")
.string_arg("-ForceEnglishOutput")
.string_arg("-PackageSaveMode")
.string_arg("nuspec");

const auto result = flatten(cmd_execute_and_capture_output(nuget_command), Tools::NUGET);
auto cmd = Command{nuget_exe}
.string_arg("install")
.string_arg(nugetid)
.string_arg("-Version")
.string_arg(version)
.string_arg("-OutputDirectory")
.string_arg(to_path)
.string_arg("-Source")
.string_arg(archive.parent_path())
.string_arg("-nocache")
.string_arg("-DirectDownload")
.string_arg("-NonInteractive")
.string_arg("-ForceEnglishOutput")
.string_arg("-PackageSaveMode")
.string_arg("nuspec");

const auto result = flatten(cmd_execute_and_capture_output(cmd), Tools::NUGET);
if (!result)
{
Checks::msg_exit_with_message(
Expand All @@ -65,21 +65,20 @@ namespace
{
// msiexec is a WIN32/GUI application, not a console application and so needs special attention to wait
// until it finishes (wrap in cmd /c).
const auto maybe_code_and_output = cmd_execute_and_capture_output(
Command{"cmd"}
.string_arg("/c")
.string_arg("msiexec")
// "/a" is administrative mode, which unpacks without modifying the system
.string_arg("/a")
.string_arg(archive)
.string_arg("/qn")
// msiexec requires quotes to be after "TARGETDIR=":
// TARGETDIR="C:\full\path\to\dest"
.raw_arg(Strings::concat("TARGETDIR=", Command{to_path}.extract())),
default_working_directory,
default_environment,
Encoding::Utf16);

auto cmd = Command{"cmd"}
.string_arg("/c")
.string_arg("msiexec")
// "/a" is administrative mode, which unpacks without modifying the system
.string_arg("/a")
.string_arg(archive)
.string_arg("/qn")
// msiexec requires quotes to be after "TARGETDIR=":
// TARGETDIR="C:\full\path\to\dest"
.raw_arg(Strings::concat("TARGETDIR=", Command{to_path}.extract()));

RedirectedProcessLaunchSettings settings;
settings.encoding = Encoding::Utf16;
const auto maybe_code_and_output = cmd_execute_and_capture_output(cmd, settings);
if (auto code_and_output = maybe_code_and_output.get())
{
if (code_and_output->exit_code == 0)
Expand Down Expand Up @@ -112,7 +111,6 @@ namespace
.string_arg(fmt::format("-o{}", to_path))
.string_arg("-y")),
Tools::SEVEN_ZIP);

if (!maybe_output)
{
Checks::msg_exit_with_message(
Expand Down Expand Up @@ -198,9 +196,10 @@ namespace vcpkg

if (ext_type == ExtractionType::Zip)
{
const auto code =
cmd_execute(Command{"unzip"}.string_arg("-qqo").string_arg(archive), WorkingDirectory{to_path})
.value_or_exit(VCPKG_LINE_INFO);
ProcessLaunchSettings settings;
settings.working_directory = to_path;
const auto code = cmd_execute(Command{"unzip"}.string_arg("-qqo").string_arg(archive), settings)
.value_or_exit(VCPKG_LINE_INFO);
Checks::msg_check_exit(VCPKG_LINE_INFO,
code == 0,
msgPackageFailedtWhileExtracting,
Expand Down Expand Up @@ -304,8 +303,9 @@ namespace vcpkg

void extract_tar(const Path& tar_tool, const Path& archive, const Path& to_path)
{
const auto code =
cmd_execute(Command{tar_tool}.string_arg("xzf").string_arg(archive), WorkingDirectory{to_path});
ProcessLaunchSettings settings;
settings.working_directory = to_path;
const auto code = cmd_execute(Command{tar_tool}.string_arg("xzf").string_arg(archive), settings);
Checks::msg_check_exit(VCPKG_LINE_INFO,
succeeded(code),
msgPackageFailedtWhileExtracting,
Expand All @@ -316,9 +316,10 @@ namespace vcpkg
void extract_tar_cmake(const Path& cmake_tool, const Path& archive, const Path& to_path)
{
// Note that CMake's built in tar can extract more archive types than many system tars; e.g. 7z
const auto code =
cmd_execute(Command{cmake_tool}.string_arg("-E").string_arg("tar").string_arg("xzf").string_arg(archive),
WorkingDirectory{to_path});
ProcessLaunchSettings settings;
settings.working_directory = to_path;
const auto code = cmd_execute(
Command{cmake_tool}.string_arg("-E").string_arg("tar").string_arg("xzf").string_arg(archive), settings);
Checks::msg_check_exit(VCPKG_LINE_INFO,
succeeded(code),
msgPackageFailedtWhileExtracting,
Expand All @@ -344,12 +345,14 @@ namespace vcpkg
{
fs.remove(destination, VCPKG_LINE_INFO);
#if defined(_WIN32)
RedirectedProcessLaunchSettings settings;
settings.environment = get_clean_environment();
return flatten(cmd_execute_and_capture_output(
Command{seven_zip}.string_arg("a").string_arg(destination).string_arg(source / "*"),
default_working_directory,
get_clean_environment()),
Command{seven_zip}.string_arg("a").string_arg(destination).string_arg(source / "*")),
Tools::SEVEN_ZIP);
#else
RedirectedProcessLaunchSettings settings;
settings.working_directory = source;
return flatten(cmd_execute_and_capture_output(Command{"zip"}
.string_arg("--quiet")
.string_arg("-y")
Expand All @@ -358,7 +361,7 @@ namespace vcpkg
.string_arg("*")
.string_arg("--exclude")
.string_arg(".DS_Store"),
WorkingDirectory{source}),
settings),
"zip");
#endif
}
Expand Down Expand Up @@ -396,9 +399,9 @@ namespace vcpkg

std::vector<ExpectedL<Unit>> decompress_in_parallel(View<Command> jobs)
{
auto results =
cmd_execute_and_capture_output_parallel(jobs, default_working_directory, get_clean_environment());

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)
Expand Down
Loading