Skip to content

Commit c97ed62

Browse files
committed
A build program's environment contract does not depend on the children mcpp has launched (#797)
On Windows a child's environment is applied to mcpp itself (`_putenv_s`), and `build_env_for_cl` composed the toolset's PATH from the process's current PATH. The value reached `MCPP_TOOL_ENV`, part of a build program's run key: a build that had compiled its programs through the build programs' graph ran ninja first, so its PATH carried the toolset's directory once more than the next build's, and every program ran again (e2e 89, 839, 842; mcpp-plugins cmake-consumer). The toolset's PATH and a program's PATH now start from the PATH the process was started with (`env::inherited_path`), captured before anything is launched.
1 parent fa9e185 commit c97ed62

3 files changed

Lines changed: 19 additions & 2 deletions

File tree

‎modules/platform/src/env.cppm‎

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -79,6 +79,11 @@ private:
7979
};
8080

8181
std::string path_list_separator();
82+
// The PATH this process was started with. On Windows a child's environment is
83+
// applied to this process (`_putenv_s`), so `getenv("PATH")` answers what the
84+
// children launched so far were given; a value that must not depend on that --
85+
// an environment a build program's run is keyed on -- starts from this one.
86+
const std::string& inherited_path();
8287
std::string runtime_library_path_key();
8388
std::string host_tool_runtime_library_path_key();
8489

@@ -114,6 +119,16 @@ std::string build_env_prefix(
114119

115120
namespace mcpp::platform::env {
116121

122+
namespace {
123+
// Initialised before main, ahead of anything that starts a child.
124+
const std::string kInheritedPath = [] {
125+
const char* p = std::getenv("PATH");
126+
return p ? std::string(p) : std::string{};
127+
}();
128+
} // namespace
129+
130+
const std::string& inherited_path() { return kInheritedPath; }
131+
117132
bool offline_mode() {
118133
// Not cached: `mcpp test` runs nested mcpp invocations in-process in some
119134
// paths, and a test that flips the var must see the change.

‎src/build/build_program.cppm‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -789,7 +789,7 @@ contract_env(const fs::path& root, const fs::path& outDir, const BuildProgramEnv
789789
// replaces a variable outright in the child, so writing only the
790790
// project's own directory would silently be the substitution this
791791
// deliberately is not.
792-
if (const char* inherited = std::getenv("PATH"); inherited && *inherited) {
792+
if (auto const& inherited = mcpp::platform::env::inherited_path(); !inherited.empty()) {
793793
path += mcpp::platform::env::path_list_separator();
794794
path += inherited;
795795
}

‎src/toolchain/msvc.cppm‎

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -1489,8 +1489,10 @@ std::vector<EnvVar> build_env_for_cl(const std::filesystem::path& clPath,
14891489
sdk.root / "Lib" / sdk.version / "ucrt" / a,
14901490
sdk.root / "Lib" / sdk.version / "um" / a,
14911491
})});
1492+
// The PATH mcpp was started with: the process's own has been given to the
1493+
// children launched so far, this value among them.
14921494
std::string path = clDir.string();
1493-
if (const char* p = std::getenv("PATH"); p && *p) {
1495+
if (auto const& p = mcpp::platform::env::inherited_path(); !p.empty()) {
14941496
path += ';';
14951497
path += p;
14961498
}

0 commit comments

Comments
 (0)