From fcd7d42b23c35616e46d7484321ad324c886fd5f Mon Sep 17 00:00:00 2001 From: Mura Li <2606021+typeless@users.noreply.github.com> Date: Sun, 20 Sep 2026 20:44:41 +0800 Subject: [PATCH 1/2] process: forward HOME to build commands, and pin the forwarded set out of identity putup's default child environment diverged from tup's default_env[] in two ways that no requirement stated, so a reader diffing the two found them as defects rather than decisions. A rule reading $HOME saw an empty string, and a PATH change re-ran nothing. HOME is now forwarded, under the set-non-empty rule #478 established for the temporary-directory variables: the value when putup's own environment holds a non-empty one, no entry at all when it is unset or empty, and no invented default, since none is correct for a per-user directory. The Windows keep list gains it too, so the requirement needs no platform scope; cmd and PowerShell leave HOME unset and contribute nothing. What its absence breaks was measured rather than assumed: `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 a putup command runs under on Debian-derived systems - leaves ~ unexpanded, so a rule containing it failed under putup and succeeded in the shell that invoked putup. The ccache motive the issue inherited turned out to be an inference: the upstream commit adding HOME records no reason and ccache falls back to getpwuid. Neither HOME nor PATH joins command identity. Recording them was argued and rejected: their values say where the invoking shell put things rather than what a command produces, which is the axis REQ-ENV-IMPORTED and REQ-ENV-TEMPDIR-IDENTITY already settled, and a recorded PATH would re-run every command in a project whenever direnv, nvm, an IDE terminal or a CI job composed PATH differently - putup's own 33-rule test runner first. The case a sticky PATH exists to catch, the compiler you will actually run having changed, is already reached by CONFIG_TRACKED_TOOLS: a bare tracked name is resolved through putup's PATH and its path:size:mtime enters identity, so a PATH change that shadows the tool re-runs its commands and one that does not re-runs nothing - a distinction a recorded PATH string cannot draw. That route was opt-in and unspecified; it is now a requirement, and its PATH branch has a scenario for the first time. It stays opt-in, so in a default configuration a PATH change that swaps the compiler still produces a stale build with no diagnostic; that gap is stated in the requirement rather than hidden, and is worth its own issue. Five requirements in command-record.ears.md's env-values group now carry this: REQ-ENV-PATH and REQ-ENV-HOME for what reaches the subprocess, REQ-ENV-TEMPDIR unchanged but no longer noting HOME as an open divergence, REQ-ENV-FORWARDED-IDENTITY replacing REQ-ENV-TEMPDIR-IDENTITY over the whole forwarded set, and REQ-ENV-TRACKED-TOOLS for the compensating route. Every discharge names a test that asserts its sentence. The two new unit tests and the HOME-forwarding scenario were observed red before the change; the three no-op pins and the PATH-default pin pass at HEAD, so each was shown able to fail - `export HOME` and `export PATH` in the fixture re-run the command, dropping CONFIG_TRACKED_TOOLS makes the shadowed-tool build a no-op, and a changed literal breaks the PATH default. A different-model review of the branch found the `/usr/bin:/bin` substitution stated without platform scope, though only the POSIX backend performs it - the Windows keep list copies a PATH that exists and synthesises none - and found the PATH unit test asserting nothing on Windows where its two siblings each carry an arm; both are corrected here. It also rejected two charges after measuring them: REQ-ENV-PATH stays tup-conformant, since the file already labels a requirement that way and records the narrow divergence in its reference (REQ-SIG-RECORD does exactly this), and a PATH change that preserves a tracked tool's resolution is already entailed by REQ-SIG-RECORD together with REQ-ENV-TRACKED-TOOLS rather than left unspecified. No record layout or content changes, so INDEX_VERSION is unchanged. Ref: https://github.com/typeless/putup/issues/480 Co-Authored-By: Claude Opus 5 (1M context) --- docs/reference.md | 23 ++-- include/pup/platform/process.hpp | 11 +- spec/requirements/command-record.ears.md | 47 ++++++- src/platform/process-posix.cpp | 2 +- src/platform/process-win32.cpp | 2 +- .../fixtures/home_forwarded/Tupfile.fixture | 1 + .../fixtures/path_forwarded/Tupfile.fixture | 1 + .../tracked_tool_path/Tupfile.fixture | 1 + test/unit/test_e2e.cpp | 130 ++++++++++++++++++ test/unit/test_platform_process.cpp | 117 ++++++++++++++++ 10 files changed, 314 insertions(+), 21 deletions(-) create mode 100644 test/e2e/fixtures/home_forwarded/Tupfile.fixture create mode 100644 test/e2e/fixtures/path_forwarded/Tupfile.fixture create mode 100644 test/e2e/fixtures/tracked_tool_path/Tupfile.fixture 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..46e4c604 100644 --- a/test/unit/test_platform_process.cpp +++ b/test/unit/test_platform_process.cpp @@ -309,3 +309,120 @@ 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") + { + auto path = EnvGuard { "PATH", "C:\\pup-probe\\bin" }; + auto env = base_child_env(); + + auto has_path = false; + for (auto var : env) { + has_path = has_path || sv(var).starts_with("PATH="); + } + 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 +} From c4cbe05685236f60d3b778bce4c8528129df3fb2 Mon Sep 17 00:00:00 2001 From: Mura Li <2606021+typeless@users.noreply.github.com> Date: Sun, 20 Sep 2026 21:08:14 +0800 Subject: [PATCH 2/2] test: match the Windows PATH entry by name rather than by uppercase prefix The Windows arm added for the PATH requirement asserted entry.starts_with("PATH="), but base_child_env copies each kept entry out of GetEnvironmentStringsW verbatim and Windows spells the variable Path, so the section failed on the windows-latest runner while every other job passed. Its two sibling sections match a name case-insensitively for exactly this reason; this one now does too. Wine is not a witness here: it spells the entry PATH=, so the original assertion passes under it and the defect is only observable on the CI runner. The pre-existing TMPDIR section, which asserts Temp and Tmp through the same case-insensitive comparison and passes there, is the evidence that the shape is right. Ref: https://github.com/typeless/putup/issues/480 Co-Authored-By: Claude Opus 5 (1M context) --- test/unit/test_platform_process.cpp | 16 ++++++++++++++-- 1 file changed, 14 insertions(+), 2 deletions(-) diff --git a/test/unit/test_platform_process.cpp b/test/unit/test_platform_process.cpp index 46e4c604..665c51de 100644 --- a/test/unit/test_platform_process.cpp +++ b/test/unit/test_platform_process.cpp @@ -313,14 +313,26 @@ TEST_CASE("base_child_env forwards the temporary-directory variables the tools r 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") + 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) { - has_path = has_path || sv(var).starts_with("PATH="); + 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); }