diff --git a/docs/reference.md b/docs/reference.md index 2748d692..a9e73edd 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -1142,9 +1142,13 @@ export PKG_CONFIG_PATH : foo.c |> $(CC) -c %f -o %o |> foo.o ``` -Commands run with a **minimal environment**: on POSIX `PATH`, plus `TMPDIR`, -`TMP` and `TEMP` when putup's own environment sets them to a non-empty value -(Windows passes `PATH` plus the system set: `SystemRoot`, `ComSpec`, `PATHEXT`, `TEMP`, `TMP`, `windir`). The +Commands run with a **minimal environment**: on POSIX `PATH`, plus `HOME`, +`TMPDIR`, `TMP` and `TEMP` when putup's own environment sets them to a non-empty +value (Windows passes `PATH` plus the system set: `SystemRoot`, `ComSpec`, `PATHEXT`, `TEMP`, `TMP`, `windir`, and +`HOME` when set). On POSIX an unset `PATH` is replaced by `/usr/bin:/bin`, since a +command with no `PATH` resolves no tool name at all. `HOME` is forwarded because tools +locate their caches and configuration under it — `go build` refuses to run +without it, and a `dash` `/bin/sh` leaves `~` unexpanded. The temporary-directory variables are forwarded because a command runs with its Tupfile's directory as cwd, and a tool that finds no writable temporary directory falls back to cwd, which puts its scratch files in the **source** @@ -1154,10 +1158,11 @@ variable is simply absent from the child environment. Only `export`ed variables are folded into command identity, so changing an exported variable's **value** re-runs the affected commands even when the command text is unchanged. The forwarded set above is **not** recorded: a -command whose text reads `$PATH` or `$TMPDIR` sees whatever the invoking shell -had, and changing that value alone re-runs nothing, except that a bare name in -`CONFIG_TRACKED_TOOLS` (§6.1) is resolved through `PATH`, so a `PATH` change -that resolves it to a different binary re-runs every command. `export` the +command whose text reads `$PATH`, `$HOME` or `$TMPDIR` sees whatever the +invoking shell had, and changing that value alone re-runs nothing, except that a +bare name in `CONFIG_TRACKED_TOOLS` (§6.1) is resolved through putup's own +`PATH`, so a `PATH` change that resolves it to a different binary re-runs every +command while one that does not re-runs nothing. `export` the variable if its value must decide whether a command is out of date. **`import`** - Import an environment variable into the Tupfile namespace: @@ -1276,8 +1281,8 @@ paths whose binaries the build's outputs depend on: CONFIG_TRACKED_TOOLS=gcc ld ./scripts/codegen.sh ``` -Each entry is resolved (bare names through `PATH` — the same `PATH` build -commands receive; entries containing `/` against the source root) and its +Each entry is resolved (bare names through putup's own `PATH`; entries +containing `/` against the source root) and its `(path, size, mtime)` is folded into every command's identity. An in-place toolchain upgrade — same command text, same inputs, different compiler — then triggers a rebuild, closing the gap that command text alone cannot see. diff --git a/include/pup/platform/process.hpp b/include/pup/platform/process.hpp index be2e8cd8..fc27907d 100644 --- a/include/pup/platform/process.hpp +++ b/include/pup/platform/process.hpp @@ -61,11 +61,12 @@ auto build_env_strings( ) -> Vec; /// Minimal "VAR=value" environment for build commands: the variables a child -/// needs to run tools at all, and the ones its toolchain searches to place -/// temporaries (POSIX: PATH, plus TMPDIR, TMP and TEMP when set; Windows: the -/// system set, whose TEMP and TMP serve that role). A variable here is -/// forwarded, not recorded: its value is absent from command identity. -/// Everything else must be passed via `export`. +/// needs to run tools at all, the per-user directory tools locate their caches +/// and configuration under, and the ones its toolchain searches to place +/// temporaries (POSIX: PATH, plus HOME, TMPDIR, TMP and TEMP when set; Windows: +/// the system set plus HOME when set, its TEMP and TMP serving that last role). +/// A variable here is forwarded, not recorded: its value is absent from command +/// identity. Everything else must be passed via `export`. [[nodiscard]] auto base_child_env() -> Vec; diff --git a/spec/requirements/command-record.ears.md b/spec/requirements/command-record.ears.md index c5714b0e..b87e6ad7 100644 --- a/spec/requirements/command-record.ears.md +++ b/spec/requirements/command-record.ears.md @@ -728,7 +728,7 @@ value it recorded for that variable rather than an empty one. - leg: invariant - conformance: deliberate-deviation -- reference: upstream's per-command default environment is `default_env[]` in environ.c (`PATH` and `HOME` on every platform; under `_WIN32` also `SYSTEMROOT`, `TEMP`, `TMP` and a set of Visual Studio variables), collected into each Tupfile by `environ_add_defaults` and emitted into the subprocess block by `tup_db_get_environ` (db.c), so on POSIX tup forwards no temporary-directory variable and a child falls back to `/tmp`; putup forwards the three GCC's `choose_tmpdir` (libiberty) reads ahead of `/tmp` and cwd because a putup command's cwd is its Tupfile's source directory, so with `/tmp` unwritable the fallback writes scratch files into the source tree (issue #478); putup's Windows system set already carries `TEMP` and `TMP`, which the platform always sets ahead of any cwd fallback, so `TMPDIR` is not added there; the same upstream list carries `HOME`, which putup still does not forward, unchanged by this requirement +- reference: upstream's per-command default environment is `default_env[]` in environ.c (`PATH` and `HOME` on every platform; under `_WIN32` also `SYSTEMROOT`, `TEMP`, `TMP` and a set of Visual Studio variables), collected into each Tupfile by `environ_add_defaults` and emitted into the subprocess block by `tup_db_get_environ` (db.c), so on POSIX tup forwards no temporary-directory variable and a child falls back to `/tmp`; putup forwards the three GCC's `choose_tmpdir` (libiberty) reads ahead of `/tmp` and cwd because a putup command's cwd is its Tupfile's source directory, so with `/tmp` unwritable the fallback writes scratch files into the source tree (issue #478); putup's Windows system set already carries `TEMP` and `TMP`, which the platform always sets ahead of any cwd fallback, so `TMPDIR` is not added there; the same upstream list carries `HOME`, which REQ-ENV-HOME forwards under the same set-non-empty rule - discharge: test "base_child_env forwards the temporary-directory variables the tools read" - discharge: test "Scenario: TMPDIR set for putup reaches every build command" @@ -737,12 +737,49 @@ POSIX; `TEMP` or `TMP` on Windows) is set to a non-empty value in putup's own en shall give every command's subprocess that variable with that value, and shall give it no such variable that is unset or empty. -### REQ-ENV-TEMPDIR-IDENTITY +### REQ-ENV-PATH + +- leg: invariant +- conformance: tup-conformant +- reference: upstream's `default_env[]` (environ.c) leads with `PATH` on every platform, `environ_add_defaults` makes it a sticky env node of every Tupfile's commands and `tup_db_get_environ` (db.c) emits it into the subprocess block from the value `tup_db_findenv` read with `getenv`, so a tup command resolves tool names through the `PATH` tup itself ran under; where tup emits nothing because the variable is unset, putup substitutes the default search path, since a command with no `PATH` resolves no tool name at all +- discharge: test "base_child_env always gives the child a PATH and substitutes a default when putup has none" +- discharge: test "Scenario: Changing PATH alone re-runs no command" + +putup shall give every command's subprocess `PATH` with the value putup's own environment sets, and +on POSIX shall give it `/usr/bin:/bin` when putup's own environment leaves `PATH` unset. + +### REQ-ENV-HOME - leg: invariant - conformance: deliberate-deviation -- reference: upstream's `default_env[]` (environ.c) carries `TEMP` and `TMP` only under `_WIN32` and never `TMPDIR`; there `environ_add_defaults` makes each a sticky env node and `tup_db_check_env` (db.c) marks it modified when `getenv` disagrees with the stored `VAR=value`, so on Windows tup re-runs every command when `TEMP` or `TMP` changes while putup's Windows system set forwards them unrecorded, and on POSIX upstream forwards none of the three so both agree that a change re-runs nothing; putup records only exported variables (REQ-ENV-SUBPROCESS), and a temporary-directory value says where scratch files live rather than what a command produces, so a sandbox that hands each session a fresh `TMPDIR` would otherwise re-run the whole build every session +- reference: upstream's `default_env[]` (environ.c) carries `HOME` on every platform - its `_WIN32` block appends to `PATH` and `HOME` rather than replacing them - so `environ_add_defaults` makes it a sticky env node of every Tupfile's commands, whose value `tup_db_findenv` (db.c) reads from `getenv`, `envdb_set` stores as `HOME=` for any non-NULL value including an empty one, and `tup_db_get_environ` emits whenever that stored value is non-NULL; the upstream commit adding it records no reason, and the ccache motive is an inference - ccache falls back to `getpwuid` when `HOME` is unset - while what does break was measured for issue #480: `go build` refuses to run at all ("GOCACHE is not defined and neither $XDG_CACHE_HOME nor $HOME are defined"), and dash, the `/bin/sh` putup's commands run under on Debian-derived systems, leaves `~` unexpanded so a rule containing it fails under putup and succeeds in the shell that invoked it; putup forwards the value only when it is non-empty, the shape REQ-ENV-TEMPDIR established for the forwarded-when-set variables, and supplies no default because none is correct for a per-user directory +- discharge: test "base_child_env forwards HOME when set and omits it when unset or empty" +- discharge: test "Scenario: HOME set for putup reaches every build command" + +putup shall give every command's subprocess `HOME` with the value putup's own environment sets when +that value is non-empty, and shall give it no `HOME` when putup's own environment leaves it unset or +empty. + +### REQ-ENV-FORWARDED-IDENTITY + +- leg: invariant +- conformance: deliberate-deviation +- reference: upstream forwards its default set because it records it - `environ_add_defaults` (environ.c, called per Tupfile from `parse` in parser.c) makes each `default_env[]` name a sticky env node of that Tupfile's commands, `tup_db_check_env` (db.c) marks the node modified when `getenv` disagrees with the stored `VAR=value` (`env_cb`, both-NULL matching), and `tup_db_get_environ` builds the subprocess block from those sticky entries alone plus `CCACHE_NODIRECT=1` - so tup re-runs every command when `PATH` or `HOME` changes, which tup.1's `export` entry states for `PATH` ("if PATH is changed, all commands will run again"), and on Windows also when `TEMP`, `TMP` or a Visual Studio variable changes; upstream forwards no `TMPDIR`, so on that one variable the two systems agree that a change re-runs nothing; `--no-environ-check` (updater.c) skips the comparison, but tup.1 gives the monitor's frozen environment as its purpose rather than the cost of the check; putup forwards the set unrecorded because these values say where the invoking shell put things rather than what a command produces, the axis REQ-ENV-IMPORTED already settled, and because a `PATH` that varies between one shell and the next would otherwise re-run every command in the project, putup's own test runner included; the case a recorded `PATH` exists to catch is reached instead by REQ-ENV-TRACKED-TOOLS, which re-runs on a `PATH` change that resolves a tracked name to a different binary and not on one that does not, a distinction a recorded `PATH` string cannot draw; a project that wants upstream's behaviour writes `export PATH`, which REQ-ENV-SUBPROCESS folds into identity - discharge: test "Scenario: Changing TMPDIR alone re-runs no command" +- discharge: test "Scenario: Changing HOME alone re-runs no command" +- discharge: test "Scenario: Changing PATH alone re-runs no command" + +While a Tupfile neither exports nor imports a variable putup forwards to every command's subprocess +by default, and no `CONFIG_TRACKED_TOOLS` entry resolves through that variable, putup shall exclude +its value from the identity of every command it declares. + +### REQ-ENV-TRACKED-TOOLS + +- leg: invariant +- conformance: putup-only +- discharge: test "Scenario: Tracked tool binaries fold into command identity" +- discharge: test "Scenario: A PATH change that shadows a tracked tool re-runs its commands" -While a Tupfile neither exports nor imports a temporary-directory variable putup forwards, putup -shall exclude that variable's value from the identity of every command it declares. +Where `CONFIG_TRACKED_TOOLS` names a tool, putup shall fold the path, size and modification time it +resolves that name to - a bare name through putup's own `PATH`, a name containing `/` against the +source root - into the identity of every command the project declares. diff --git a/src/platform/process-posix.cpp b/src/platform/process-posix.cpp index 202858d7..215a5402 100644 --- a/src/platform/process-posix.cpp +++ b/src/platform/process-posix.cpp @@ -60,7 +60,7 @@ auto base_child_env() -> Vec buf.append(std::string_view { "/usr/bin:/bin" }); } result.push_back(buf.intern(pool)); - static constexpr char const* forwarded_when_set[] = { "TMPDIR", "TMP", "TEMP" }; + static constexpr char const* forwarded_when_set[] = { "HOME", "TMPDIR", "TMP", "TEMP" }; for (auto const* name : forwarded_when_set) { auto const* value = sys::getenv(name); if (value == nullptr || *value == '\0') { diff --git a/src/platform/process-win32.cpp b/src/platform/process-win32.cpp index 1a1e88ce..7a87648d 100644 --- a/src/platform/process-win32.cpp +++ b/src/platform/process-win32.cpp @@ -53,7 +53,7 @@ auto base_child_env() -> Vec { // Case-insensitive names: Windows children need the system set to run at all. static constexpr std::wstring_view keep[] = { - L"SYSTEMROOT", L"COMSPEC", L"PATHEXT", L"PATH", L"TEMP", L"TMP", L"WINDIR" + L"SYSTEMROOT", L"COMSPEC", L"PATHEXT", L"PATH", L"HOME", L"TEMP", L"TMP", L"WINDIR" }; auto& pool = global_pool(); diff --git a/test/e2e/fixtures/home_forwarded/Tupfile.fixture b/test/e2e/fixtures/home_forwarded/Tupfile.fixture new file mode 100644 index 00000000..7db12359 --- /dev/null +++ b/test/e2e/fixtures/home_forwarded/Tupfile.fixture @@ -0,0 +1 @@ +: |> echo "HOME=$HOME" > %o |> out.txt diff --git a/test/e2e/fixtures/path_forwarded/Tupfile.fixture b/test/e2e/fixtures/path_forwarded/Tupfile.fixture new file mode 100644 index 00000000..38a71b0e --- /dev/null +++ b/test/e2e/fixtures/path_forwarded/Tupfile.fixture @@ -0,0 +1 @@ +: |> echo "PATH=$PATH" > %o |> out.txt diff --git a/test/e2e/fixtures/tracked_tool_path/Tupfile.fixture b/test/e2e/fixtures/tracked_tool_path/Tupfile.fixture new file mode 100644 index 00000000..1f2c8a38 --- /dev/null +++ b/test/e2e/fixtures/tracked_tool_path/Tupfile.fixture @@ -0,0 +1 @@ +: |> mytool > %o |> out.txt diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 3806c37c..464f312f 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -9271,6 +9271,94 @@ SCENARIO("Changing TMPDIR alone re-runs no command", "[e2e][envdep]") } } +SCENARIO("HOME set for putup reaches every build command", "[e2e][envdep]") +{ + GIVEN("a rule that prints the HOME its shell sees") + { + auto f = E2EFixture { "home_forwarded" }; + f.mkdir("home"); + auto home = (f.workdir() / "home").string(); + REQUIRE(f.init().success()); + + WHEN("putup runs with HOME set in its own environment") + { + auto env = EnvGuard { "HOME", home }; + auto result = f.build(); + + THEN("the command's environment carries the same value") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE(f.read_file("out.txt") == "HOME=" + home + "\n"); + } + } + } +} + +SCENARIO("Changing HOME alone re-runs no command", "[e2e][envdep]") +{ + GIVEN("a project built with one HOME by a rule whose text reads it") + { + auto f = E2EFixture { "home_forwarded" }; + f.mkdir("first"); + f.mkdir("second"); + auto first = (f.workdir() / "first").string(); + auto second = (f.workdir() / "second").string(); + REQUIRE(f.init().success()); + { + auto env = EnvGuard { "HOME", first }; + REQUIRE(f.build().success()); + REQUIRE(f.read_file("out.txt") == "HOME=" + first + "\n"); + } + + WHEN("putup runs again with a different HOME and nothing else changed") + { + auto env = EnvGuard { "HOME", second }; + auto result = f.build(); + + THEN("the build is a no-op and the output keeps the first value") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.is_noop()); + REQUIRE(f.read_file("out.txt") == "HOME=" + first + "\n"); + } + } + } +} + +SCENARIO("Changing PATH alone re-runs no command", "[e2e][envdep]") +{ + GIVEN("a project built with one PATH by a rule whose text reads it") + { + auto f = E2EFixture { "path_forwarded" }; + auto const* inherited = std::getenv("PATH"); + auto first = std::string { inherited != nullptr ? inherited : "/usr/bin:/bin" }; + auto second = first + ":" + (f.workdir() / "extra").string(); + REQUIRE(f.init().success()); + { + auto env = EnvGuard { "PATH", first }; + REQUIRE(f.build().success()); + REQUIRE(f.read_file("out.txt") == "PATH=" + first + "\n"); + } + + WHEN("putup runs again with a different PATH and nothing else changed") + { + auto env = EnvGuard { "PATH", second }; + auto result = f.build(); + + THEN("the build is a no-op and the output keeps the first value") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.is_noop()); + REQUIRE(f.read_file("out.txt") == "PATH=" + first + "\n"); + } + } + } +} + SCENARIO("Tracked tool binaries fold into command identity", "[e2e][incremental]") { GIVEN("a project whose config tracks a tool that is not a rule input") @@ -9300,6 +9388,48 @@ SCENARIO("Tracked tool binaries fold into command identity", "[e2e][incremental] } } +SCENARIO("A PATH change that shadows a tracked tool re-runs its commands", "[e2e][incremental][envdep]") +{ + GIVEN("a project whose config tracks a tool by a bare name found on PATH") + { + auto f = E2EFixture { "tracked_tool_path" }; + f.mkdir("build"); + f.mkdir("first"); + f.mkdir("second"); + f.write_file("first/mytool", "#!/bin/sh\necho v1\n"); + f.write_file("second/mytool", "#!/bin/sh\necho v2\n"); + REQUIRE(f.run("/bin/chmod", { "+x", "first/mytool", "second/mytool" }).exit_code == 0); + f.write_file("build/tup.config", "CONFIG_TRACKED_TOOLS=mytool\n"); + + auto const* inherited = std::getenv("PATH"); + auto base = std::string { inherited != nullptr ? inherited : "/usr/bin:/bin" }; + auto first = (f.workdir() / "first").string(); + auto second = (f.workdir() / "second").string(); + { + auto env = EnvGuard { "PATH", first + ":" + base }; + REQUIRE(f.pup({ "configure", "-B", "build" }).success()); + REQUIRE(f.build({ "-B", "build" }).success()); + REQUIRE(f.read_file("build/out.txt") == "v1\n"); + REQUIRE(f.build({ "-B", "build" }).is_noop()); + } + + WHEN("a PATH change resolves the tracked name to a different binary") + { + auto env = EnvGuard { "PATH", second + ":" + first + ":" + base }; + auto result = f.build({ "-B", "build" }); + + THEN("the command re-runs against the newly resolved tool") + { + INFO("stdout: " << result.stdout_output); + INFO("stderr: " << result.stderr_output); + REQUIRE(result.success()); + REQUIRE_FALSE(result.is_noop()); + REQUIRE(f.read_file("build/out.txt") == "v2\n"); + } + } + } +} + SCENARIO("Content change with preserved size and mtime", "[e2e][incremental]") { GIVEN("a built project whose input has an aged mtime") diff --git a/test/unit/test_platform_process.cpp b/test/unit/test_platform_process.cpp index 05c163b3..665c51de 100644 --- a/test/unit/test_platform_process.cpp +++ b/test/unit/test_platform_process.cpp @@ -309,3 +309,132 @@ TEST_CASE("base_child_env forwards the temporary-directory variables the tools r } #endif } + +TEST_CASE("base_child_env always gives the child a PATH and substitutes a default when putup has none", "[platform][process]") +{ +#ifdef _WIN32 + SECTION("the Windows keep list carries PATH under the name Windows spells it") + { + auto path = EnvGuard { "PATH", "C:\\pup-probe\\bin" }; + auto env = base_child_env(); + + auto has_path = false; + for (auto var : env) { + auto entry = sv(var); + if (entry.find('=') != 4) { + continue; + } + auto name = std::string_view { "PATH" }; + auto same = true; + for (std::size_t i = 0; i < name.size(); ++i) { + same = std::toupper(static_cast(entry[i])) == name[i]; + if (!same) { + break; + } + } + has_path = has_path || same; + } + REQUIRE(has_path); + } +#else + auto contains = [](pup::Vec const& env, std::string_view entry) { + for (auto var : env) { + if (sv(var) == entry) { + return true; + } + } + return false; + }; + + SECTION("the child gets putup's own PATH") + { + auto path = EnvGuard { "PATH", "/pup-probe/bin:/usr/bin" }; + auto env = base_child_env(); + + REQUIRE(contains(env, "PATH=/pup-probe/bin:/usr/bin")); + } + + SECTION("an unset PATH becomes the default search path rather than an empty one") + { + auto restore = EnvGuard { "PATH", "/pup-probe/bin" }; + pup::platform::unset_env("PATH"); + auto env = base_child_env(); + + REQUIRE(contains(env, "PATH=/usr/bin:/bin")); + } +#endif +} + +TEST_CASE("base_child_env forwards HOME when set and omits it when unset or empty", "[platform][process]") +{ + auto has_name = [](pup::Vec const& env, std::string_view name) { + for (auto var : env) { + auto entry = sv(var); + if (entry.find('=') != name.size()) { + continue; + } + auto same = true; + for (std::size_t i = 0; i < name.size(); ++i) { +#ifdef _WIN32 + same = std::toupper(static_cast(entry[i])) == name[i]; +#else + same = entry[i] == name[i]; +#endif + if (!same) { + break; + } + } + if (same) { + return true; + } + } + return false; + }; + +#ifdef _WIN32 + SECTION("the Windows keep list carries HOME") + { + auto home = EnvGuard { "HOME", "C:\\pup-probe\\home" }; + auto env = base_child_env(); + + REQUIRE(has_name(env, "HOME")); + } +#else + auto contains = [](pup::Vec const& env, std::string_view entry) { + for (auto var : env) { + if (sv(var) == entry) { + return true; + } + } + return false; + }; + + SECTION("HOME reaches the child when set") + { + auto home = EnvGuard { "HOME", "/pup-probe/home" }; + auto env = base_child_env(); + + REQUIRE(contains(env, "HOME=/pup-probe/home")); + REQUIRE(has_name(env, "PATH")); + } + + SECTION("an unset HOME is absent rather than empty") + { + auto restore = EnvGuard { "HOME", "/pup-probe/home" }; + pup::platform::unset_env("HOME"); + auto env = base_child_env(); + + REQUIRE_FALSE(has_name(env, "HOME")); + REQUIRE(has_name(env, "PATH")); + } + + SECTION("a set-but-empty HOME is absent rather than empty") + { + auto empty = EnvGuard { "HOME", "" }; + auto env = base_child_env(); + + REQUIRE_FALSE(has_name(env, "HOME")); + REQUIRE(has_name(env, "PATH")); + } +#endif +}