export or import of TUP_TOOLCHAIN or TUP_TOOL_<name> overwrites putup's own tool node and silently disables tool-change detection #484

Symptom

putup builds two kinds of synthetic Variable node whose names it reserves for itself — TUP_TOOLCHAIN (the CONFIG_TRACKED_TOOLS fingerprint) and, since #483, TUP_TOOL_<name> (each command's own resolved first word). Nothing stops a Tupfile from naming the same variable in export or import. When it does, ensure_env_var_node finds putup's node by name and overwrites its name and content_hash with the environment value (src/graph/builder.cpp:1310-1317), so the tool stat is gone for the whole build and the tool-change detection it exists for silently stops working.

The result is a stale build with no diagnostic — the same failure #482 was filed and #483 fixed, reachable again through a name.

Reproductions

Both on main 2987923bf, each an A/B against an identical project whose only difference is the export line. first/mytool prints v1, second/mytool prints v2.

1. export TUP_TOOL_<name> defeats #483's auto-seed. Tupfile:

export TUP_TOOL_mytool
: |> mytool > %o |> out.txt

$ PATH=$d/first:$PATH            putup -B .   →  Build completed: 1 commands    out.txt: v1
$ PATH=$d/second:$d/first:$PATH  putup -B .   →  Nothing to do (up to date).    out.txt: v1   # stale

Without the export line, byte-identical otherwise, the second build re-runs and out.txt is v2.

2. export TUP_TOOLCHAIN defeats CONFIG_TRACKED_TOOLS. This one predates #483. The rule's first word is sh, so #483's seed does not mask it:

export TUP_TOOLCHAIN
: |> sh -c 'mytool > %o' |> out.txt

with CONFIG_TRACKED_TOOLS=mytool in tup.config:

$ PATH=$d/first:$PATH            putup -B build   →  Build completed: 1 commands    out.txt: v1
$ PATH=$d/second:$d/first:$PATH  putup -B build   →  Nothing to do (up to date).    out.txt: v1   # stale

Without the export line: re-runs, out.txt is v2.

Mechanism

ensure_env_var_node (builder.cpp:1293) keys every node on the variable name in state.imported_env_var_nodes. The tool seed (builder.cpp:525-533), the toolchain fingerprint (builder.cpp:2517), process_export (builder.cpp:1398) and process_import (builder.cpp:1436) all go through it, sharing one namespace. A find-by-name hit is treated as "same variable, new value", which is right for a Tupfile variable and wrong for a node putup owns.

Note this is not reachable through $(TUP_TOOL_gcc) alone: lookup_var_with_bank returns VarBank::NotFound for an unknown regular name and on_env_var_used fires only for VarBank::Env (src/parser/eval.cpp:186, :249). export and import are the two routes.

Why it is worth closing rather than documenting

The names are not currently reserved anywhere a Tupfile author would look — docs/reference.md documents CONFIG_TRACKED_TOOLS without saying that TUP_TOOLCHAIN is spoken for, and #483 added a whole family of TUP_TOOL_* names with no note at all. A project that happens to export a matching name gets silently weaker change detection, and the failure mode is a wrong build rather than an error.

Decisions to make

• Whether putup's own nodes should live in a namespace no Tupfile can spell, rather than sharing imported_env_var_nodes with user variables — the "make the illegal state unrepresentable" answer, and the one that scales as more synthetic nodes are added. Needs a check on which characters a variable name admits at the lexer.
• Failing that, whether export/import of a reserved name should be a parse error naming the conflict, which is at least loud.
• Whether, either way, the reserved names belong in docs/reference.md and are pinned by a requirement — command-record.ears.md's env-values group now carries REQ-ENV-COMMAND-TOOL and REQ-ENV-TRACKED-TOOLS, and neither says the names are putup's.

Found while settling #482 (PR #483); raised there by the design review as a residual and left out of scope because half of it predates that change.