diff --git a/docs/reference.md b/docs/reference.md index a9e73edd..02af6f24 100644 --- a/docs/reference.md +++ b/docs/reference.md @@ -1159,10 +1159,16 @@ 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`, `$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 +invoking shell had, and changing that value alone re-runs nothing. `PATH` is the +exception, and only by resolution, never by value: putup resolves the first word +of every command — the first that is not a `NAME=value` assignment, and only when +it contains no `/` — and folds that binary's path, size and mtime into the +command's identity, so a `PATH` change that resolves `gcc` to a different +compiler re-runs the commands that name it, while one that moves nothing they +name re-runs nothing. A tool a command reaches only past a shell operator +(`cd build && gcc ...`), through `sh -c`, or through a wrapper script or compiler +driver is not the first word and is not covered — name it in +`CONFIG_TRACKED_TOOLS` (§6.1). `export` the variable if its value must decide whether a command is out of date. **`import`** - Import an environment variable into the Tupfile namespace: @@ -1287,7 +1293,10 @@ containing `/` against the source root) and its toolchain upgrade — same command text, same inputs, different compiler — then triggers a rebuild, closing the gap that command text alone cannot see. A missing tool records ``, so it appearing later also rebuilds. -Unset (the default) disables tracking. The fingerprint trusts size+mtime, +Unset (the default) tracks only each command's own first word (§ `export`); name +a tool here when a rule reaches it past a shell operator, through `sh -c`, or +through a wrapper or driver, or to catch an in-place upgrade of a tool no command +names directly. The fingerprint trusts size+mtime, the same trade-off as the stat cache (§9.1). **CLI Overrides:** diff --git a/spec/requirements/command-record.ears.md b/spec/requirements/command-record.ears.md index b87e6ad7..29cc06c4 100644 --- a/spec/requirements/command-record.ears.md +++ b/spec/requirements/command-record.ears.md @@ -764,14 +764,27 @@ empty. - 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 +- 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-COMMAND-TOOL in every project and by REQ-ENV-TRACKED-TOOLS where a project names more, each of which re-runs on a `PATH` change that resolves a 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. +by default, and neither a `CONFIG_TRACKED_TOOLS` entry nor a command's own first word resolves +through that variable, putup shall exclude its value from the identity of every command it +declares. + +### REQ-ENV-COMMAND-TOOL + +- leg: invariant +- conformance: deliberate-deviation +- reference: upstream reaches this case through `PATH` itself - `environ_add_defaults` (environ.c) makes `PATH` a sticky env node of every Tupfile's commands and `env_cb` (db.c) marks it modified whenever `getenv` disagrees with the stored value - so tup re-runs every command on any `PATH` change, the cost tup.1's `export` entry states; putup resolves the one name each command actually leads with and folds that binary's stat instead, so a `PATH` change that moves the tool re-runs the commands that name it and one that does not re-runs nothing, which is what REQ-ENV-FORWARDED-IDENTITY's exclusion of `PATH`'s value leaves uncovered; the first word is taken after any leading `NAME=value` words because the shell runs those as assignments rather than as the program, and a word containing `/` is skipped because `append_tool_stat` joins such a name to the source root while a command runs in its Tupfile's directory, so the two disagree; a tool a command reaches only past a shell operator, through `sh -c`, or through a wrapper or driver is not the first word and is not covered - `CONFIG_TRACKED_TOOLS` is the route for those, and on Windows every bare name records `` because `append_tool_stat` does not consult `PATHEXT` +- discharge: test "Scenario: A PATH change that swaps a tool no config tracks re-runs its commands" +- discharge: test "Scenario: Changing PATH alone re-runs no command" + +putup shall fold the path, size and modification time it resolves each command's first word to - +the first word that is not a `NAME=value` assignment, and only where that word contains no `/` - +into the identity of that command. ### REQ-ENV-TRACKED-TOOLS diff --git a/src/graph/builder.cpp b/src/graph/builder.cpp index f07b5ecc..b9a5e2c6 100644 --- a/src/graph/builder.cpp +++ b/src/graph/builder.cpp @@ -478,6 +478,10 @@ auto resolve_group_operand_node( return get_or_create_group_node(ctx, state, str(parsed->group_dir), parsed->group_name); } +auto ensure_env_var_node(BuilderContext& ctx, Builder& state, std::string_view var_name, std::string_view value) + -> std::optional; +auto append_tool_stat(Buf& out, std::string_view name, std::string_view source_root) -> void; + auto create_command_node( BuilderContext& ctx, Builder& state, @@ -507,6 +511,32 @@ auto create_command_node( (void)add_edge(ctx.state->graph, src_id, cmd_id, LinkType::Sticky); } + auto tool_words = tokenize_command(get(ctx.state->graph, cmd_id)); + auto tool = std::string_view {}; + for (auto word : tool_words.words()) { + if (word.find('=') != std::string_view::npos) { + continue; + } + tool = word; + break; + } + if (!tool.empty() && tool.find('/') == std::string_view::npos) { + auto key = Buf {}; + key += "TUP_TOOL_"; + key += tool; + auto const key_view = key.view(); + auto const* cached = state.imported_env_var_nodes.find(to_underlying(intern(key_view))); + auto tool_node = (cached != nullptr) ? std::optional { *cached } : std::nullopt; + if (!tool_node) { + auto stat = Buf {}; + append_tool_stat(stat, tool, str(state.options.source_root)); + tool_node = ensure_env_var_node(ctx, state, key_view, stat.view()); + } + if (tool_node) { + (void)add_edge(ctx.state->graph, *tool_node, cmd_id, LinkType::Sticky); + } + } + auto const* cv = ctx.used_config_vars.data(); for (std::size_t i = 0, n = ctx.used_config_vars.size(); i < n; ++i) { auto const* node_id = state.config_var_nodes.find(cv[i]); diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 464f312f..e3c17181 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -9430,6 +9430,47 @@ SCENARIO("A PATH change that shadows a tracked tool re-runs its commands", "[e2e } } +SCENARIO("A PATH change that swaps a tool no config tracks re-runs its commands", "[e2e][incremental][envdep]") +{ + GIVEN("a default-configured project whose rule names a tool 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); + + 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 rule's tool 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")