Skip to content

Fold env-driven conditionals into command identity#53

Merged
typeless merged 1 commit into
mainfrom
fix/condition-env-vars
May 28, 2026
Merged

Fold env-driven conditionals into command identity#53
typeless merged 1 commit into
mainfrom
fix/condition-env-vars

Conversation

@typeless

Copy link
Copy Markdown
Owner

Summary

Closes the residual hidden-input gap noted in PR #52: when TUP_PLATFORM (or any env var) is sourced from the environment and used inside a conditional like ifeq ($(TUP_PLATFORM),win32), flipping it between builds did not trigger a rebuild — the output stayed stale and the build reported "Nothing to do."

The root cause: an asymmetry the builder built up over time

The builder already folded config vars used in a guard into every guarded command's identity, via condition_config_varsSticky edge. The env-var path had no equivalent:

Var class Used in command text Used in a guard
Config (@(VAR)) used_config_vars → sticky condition_config_vars → sticky to every command in the block
Imported env ($(VAR) after import) used_env_vars → sticky ❌ — recorded but never reaches a sticky edge
TUP_PLATFORM/TUP_ARCH from env (auto-resolved, no import) ❌ — recorded in used_env_vars but no node exists for the sticky edge to point at ❌ — same hole, plus the missing node

When env TUP_PLATFORM flipped between builds, the phi-node's two branch commands sat in the graph unchanged (same text, same outputs, same identity), the output file on disk was unchanged, and change detection saw nothing to do.

What changed

Symmetric mirror of the config machinery, end to end:

  • condition_env_vars added to BuilderContext (include/pup/graph/builder.hpp), symmetric to condition_config_vars.
  • process_conditional captures used_env_vars from the condition's expansion and merges it into condition_env_vars under the same ScopeGuard that already handles nested conditionals.
  • create_command_node emits Sticky edges from those env Variable nodes to every command in the block — exact mirror of the existing config loop.

And to make sure those sticky edges land on real nodes:

  • ensure_env_var_node(ctx, state, name, value) extracted from process_import (its Variable-node creation/update block). process_import is refactored to delegate to it, with no behavioral change to its env → cached → default fallback.
  • on_env_var_used callback now calls the helper too, so TUP_PLATFORM/TUP_ARCH — auto-resolved from getenv without an explicit import — get the same tracking node any imported var would.

Effect on command identity

compute_command_identity (from PR #52) already folds every Sticky-linked Variable node, so no change is needed in dag.cpp. And no on-disk format change: the new sticky edges are persisted by serialize_edges automatically.

Test plan

  • Reproducer [platform-incremental] + fixture platform_conditional_incremental: both ifeq branches produce the same output file with distinct content; the platform string never appears in either command's text, so only the sticky edge can detect the flip. Build with TUP_PLATFORM=win32 (asserts WINBUILD, no-op stability), then re-bind to linux (asserts !is_noop() and NIXBUILD). Red before this change, green after.
  • Full suite green: 466 cases / 23,671 assertions.
  • Targeted regression coverage: [config] (the symmetric config path stays intact), [import] (env_var_persist, env_dep_subprocess, equals-in-value), [platform] (first-build platform_conditional), [idshift] and [envdep] (the regression guards from Identify commands by a structural hash, not a string or position #52).
  • clang-format, clang-include-cleaner, and scoped clang-tidy on changed files are clean for new code (the file's pre-existing std::move-on-trivially-copyable / qualified-auto style is unchanged).

Out of scope

  • TUP_PLATFORM/TUP_ARCH resolved from the compile-time default (Builtin bank) need no tracking — constant per putup binary.
  • No graph-wide refactor of implicit-dep representation; targeted at the env-var → condition asymmetry only.

🤖 Generated with Claude Code

Symmetry gap in the builder: a config var used in a guard ($(MODE) in an ifeq) had its value folded into every guarded command's identity via the condition_config_vars -> Sticky-edge path; an env var used the same way did not. So flipping env TUP_PLATFORM between builds left the phi-node's branch commands byte-identical, the output file unchanged on disk, and change detection reported "Nothing to do" with a stale output.

Close it by mirroring the existing config machinery, end to end. Add condition_env_vars to BuilderContext (symmetric to condition_config_vars). In process_conditional, capture used_env_vars from the condition's expansion and merge it into condition_env_vars under the same ScopeGuard that handles nested conditionals. In create_command_node, emit Sticky edges from those env Variable nodes to every command in the block.

Also close the upstream half so the sticky edges land on real nodes: extract process_import's node-creation block into ensure_env_var_node(ctx, state, name, value) and call it from the on_env_var_used callback so TUP_PLATFORM/TUP_ARCH (auto-resolved from the environment, not via an explicit import) get the same Variable node any imported var would. process_import is refactored to delegate node creation to the same helper with no behavioral change to its env->cached->default fallback dance.

compute_command_identity already folds every Sticky-linked Variable node, so no change is needed in dag.cpp and no on-disk format bump is required - the new sticky edges are persisted by serialize_edges automatically.

Reproducer: [platform-incremental] / fixture platform_conditional_incremental. Both ifeq branches produce the same output file with distinct content; the platform string never appears in either command's text, so only the sticky edge can detect the flip. Red before, green after. Full suite green (466 cases, 23671 assertions).

Co-Authored-By: Claude Opus 4.7 (1M context) <noreply@anthropic.com>
@typeless typeless merged commit fd48316 into main May 28, 2026
9 checks passed
@typeless typeless deleted the fix/condition-env-vars branch May 28, 2026 03:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant