Skip to content

Commit

Permalink
[vcpkg] Rewriting CmdLineBuilder (2/n) (microsoft#15627)
Browse files Browse the repository at this point in the history 
* [vcpkg] Rewriting CmdLineBuilder (2/n)

I would like, and I think the team would like generally,
to switch to using stuff like `posix_spawn`, as opposed to the
existing use of `system` and `popen`.

This requires a pretty large change to how we use CmdLineBuilder.
The first change we have to make is that the execute functions
_cannot_ take a StringView anymore.
This PR makes that change.
  • Loading branch information
@strega-nil
strega-nil committed Jan 14, 2021
1 parent 5985c51 commit 41fb7fb
Show file tree
Hide file tree
Showing 24 changed files with 257 additions and 189 deletions.
4 changes: 2 additions & 2 deletions include/vcpkg/base/cache.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -4,7 +4,7 @@

namespace vcpkg
{
template<class Key, class Value>
template<class Key, class Value, class Less = std::less<Key>>
struct Cache
{
template<class F>
Expand All @@ -16,6 +16,6 @@ namespace vcpkg
}

private:
mutable std::map<Key, Value> m_cache;
mutable std::map<Key, Value, Less> m_cache;
};
}
42 changes: 25 additions & 17 deletions include/vcpkg/base/system.process.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,6 @@ namespace vcpkg::System
std::string s;
};

std::string make_basic_cmake_cmd(const fs::path& cmake_tool_path,
const fs::path& cmake_script,
const std::vector<CMakeVariable>& pass_variables);

struct CmdLineBuilder
{
CmdLineBuilder() = default;
Expand All @@ -46,15 +42,27 @@ namespace vcpkg::System
CmdLineBuilder&& raw_arg(StringView s) && { return std::move(raw_arg(s)); }

std::string&& extract() && { return std::move(buf); }
operator StringView() noexcept { return buf; }
StringView command_line() const { return buf; }

void clear() { buf.clear(); }
bool empty() const { return buf.empty(); }

private:
std::string buf;
};

struct CmdLineBuilderMapLess
{
bool operator()(const CmdLineBuilder& lhs, const CmdLineBuilder& rhs) const
{
return lhs.command_line() < rhs.command_line();
}
};

CmdLineBuilder make_basic_cmake_cmd(const fs::path& cmake_tool_path,
const fs::path& cmake_script,
const std::vector<CMakeVariable>& pass_variables);

fs::path get_exe_path_of_current_process();

struct ExitCodeAndOutput
Expand All @@ -79,48 +87,48 @@ namespace vcpkg::System
const fs::path& working_directory;
};

int cmd_execute(StringView cmd_line, InWorkingDirectory wd, const Environment& env = {});
inline int cmd_execute(StringView cmd_line, const Environment& env = {})
int cmd_execute(const CmdLineBuilder& cmd_line, InWorkingDirectory wd, const Environment& env = {});
inline int cmd_execute(const CmdLineBuilder& cmd_line, const Environment& env = {})
{
return cmd_execute(cmd_line, InWorkingDirectory{fs::path()}, env);
}

int cmd_execute_clean(StringView cmd_line, InWorkingDirectory wd);
inline int cmd_execute_clean(StringView cmd_line)
int cmd_execute_clean(const CmdLineBuilder& cmd_line, InWorkingDirectory wd);
inline int cmd_execute_clean(const CmdLineBuilder& cmd_line)
{
return cmd_execute_clean(cmd_line, InWorkingDirectory{fs::path()});
}

#if defined(_WIN32)
Environment cmd_execute_modify_env(StringView cmd_line, const Environment& env = {});
Environment cmd_execute_modify_env(const CmdLineBuilder& cmd_line, const Environment& env = {});

void cmd_execute_background(const StringView cmd_line);
void cmd_execute_background(const CmdLineBuilder& cmd_line);
#endif

ExitCodeAndOutput cmd_execute_and_capture_output(StringView cmd_line,
ExitCodeAndOutput cmd_execute_and_capture_output(const CmdLineBuilder& cmd_line,
InWorkingDirectory wd,
const Environment& env = {});
inline ExitCodeAndOutput cmd_execute_and_capture_output(StringView cmd_line, const Environment& env = {})
inline ExitCodeAndOutput cmd_execute_and_capture_output(const CmdLineBuilder& cmd_line, const Environment& env = {})
{
return cmd_execute_and_capture_output(cmd_line, InWorkingDirectory{fs::path()}, env);
}

int cmd_execute_and_stream_lines(StringView cmd_line,
int cmd_execute_and_stream_lines(const CmdLineBuilder& cmd_line,
InWorkingDirectory wd,
std::function<void(StringView)> per_line_cb,
const Environment& env = {});
inline int cmd_execute_and_stream_lines(StringView cmd_line,
inline int cmd_execute_and_stream_lines(const CmdLineBuilder& cmd_line,
std::function<void(StringView)> per_line_cb,
const Environment& env = {})
{
return cmd_execute_and_stream_lines(cmd_line, InWorkingDirectory{fs::path()}, std::move(per_line_cb), env);
}

int cmd_execute_and_stream_data(StringView cmd_line,
int cmd_execute_and_stream_data(const CmdLineBuilder& cmd_line,
InWorkingDirectory wd,
std::function<void(StringView)> data_cb,
const Environment& env = {});
inline int cmd_execute_and_stream_data(StringView cmd_line,
inline int cmd_execute_and_stream_data(const CmdLineBuilder& cmd_line,
std::function<void(StringView)> data_cb,
const Environment& env = {})
{
Expand Down
4 changes: 2 additions & 2 deletions include/vcpkg/build.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ namespace vcpkg::Build
const VcpkgPaths& m_paths;
};

std::string make_build_env_cmd(const PreBuildInfo& pre_build_info, const Toolset& toolset);
System::CmdLineBuilder make_build_env_cmd(const PreBuildInfo& pre_build_info, const Toolset& toolset);

struct ExtendedBuildResult
{
Expand Down Expand Up @@ -369,7 +369,7 @@ namespace vcpkg::Build
struct EnvMapEntry
{
std::unordered_map<std::string, std::string> env_map;
Cache<std::string, System::Environment> cmd_cache;
Cache<System::CmdLineBuilder, System::Environment, System::CmdLineBuilderMapLess> cmd_cache;
};

Cache<std::vector<std::string>, EnvMapEntry> envs;
Expand Down
6 changes: 3 additions & 3 deletions include/vcpkg/buildenvironment.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@

namespace vcpkg
{
std::string make_cmake_cmd(const VcpkgPaths& paths,
const fs::path& cmake_script,
std::vector<System::CMakeVariable>&& pass_variables);
System::CmdLineBuilder make_cmake_cmd(const VcpkgPaths& paths,
const fs::path& cmake_script,
std::vector<System::CMakeVariable>&& pass_variables);
}
33 changes: 21 additions & 12 deletions src/vcpkg/archives.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,13 +44,21 @@ namespace vcpkg::Archives
const std::string nugetid = match[1];
const std::string version = match[2];

const auto code_and_output = System::cmd_execute_and_capture_output(Strings::format(
R"("%s" install %s -Version %s -OutputDirectory "%s" -Source "%s" -nocache -DirectDownload -NonInteractive -ForceEnglishOutput -PackageSaveMode nuspec)",
fs::u8string(nuget_exe),
nugetid,
version,
fs::u8string(to_path_partial),
fs::u8string(paths.downloads)));
const auto code_and_output = System::cmd_execute_and_capture_output(System::CmdLineBuilder{nuget_exe}
.string_arg("install")
.string_arg(nugetid)
.string_arg("-Version")
.string_arg(version)
.string_arg("-OutputDirectory")
.path_arg(to_path_partial)
.string_arg("-Source")
.path_arg(paths.downloads)
.string_arg("-nocache")
.string_arg("-DirectDownload")
.string_arg("-NonInteractive")
.string_arg("-ForceEnglishOutput")
.string_arg("-PackageSaveMode")
.string_arg("nuspec"));

Checks::check_exit(VCPKG_LINE_INFO,
code_and_output.exit_code == 0,
Expand All @@ -65,11 +73,12 @@ namespace vcpkg::Archives
Checks::check_exit(VCPKG_LINE_INFO, !recursion_limiter_sevenzip);
recursion_limiter_sevenzip = true;
const auto seven_zip = paths.get_tool_exe(Tools::SEVEN_ZIP);
const auto code_and_output =
System::cmd_execute_and_capture_output(Strings::format(R"("%s" x "%s" -o"%s" -y)",
fs::u8string(seven_zip),
fs::u8string(archive),
fs::u8string(to_path_partial)));
const auto code_and_output = System::cmd_execute_and_capture_output(
System::CmdLineBuilder{seven_zip}
.string_arg("x")
.path_arg(archive)
.string_arg(Strings::format("-o%s", fs::u8string(to_path_partial)))
.string_arg("-y"));
Checks::check_exit(VCPKG_LINE_INFO,
code_and_output.exit_code == 0,
"7zip failed while extracting '%s' with message:\n%s",
Expand Down
81 changes: 45 additions & 36 deletions src/vcpkg/base/system.process.cpp
View file
Original file line number Diff line number Diff line change
Expand Up @@ -184,18 +184,17 @@ namespace vcpkg
}
System::CMakeVariable::CMakeVariable(std::string var) : s(std::move(var)) { }

std::string System::make_basic_cmake_cmd(const fs::path& cmake_tool_path,
const fs::path& cmake_script,
const std::vector<CMakeVariable>& pass_variables)
System::CmdLineBuilder System::make_basic_cmake_cmd(const fs::path& cmake_tool_path,
const fs::path& cmake_script,
const std::vector<CMakeVariable>& pass_variables)
{
System::CmdLineBuilder cmd;
cmd.path_arg(cmake_tool_path);
System::CmdLineBuilder cmd{cmake_tool_path};
for (auto&& var : pass_variables)
{
cmd.string_arg(var.s);
}
cmd.string_arg("-P").path_arg(cmake_script);
return std::move(cmd).extract();
return cmd;
}

System::CmdLineBuilder& System::CmdLineBuilder::string_arg(StringView s) &
Expand Down Expand Up @@ -384,7 +383,7 @@ namespace vcpkg
return clean_env;
}

int System::cmd_execute_clean(StringView cmd_line, InWorkingDirectory wd)
int System::cmd_execute_clean(const CmdLineBuilder& cmd_line, InWorkingDirectory wd)
{
return cmd_execute(cmd_line, wd, get_clean_environment());
}
Expand Down Expand Up @@ -565,12 +564,12 @@ namespace vcpkg
#endif

#if defined(_WIN32)
void System::cmd_execute_background(StringView cmd_line)
void System::cmd_execute_background(const CmdLineBuilder& cmd_line)
{
auto timer = Chrono::ElapsedTimer::create_started();

auto process_info =
windows_create_windowless_process(cmd_line,
windows_create_windowless_process(cmd_line.command_line(),
InWorkingDirectory{fs::path()},
{},
CREATE_NEW_CONSOLE | CREATE_NO_WINDOW | CREATE_BREAKAWAY_FROM_JOB);
Expand All @@ -582,48 +581,55 @@ namespace vcpkg
Debug::print("cmd_execute_background() took ", static_cast<int>(timer.microseconds()), " us\n");
}

Environment System::cmd_execute_modify_env(StringView cmd_line, const Environment& env)
Environment System::cmd_execute_modify_env(const CmdLineBuilder& cmd_line, const Environment& env)
{
static StringLiteral magic_string = "cdARN4xjKueKScMy9C6H";

auto actual_cmd_line = Strings::concat(cmd_line, " & echo ", magic_string, "& set");
auto actual_cmd_line = cmd_line;
actual_cmd_line.raw_arg(Strings::concat(" & echo ", magic_string, " & set"));

auto rc_output = cmd_execute_and_capture_output(actual_cmd_line, env);
Checks::check_exit(VCPKG_LINE_INFO, rc_output.exit_code == 0);
auto it = Strings::search(rc_output.output, Strings::concat(magic_string, "\r\n"));
const auto e = static_cast<const char*>(rc_output.output.data()) + rc_output.output.size();
Checks::check_exit(VCPKG_LINE_INFO, it != e);
it += magic_string.size() + 2;
Debug::print("command line: ", actual_cmd_line.command_line(), "\n");
Debug::print(rc_output.output, "\n");

auto it = Strings::search(rc_output.output, magic_string);
const char* const last = rc_output.output.data() + rc_output.output.size();

Checks::check_exit(VCPKG_LINE_INFO, it != last);
// find the first non-whitespace character after the magic string
it = std::find_if_not(it + magic_string.size(), last, ::isspace);
Checks::check_exit(VCPKG_LINE_INFO, it != last);

std::wstring out_env;

for (;;)
{
auto eq = std::find(it, e, '=');
if (eq == e) break;
StringView varname(it, eq);
auto nl = std::find(eq + 1, e, '\r');
if (nl == e) break;
StringView value(eq + 1, nl);

out_env.append(Strings::to_utf16(Strings::concat(varname, '=', value)));
auto equal_it = std::find(it, last, '=');
if (equal_it == last) break;
StringView variable_name(it, equal_it);
auto newline_it = std::find(equal_it + 1, last, '\r');
if (newline_it == last) break;
StringView value(equal_it + 1, newline_it);

out_env.append(Strings::to_utf16(Strings::concat(variable_name, '=', value)));
out_env.push_back(L'\0');

it = nl + 1;
if (it != e && *it == '\n') ++it;
it = newline_it + 1;
if (it != last && *it == '\n') ++it;
}

return {std::move(out_env)};
}
#endif

int System::cmd_execute(StringView cmd_line, System::InWorkingDirectory wd, const Environment& env)
int System::cmd_execute(const CmdLineBuilder& cmd_line, System::InWorkingDirectory wd, const Environment& env)
{
auto timer = Chrono::ElapsedTimer::create_started();
#if defined(_WIN32)
using vcpkg::g_ctrl_c_state;
g_ctrl_c_state.transition_to_spawn_process();
auto proc_info = windows_create_windowless_process(cmd_line, wd, env, 0);
auto proc_info = windows_create_windowless_process(cmd_line.command_line(), wd, env, 0);
auto long_exit_code = [&]() -> unsigned long {
if (auto p = proc_info.get())
return p->wait();
Expand All @@ -641,12 +647,15 @@ namespace vcpkg
std::string real_command_line;
if (wd.working_directory.empty())
{
real_command_line.assign(cmd_line.begin(), cmd_line.end());
real_command_line = cmd_line.command_line().to_string();
}
else
{
real_command_line =
System::CmdLineBuilder("cd").path_arg(wd.working_directory).raw_arg("&&").raw_arg(cmd_line).extract();
real_command_line = System::CmdLineBuilder("cd")
.path_arg(wd.working_directory)
.raw_arg("&&")
.raw_arg(cmd_line.command_line())
.extract();
}
Debug::print("system(", real_command_line, ")\n");
fflush(nullptr);
Expand All @@ -658,7 +667,7 @@ namespace vcpkg
return exit_code;
}

int System::cmd_execute_and_stream_lines(StringView cmd_line,
int System::cmd_execute_and_stream_lines(const CmdLineBuilder& cmd_line,
System::InWorkingDirectory wd,
std::function<void(StringView)> per_line_cb,
const Environment& env)
Expand Down Expand Up @@ -687,7 +696,7 @@ namespace vcpkg
return rc;
}

int System::cmd_execute_and_stream_data(StringView cmd_line,
int System::cmd_execute_and_stream_data(const CmdLineBuilder& cmd_line,
System::InWorkingDirectory wd,
std::function<void(StringView)> data_cb,
const Environment& env)
Expand All @@ -698,7 +707,7 @@ namespace vcpkg
using vcpkg::g_ctrl_c_state;

g_ctrl_c_state.transition_to_spawn_process();
auto maybe_proc_info = windows_create_process_redirect(cmd_line, wd, env, 0);
auto maybe_proc_info = windows_create_process_redirect(cmd_line.command_line(), wd, env, 0);
auto exit_code = [&]() -> unsigned long {
if (auto p = maybe_proc_info.get())
return p->wait_and_stream_output(data_cb);
Expand All @@ -711,14 +720,14 @@ namespace vcpkg
std::string actual_cmd_line;
if (wd.working_directory.empty())
{
actual_cmd_line = Strings::format(R"(%s 2>&1)", cmd_line);
actual_cmd_line = Strings::format(R"(%s 2>&1)", cmd_line.command_line());
}
else
{
actual_cmd_line = System::CmdLineBuilder("cd")
.path_arg(wd.working_directory)
.raw_arg("&&")
.raw_arg(cmd_line)
.raw_arg(cmd_line.command_line())
.raw_arg("2>&1")
.extract();
}
Expand Down Expand Up @@ -754,7 +763,7 @@ namespace vcpkg
return exit_code;
}

ExitCodeAndOutput System::cmd_execute_and_capture_output(StringView cmd_line,
ExitCodeAndOutput System::cmd_execute_and_capture_output(const CmdLineBuilder& cmd_line,
System::InWorkingDirectory wd,
const Environment& env)
{
Expand Down

0 comments on commit 41fb7fb

Please sign in to comment.