From fd5349c849bec865bf6d04b667e5a2a6f68347db Mon Sep 17 00:00:00 2001 From: Mura Li <2606021+typeless@users.noreply.github.com> Date: Sat, 19 Sep 2026 22:06:59 +0800 Subject: [PATCH] style: strip the pre-existing comments from the files #475 touched MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Every `//` comment that is not a `///` doc header, a license line or a NOLINT directive is removed from the seventeen C++ files PR #475 changed, together with the `} // namespace` closers and one `/* done */` parameter name. The change is comment-only: with comments and whitespace stripped, every file tokenizes identically to main 078387d5c. The other 168 files that still close namespaces with a marker are untouched; they follow in later rounds. The rule for this tree is that a non-obvious why lives in the commit body or DESIGN.md, never in a comment, so what the deleted comments said that the code cannot say is kept below, grouped by file and site, for anyone who reaches this commit through blame. Plain narration and restatements of the code were dropped without record. src/graph/builder.cpp: - resolve_input_node: Input paths are already source-relative from expand_inputs() which normalizes them by combining with current_dir. No further normalization needed here. - resolve_input_node: For variant builds, paths like "build/include/header.h" (from $(B)/include/header.h) already have the build root prefix. Strip it to get source-relative paths. - resolve_input_node: A seeded match names a file a later directory will generate; without this it would resolve to a source-side Ghost and the producer's output would become a second node. - resolve_input_node: With BUILD_ROOT_ID model: source files are under SOURCE_ROOT_ID (0) at source-relative paths; Generated/Ghost files are under BUILD_ROOT_ID at source-relative paths. - resolve_input_node: If path had build prefix, it's referencing a generated file. Even if a source file exists at the same path, create a Ghost node under BUILD_ROOT_ID so it can be upgraded to Generated when the output rule is processed. - resolve_input_node: In 3-tree builds, files may live in config_root (alongside Tupfiles) rather than source_root. Check config_root as a fallback for source file resolution. - (file scope, §4 header): Path resolution uses graph traversal instead of string manipulation (".." walks to the parent node, "name" finds/creates a child). This naturally unifies input and output path resolution because both traverse to the same node when the paths are equivalent (e.g. $(B)/include/header.h from an input and the variant- mapped output both resolve to the same node). - get_or_create_group_node: Group nodes are stored with angle-bracket name like "". - create_command_node: Sticky edges from condition env variables are symmetric to condition_config_vars — so an env-driven guard flip changes every guarded command's identity. - create_command_node: An exported var reaches the command's subprocess environment (read as bare $VAR) and so affects its output even when it never appears in the command text. Recording it as a sticky edge makes that dependency explicit and folds its value into the command identity, so a change to the var triggers a rebuild. (process_export guarantees every exported var has a value node.) - expand_glob_pattern: Unconditional: the generated half of the match set would otherwise depend on how far the parse fixpoint has progressed. - expand_glob_pattern: A file this round has not reached yet, but a previous round proved the project generates. Without it the match set is a function of where the parse has got to. - expand_glob_pattern: Sorting and deduping across two path spaces would order by where the build tree lives. - apply_exclusions: Matched against the merged list, not re-expanded against disk: generated matches are not on disk. - expand_outputs: Guards do not gate this, and an absolute path is rejected rather than re-rooted (#385). - (file scope, §8): Forward declaration for mutual recursion: include_single_file -> process_statement -> process_conditional -> process_statement. - include_single_file: A file joins each conditional context at most once: the same include in the same guard stack is redundant, but a later include under different guards is a distinct contribution (issue #91). - process_export: Per tup manual: "adds the environment variable VARIABLE to the export list for future :-rules". - process_export: An exported var reaches the command's subprocess environment, so its value is part of the command's identity even when the Tupfile never reads it. - process_import: Per tup manual: "sets a variable inside the Tupfile that has the value of the environment variable". - process_import: Need null-terminated string for getenv. - process_import: Resolution order is (1) environment; (2) if the author wrote `?= default`, that default is the explicit revert-target for the env-unset case and wins over the cache; (3) no env, no default — fall back to the cached value from the previous build so plain `import VAR` stays stable across shell sessions that forget to re-export the variable. - process_assignment: SortedIdVec moved-from state is empty, so no explicit clear() needed. - process_assignment: Capture the dependencies from RHS expansion. The value also depends on every condition guarding this assignment: reaching it required those config/env reads. - process_assignment: Config variables are read-only (loaded from tup.config), so only Regular and Node are writable. - process_conditional: Env vars are tracked symmetrically with config vars so that an env-driven guard flip (e.g. `ifeq ($(TUP_PLATFORM),win32)`) folds into the identity of every command in the conditional block. - process_conditional: Guards exist only for conditions that can flip without a Tupfile edit (config or env reads recorded): static conditionals are textual like tup. - process_conditional: Macro defs and assignments in an inactive branch serve only that branch's guarded rules; the active world after endif must not see them. - process_generated_rules: Order-only file edges are created from paths with group refs already filtered out upstream. - process_generated_rules: Edge from generated command to parent command exists because dep-scan runs before compile. - process_generated_rules: outputs intentionally left empty: generated rules are dep-scan commands whose output is captured via generated_output, not %o expansion. - expand_rule: Early macro lookup is needed to process the macro's order_only_inputs for demand-driven parsing. - expand_rule: Effective-field merge of rule + macro: command/display — macro wins (it IS the command template); outputs — macro fills in only if the rule has none; groups — rule wins, macro is fallback; order- only inputs — concatenated. - expand_rule: Extra outputs are unioned, not fallback: a macro's extra outputs are side effects its caller cannot restate (tup parse_bang_rule_internal). - expand_rule: Pre-resolve order-only group references so % can expand them in commands; this handles cross-directory groups like `| ../include/ |> cat %`. Stores known group names (sorted); the resolver constructs % on the fly. - expand_rule: Groups are first-class nodes; edges are created after all Tupfiles are parsed. - expand_rule: In tup, references are always order-only even when written in the inputs section. - expand_rule: resolve_order_only_group is overridden for this rule's command expansion so all % patterns are preserved literally for deferred resolution; the ScopeGuard restores it even on early returns. - expand_rule: Uses scanner_registry (modular approach) if available, falls back to pattern_registry. - expand_rule: An extra output is owned exactly like a primary, minus the %o operands and {group} (tup.1 extra-outputs). - expand_rule: Generated means "produced by a rule this configuration runs" (#386). - expand_rule: Multiple commands may produce the same output when they have mutually exclusive guards (same condition, opposite polarity) — the phi-node case. - expand_rule: Order-only edges from pre-expanded paths skip group references (deferred edge creation) and glob patterns (not valid paths). - process_rule: Pending weak assignments (??=) are applied before expanding commands so ??= assignments that precede rules take effect. - process_rule: Skip rules where the input pattern evaluated to empty (tup behavior): rule.inputs.empty() means no input pattern was specified (": |> cmd"), inputs->empty() means the pattern(s) evaluated to no files; only skip if a pattern was specified but produced nothing. - add_tupfile: In 3-tree builds config_root differs from source_root, but the directory structure mirrors the source tree, so current_dir relative to config_root is used for both config lookup and source file glob expansion. - add_tupfile: For 3-tree builds the Tupfile node is stored relative to config_root (the Tupfile's actual location). - add_tupfile: Config Variable nodes are created only once (first Tupfile); subsequent Tupfiles reuse existing nodes. - add_tupfile: The virtual $ directory for imported env vars mirrors tup's env_dt; initialized only once, subsequent Tupfiles reuse existing nodes. - add_tupfile: Toolchain fingerprint: CONFIG_TRACKED_TOOLS lists tools the build's outputs depend on that appear in no rule's inputs. Their resolved (path, size, mtime) fold into every command's identity, so an in-place tool upgrade rebuilds. - add_tupfile: The env-var-used callback also ensures an env Variable node exists for any env var that resolves from the environment, so TUP_PLATFORM/TUP_ARCH (auto-resolved, not via `import`) get the same tracking node as explicitly imported vars and fold into command identity. - add_tupfile: The resolve_order_only_group callback is for local group references (no directory prefix) and uses the current directory; groups are first-class nodes, looked up via graph edges (file -> group). - finalize_graph: Pass 1 accumulates members per (command, group_name) keyed by packed (command_id << 32 | interned_group_name); same-named groups from different directories contribute to the same replacement. - finalize_graph: The configure pass runs before tup.config exists, so rendered text does not yet distinguish anything and a key collision there means nothing; it schedules only the config-generating rules anyway. Last, because pass 2 rewrites command text. src/cli/cmd_build.cpp: - collect_scope_crossing_inputs: Derived from link_role rather than enumerated here, so a new link type joins this bypass by being classified once instead of by every walk remembering it — the omission that made a declared source input invisible to a scoped build (#200, #189). Ordering is excluded by the role, not by hand: order-only means existence, not content. - find_changed_files_with_implicit: A ghost recorded with no content is one whose file was absent when the record was written, not one nothing watches: its appearance is a change like any other (#386). - find_changed_files_with_implicit: Excluded dirs are not parsed this run, so nothing can consume their changes; observing them would record state we have no authority over. - find_changed_files_with_implicit: Past every skip above: this is the one place that decides the walk looked at a file, so it is the one place that can say so (#288). - find_changed_files_with_implicit: Overlay: parse-time inputs live under config_root but are indexed source-relative. - find_changed_files_with_implicit: No active rule produces this, so its *absence* is not a change; an appearance is, which is why this skip is reached only once the file is known to be gone. - find_changed_files_with_implicit: Accounted for already: deleted and routed (#213), or read absent by its reader (#281). - create_implicit_file: The run that reported this dep already saw it absent, so only its return is news (#281). - serialize_graph_nodes: Dropping a slot would shift every later load- derived id (id == position + 1). - serialize_graph_nodes: Overlay: parse-time inputs live under config_root but are indexed source-relative. - serialize_graph_nodes: Records that this build already routed the file's absence, so the next one does not read the same stat failure as news. The slot has to stay: id == position + 1. - serialize_graph_nodes: A ghost that exists on disk but is produced by no rule is a foreign input (e.g. the variant's tup.config): record its content so change detection can see it. - serialize_graph_nodes: An out-of-authority output arrives here as a ghost; the record keeps its owner (#369). - serialize_graph_nodes: Like every other file-shaped arm: one path, one entry -- a discovered dependency naming this path must join this entry rather than mint a second one. - serialize_graph_nodes: These node types must be in index to maintain consecutive ID sequence - process_implicit_deps: Only when nothing orders this command after the file. Ordering is transitive — a codegen emitting one declared and one discovered output orders its consumer through the declared one — so this asks reachability, not adjacency: enumerating path shapes is how the two previous attempts at this taxed correct builds. - preserve_old_implicit_edges: Command ids are positional and shift across builds (e.g. when an earlier-created command is removed), so the old edge's `to` id cannot be trusted to mean the same command; re- resolve each carried edge's command through the same join every other consumer uses. - preserve_old_implicit_edges: If the command is gone, drop the edge. If it survived and ran, the branch below drops it too: whatever that run reported is now the whole truth, empty included. - is_dir_authoritative: A dir under a pruned nested-project root has a Tupfile this run never saw; its absence from available_dirs does not mean it was deleted. - merge_out_of_scope_commands: Content that moved, not content that vanished (#247): the record stops claiming currency, not ownership. - merge_out_of_scope_commands: Sole witness for non-operand inputs (order-only, group, implicit, sticky). - merge_out_of_scope_commands: An extra output is owned by its edge alone, so the operand loop above cannot see it (#370). - merge_out_of_scope_commands: Marked, not dropped: dropping retracts which outputs this command owns along with the claim that they are current, and only the second is in doubt (#241). - merge_out_of_scope_commands: Omitted, not dropped (#243): an unresolvable operand is one this record cannot describe, but the outputs that did resolve are still owned by nothing else. - merge_out_of_scope_commands: An operand it could not carry means the record no longer describes what ran, so it keeps its outputs but stops claiming they are current. - merge_out_of_scope_commands: Implicit edges are carried by preserve_old_implicit_edges instead. - propagate_command_effect: Set before push_path decides: a caller that dedups an already-changed path has still found an output, and forcing the command as well would be wrong. - build_index: After the discovered deps, so a merge-created copy of an old entry cannot shadow the fresh one this build just stat'd — a carried NodeFlags::AbsenceRouted would then discharge a later real deletion as already routed, and the consumer would never run (#237). Still before preserve_old_implicit_edges, which re-attaches carried edges by identity and so must see the merged records; that, not the ordering against the deps, is the constraint. - reject_shadowed_sources: Only in-tree builds can destroy anything, now that no output may leave the build hierarchy (#385): out-of-tree, %o resolves under the build root, so a rule whose output path collides with a committed file writes beside it rather than over it. The collision is still confusing there -- the source becomes unreadable through that path -- but it is not data loss, and rejecting on it fails builds that cannot hurt anyone (a second build dir sees the first one's artifacts). - reject_shadowed_sources: What the previous build's record settles. `Known`: a path it recorded as a source File is a source whatever is on disk now, and a path it recorded while producing nothing at it is owned by nobody (#389); absence stays undecidable -- a generated file it does not mention is what a scoped build leaves behind for out-of- scope outputs -- which is why the test below is positive. `NeverBuilt`: nothing we produced can be on disk yet, so anything sitting at an output's path is a source. `Lost`: a build happened here and putup cannot tell either way, which is the only case that says so out loud. - reject_shadowed_sources: Guard-satisfied producers only, like every other command walk here: an inactive conditional branch declares outputs it will never write, and rejecting on those fails projects that build fine. - reject_shadowed_sources: The two-stage configure design has a rule produce the tup.config that the same build then reads as configuration. Match the whole basename: a suffix test also exempts anything merely ending in those characters, e.g. mytup.config. - reject_shadowed_sources: On disk in either input tree. Without this the printed remedy -- delete the file and try again -- would not clear the error, because the index still records it. - reject_unresolved_ghosts: Guard-satisfied consumers only, like every other command walk here (#386). - reject_unresolved_ghosts: -a only pulls in rules this build left out of scope; when nothing was left out, offering it sends the user round the same failure (#222). - detect_new_commands: A record that is not evidence outranks the signature: the command must run again even when nothing about it changed, and its outputs must be treated as changed so consumers of whatever it half-wrote, or never wrote, are rescheduled too. - reconcile_input_set: Every typed path, so "left the graph" below means gone, not merely not-a-source: a generated file the user deleted is still a graph node and must not read as removed. - reconcile_input_set: Explicit comparator: libc++ extern-templates std::__sort for unsigned int*, so the default form links against a libc++ we deliberately do not have. - remove_stale_outputs: Only delete outputs of a command whose directory we have authoritative knowledge of this run; anything else is preserved. - remove_stale_outputs: Staleness is per file, not per command: a rule that drops one of its outputs still joins through the ones it kept, so asking only "did this command survive" would leave the dropped file owned by nothing and never delete it. - remove_stale_outputs: Not graph::command_label: the command has left the graph, but the index keeps its operands, so a pattern shared by a foreach still names one command. - build_single_variant: Only scope parsing when explicit targets are given. CWD-derived scoping should still parse all Tupfiles so that out-of-scope Tupfile changes are detected for incremental builds. When -a is set, always parse all Tupfiles so that cross-directory producers are discovered and ghost nodes get resolved. - build_single_variant: build_context loads the old index as part of its work; that span is timed separately, so discount it here to keep the phases disjoint. - build_single_variant: No early exit on an empty graph: having nothing to run is not having nothing to clean up (#231). - build_single_variant: One line, not one per rule: a warning that fires on every rule of a green build teaches everyone to scroll past warnings. The per-object findings live in `parse`. - build_single_variant: A record too old for read_index still says which paths it recorded as sources; the version gate retracts its currency, not that (#291). Nothing recovered this way is ever loaded as `old_idx_ptr` -- it answers this one question and dies here. - build_single_variant: Carries "has not succeeded since it last failed": seeded from the previous index, cleared only by a successful run. A build in which the command does not run at all -- a target or scoped build -- must not forget it. - build_single_variant: What the comparison below looked at, and so the only files this build may restate (#288). - build_single_variant: Outlives the block only so the ordering can be derived after the up-to-date exit below. - build_single_variant: Build the identity -> NodeId map: the cross- build join key for commands. Must happen after parsing (operands set) but before incremental logic. - build_single_variant: Always include implicit deps (headers from .d files) for in-scope commands, even if the headers live outside the scoped directories. - build_single_variant: Before detection, not after: a deleted output is a change like any other, and a consumer reaching it order-only is only notified if detection sees it gone. - build_single_variant: A retired record is not "nothing to do": only the write below persists the retirement (#245). - build_single_variant: The command, not the display: "CC main.o" does not say which flags broke, and this is the only line a build prints of what actually ran. - build_single_variant: Recorded even when it discovered nothing: an empty report is a report, and treating it as silence carried dead edges forever (#224). - build_single_variant: Dropping a dep loses an edge no later build re- derives, so neither arm is gated on -v. - build_single_variant: Recorded, not ignored: tup drops out-of-tree deps unless --full-deps, putup tracks them (DESIGN.md). - build_single_variant: Nothing here will ever run a config rule, so a needing-to-run record on one is undischargeable. - build_single_variant: Past the up-to-date exit, not with the rest of the incremental work: this is one pair per recorded discovery, and a build that schedules nothing would derive all of them to run none. - build_single_variant: Same pairs the scheduler gets, but kept on contradiction: over-routing costs a run, not the build. - build_single_variant: A command this build means to run is unverified until it succeeds: an abort strands or kills it silently (#304). - build_single_variant: Nothing here will ever run a config rule, so a mark on one is undischargeable (as above). - build_single_variant: Ahead of the dry-run branch so that a scheduler which ever fails a dry job reports it. - build_single_variant: "Would run", matching clean's "Would remove": a dry run has completed nothing. - build_single_variant: Persisting a partial failure is safe because must_rerun records it, not because a failed command's outputs are missing. - build_single_variant: A build owes a record of what it ran: without one the next build cannot tell it happened. src/graph/dag.cpp: - make_graph: Reserve BUILD_ROOT_ID (1) for the build root node. All Generated/Ghost nodes will be parented under this node. The build root's filesystem location is determined at build time (source_root for in-tree, output_root for variant builds). - make_graph: Index 0 unused, index 1 = build root - make_graph: Name set by set_build_root_name() - ensure_file_node: In-tree builds share one physical tree: a BuildRoot- grounded path may already have a SourceRoot node (consumer parsed before producer sees the on-disk output as a source file). Alias instead of splitting. - ensure_file_node: Lazy resolution: ungrounded PathIds are grounded before creation. Try BuildRoot first (outputs are more common), then SourceRoot. - set_build_root_name: path_id stays PathId::BuildRoot (set in make_graph). The name is for display only (get_full_path), not for PathId identity. - compute_command_key: Command text is Tupfile-relative, so the same rule in sibling directories renders identically; without the directory those distinct rules share one key. - compute_command_key: A dep-scan command is output-less and drops its parent's -o, so two compiles of one source with equal flags render byte-identical scans; whose deps they inject is the only thing that tells them apart. One level: a parent is rule-authored, so has none. - compute_command_signature: An output the text never names -- every extra output, a %o-less primary -- would otherwise leave identity unchanged when the declaration changes; sorted for insertion independence. A dep-scan command's data-flow edge to its parent shares this walk and names no file. - compute_command_signature: Fold in (name, value-hash) of each Variable node reached via a Sticky edge. This captures vars that affect output without appearing in the rendered text — exported env vars the subprocess reads as $VAR, config vars gating an export, etc. Sorted and deduped by name so identity is independent of edge insertion order and tolerant of duplicate sticky edges (the graph permits them). - compute_affected_commands: A contradictory pair only adds a command here: the set only grows, so no cycle check. - compute_affected_commands: Joined after the cascade, not before: a forced command runs, but nothing about it says its consumers must, and seeding it upstream would say exactly that. - compute_affected_commands: InjectImplicitDeps siblings (dep-scan commands) have no graph outputs, so the cascade above can't reach them. They must run whenever their parent compile runs — otherwise newly-introduced transitive includes are never re-discovered, and the parent's run drops the recorded ones with nothing to re-report them (#228). Walk all commands and attach any whose parent_command was just marked affected. src/parser/eval.cpp: - lookup_var_with_bank: Context-computed special variables (highest priority) - NOT overridable - lookup_var_with_bank: TUP_PLATFORM and TUP_ARCH: env > config > context > default - lookup_var_with_bank: Fall back to config (tup behavior: CONFIG_* accessible via $()) - expand_var: Dependency tracking based on which bank was used. An @() reference is a config read even when the variable is undefined — defining it later must count as a change. - expand_var: TUP_PLATFORM/TUP_ARCH fall back to compiled-in defaults when neither env nor config provides them; setting the env var later must count as a change, so the fallback still records an env read. - expand(EvalContext&, Expression const&): Recursively expand any variable references that were embedded in literals (e.g., from escaped quotes like \"$(VAR)\") - expand_path: Split result by whitespace - variables may contain multiple files src/index/reader.cpp: - declared_layout_fits: Operand data is the only section whose size the header does not carry: it runs from its own offset to the string table. - open_index_in_window: Damage before version: neither a sub-header file nor foreign magic is a record of any version (#383). - open_index_in_window: What makes the bytes worth reading at all: that they are the ones a putup wrote, not a file that merely starts with the right four and lays its sections out plausibly (#294). - read_index: A field this read cannot reproduce faithfully makes the whole record unreadable rather than a weaker claim in its place: an empty name or operand set is something callers act on (#381). - prior_paths: Total by construction: a reader that decides whether to overwrite a file must not consume a projection that drops an entry type silently (#389). - prior_paths: Each entry feeds one list, so a path appearing twice -- in one list or across two -- is a record holding two entries for one path, the state its own writer forbids, and this is the last check available before an irreversible action (#382). Compared as stored: an out-of-tree record legitimately holds one file as "a/bar.txt" and "build/a/bar.txt", which stripping a prefix would collapse. - read_prior_paths: The file table and nothing else. Everything a version bump invalidates -- command identities, signatures, recorded currency -- stays unread, so it cannot reach a caller through this return type. - index_get_semantic_string: The string table's own end, not the file's: a string declared past the table but inside the file is still a string this record does not contain. - index_get_semantic_string: Offset 0 is the table's empty entry, so this is a value the record states, not a failure. - index_get_operands: Widened like every position here: in u32 this sum wraps, and the check below then passes (#372). The operand section's own end, like the string table's: the writer lays the string table directly after this section, so a record declared past it is one this record does not hold. src/index/writer.cpp: - serialize_index: Load derives ids from position, so a gap silently rewires every later edge. - serialize_index (add_display): Identity is key/signature, not this text (#360): mark and shorten rather than fail the record. - serialize_index (command_entries loop): v8: Use instruction_pattern instead of fully-expanded command. - serialize_index (push_u32): The 4GB guard below is the only bound on these, and only while each counted operand costs >=1 byte of this stream (#365). - serialize_index (edge sort): Canonical order: edge bytes must not depend on job-completion order (#120). - serialize_index (save_time_ns): Get current time for racy-clean detection. src/index/entry.cpp: - Index::compute_paths: File IDs are 1-based contiguous: files_[i].id == i + 1. Use direct indexing instead of a hash map. - Index::compute_paths: Resolve paths top-down (chain is bottom-up, so iterate in reverse) include/pup/graph/dag.hpp: - CommandNode::guards: Condition guards - command executes only if ALL guards are satisfied - CommandNode::guards: For nested conditionals, this accumulates all enclosing conditions - Graph::paths: mutable: memoized interning cache, written on the (const) read path. Single-threaded access only — not safe to share across threads. - BuildGraph::path_cache: mutable: memoization written by get_full_path on the (const) read path. Single-threaded access only — not safe to share across threads. include/pup/index/format.hpp: - RawCommandEntry (static_assert on sizeof(RawCommandEntry) == 88): Widening this record adds a category of recorded state, and a category is only useful once all three of its legs exist: written here, compared where staleness is decided, and routed so the commands that depend on it are scheduled. A category with a missing leg is a silent wrong build, which is the shape #189 catalogues — so the size is fixed deliberately, to stop a new field reaching the index before someone has answered for all three. include/pup/index/entry.hpp: - Index (save_time_ns_ field): Index save time (nanoseconds since epoch) for racy-clean detection test/unit/test_e2e.cpp: - is_noop accepts only a build that ran nothing: The assertion most incremental scenarios rest on, so it has to be exact: a substring test for "0 commands" also accepts every multiple of ten, and a quiescence check that can pass while a build ran hides the very defects those scenarios exist to catch (#234). - An unterminated caret is a parse error, not a shell error: Upstream rejects this at parse time; falling through left the ^ in the command text, so the rule died mid-build as "sh: ^: not found" — blamed on the tool, not the typo (#217). - An upstream caret flag is rejected, not rendered as a label: tup reads the non-space run after ^ as flags (t, o); putup implements neither, and printing "t" as the rule's label honours nothing and refuses nothing (#217). - A failing command is reported by its command line, not its display: The display names the step, not what broke, and this is the only line a build prints of what actually ran. - A bang macro's display wins over one written on the rule: Outputs and groups resolve rule-over-macro; display is the one field that goes the other way, because the macro owns the command the display names. - Bang macro order-only groups trigger demand-driven parsing: This test verifies that order-only group references embedded in bang macros correctly trigger demand-driven parsing of the directory containing the group. Bug: the group reference in !cc = | $(TOROOT)/include/ |> ... was not triggering parsing of include/Tupfile before looking up the group. - Bang macro order-only groups work in variant builds: This test verifies that order-only group references in bang macros work correctly in variant builds. The bug was that DEP (implicit dep scanning) commands were not inheriting the order-only edges from their parent compile commands when using bang macros with TOROOT-based group references like: !cc = | $(TOROOT)/include/ |> ... This caused DEP commands to run before headers were generated. - Group references in regular inputs expand correctly: This tests the spos pattern: $(ROOT)/modules/ |> cat % Group references are order-only even when in regular inputs section - A Tupfile edit that changes no command re-runs nothing: Complement of "Editing an output-less command re-runs it". The rule needs an output: the Sticky route propagates a command's outputs, so an output-less one cannot exhibit this at all (#225). - Touch does not trigger unnecessary rebuild: Use absolute path to touch since it's not in workdir - A build record that is not putup's own is refused out loud: Mid-file: past the header putup already validates, ahead of the footer, so nothing but the checksum can notice. - A build record that is not putup's own is refused out loud: With the record refused, nothing left says which files on disk this project produced, and #291's rule is that putup does not guess -- so the build stops and names them rather than treating its own outputs as checked- in sources. - A build records one entry per path: The directory walk creates an entry and registers it for the next lookup; register anything but what it created and the next chain re-creates it (#325). - A dependency outside the source tree is recorded rather than dropped: The arms that drop a discovered dep sit above an else that keeps the out-of-tree ones as absolute paths. Nothing pinned that, and #305 was filed on the assumption it drops them. - A changed header re-runs the output-less command that read it: Routing a discovered dep pushes the reading command's outputs, so a command with none was reached and then dropped. A compile gate is the shape that has no outputs on purpose (#228). - A scoped build sees a declared input outside the scope change: Detection skips out-of-scope files unless a bypass covers them, and the bypass admitted Implicit and Sticky edges but not Normal — so a plainly declared source input was the one kind of dependency a scoped build could not see (#200). - A command that raced its discovered dependency runs again: Nothing orders a consumer against a producer it only discovers, so it can read the file before it exists. The dep is then recorded from a post-run stat — as already satisfied — and no later build re-runs it, leaving the output permanently wrong (#274). - A consumer ordered through a sibling output is not taxed for discovering the other: Codegen emitting one declared and one discovered output: the consumer is ordered through the declared one, so it cannot have raced the discovered one. Marking it anyway doubles every codegen rebuild — the shape a two-hop ordering check cannot see (#274). - A discovered dependency orders its consumer on a later build: A discovery is index-only, so it orders nothing: every later build that runs both the producer and the consumer races them again, and the consumer is taxed with an extra run to catch up. The previous build's discovery is what the scheduler orders by now (#276). - A recorded discovery that the rules now contradict does not stall the build: The ordering carried from the last build is stale by construction: the rules can since have turned the discovered file's producer into a consumer of the discoverer's output. The rules are this build's truth, so the contradiction retracts the carried ordering — taking it as binding leaves both commands waiting for each other, and a scheduler with nothing runnable and nothing running reports the build complete having run neither (#276). - A discovery whose producing rule is gone orders nothing: Ordering carried from the last build names a producer by the file it produces, so a rule that has since stopped producing it names nothing and the consumer waits for no one (#276). - Ordering the scheduler did not enforce does not excuse a race: A carried ordering is only real for a pair this build actually scheduled: the consumer may be quiescent and absent from the job set, and then nothing enforced it. Crediting it anyway lets a command reach its own excuse through the missing one's outputs, and the race it did commit goes unmarked — the permanently wrong output #274 exists to prevent (#276). - A producer's own input change reaches the consumer that only discovered it: The affected cascade walks graph edges from the pre- build changed set, and a discovered dependency has none; the file is not known changed until its producer has run, and the index then stamps it from a post-run stat. So the consumer was never scheduled at all and its output stayed wrong while the build reported the tree up to date (#277). - A producer's input change reaches a chain of discovered consumers: Routing a discovered consumer makes its own outputs change, so whatever discovered those must follow. Expanding the recorded pairs once rather than inside the cascade's fixpoint would reach the first consumer and stop (#277). - A producer's input change reaches an output-less discovered consumer: The cascade marks the consumer command node itself, so a reader with no output path is reached the same way one with outputs is; routing through outputs instead would drop it, the shape #228 had on the comparison route (#284). - A producer's input change reaches an output-less discovered consumer: The gate's dep scan must see the header to record it; introduced together they race and the discovery is silently empty. - A discovered consumer re-runs for a producer that rewrites the same bytes: Membership routes, not content: the consumer re-runs because its producer ran, exactly as a declared consumer does. Pinned so the pessimism is a decision rather than a surprise, and so that it costs one run per producer run and not one per build (#277). - A dependency absent when it was recorded settles rather than re- running forever: A command may report reading a file that is not there -- a conditional include that resolved to nothing. Reading the same stat failure as news every build re-runs the command forever for output that cannot change, which is the loop the campaign exists to kill. tup records the absence as a ghost and settles, and re-runs only if the file appears (#281). - A dependency that was absent when recorded still re-runs its reader when it appears: What settling must not cost: the absence is recorded as zero size, zero mtime and a zero hash, and a file arriving has to be read as a change against all three (#281). - A dependency that was absent when recorded still re-runs its reader when it appears: Size and mtime match the sentinel zeros, so only the recorded hash separates it. - A deleted dependency a command still reports re-runs it once: The deletion is a real change and must reach the reader, but the run that follows records the file as absent -- so the build after it has nothing new to say and must settle (#281). - A recreated dependency does not carry its deletion mark forward: The merge copies an out-of-scope file's entry verbatim. Run before the discovered deps, that copy shadowed the fresh one, so a carried NodeFlags::AbsenceRouted discharged the next real deletion as "already routed" and the consumer never ran again (#237). - A recreated dependency does not carry its deletion mark forward: Nothing orders these two commands on a first build — b/ depends on a/p.txt only by discovery — so c.o's content here is whichever won, and asserting it raced on CI. gen.sh writes the .d either way, so the dependency is recorded regardless, which is all the steps below need. - A build whose discovered dependency was deleted quiesces: The command re-runs and rediscovers nothing, which the carry logic could not tell from "did not run", so the dead edge and its file entry came back every build (#224). - Implicit deps survive identical rules in sibling directories: Command text is Tupfile-relative, so these two rules render the same string. If identity ignores the directory they collide, and one directory's header edges get attached to the other's command. - A header the compile reads only under -O2 is tracked: A scan without the compile's flags resolves the other branch and records the wrong header. - A dep scan that prints anything but its rule fails the build: Chatter parsed as dependencies never stats, so the command would re-run for ever. - Implicit deps survive a flag whose path is a separate word: A flag's path reaches the scan whichever spelling carries it, or the scan has no input file. - Implicit deps cover every source of a multi-source command: gcc -M emits one rule per source; stopping at the first leaves b.h untracked and the program silently stale - Implicit deps survive command-id shift from a removed source: Implicit (header→command) edges discovered last build are carried forward for commands that don't rebuild. If that carry-forward keys on the command's array position (NodeId), removing a glob-matched source whose command was created earlier shifts every later command's id down — and the carried edge gets misattributed to whatever command now occupies the old id. A later edit to the header then rebuilds the wrong unit and leaves the real output stale. The carry-forward must key on the command's structural identity, stable across shifts. - Implicit deps survive command-id shift from a removed source: No Tupfile edit: the glob drops m_b.c. m_a.c is not recompiled this build, so its m_a.h dependency must be carried forward — at the new command id. - A build run from a subdirectory does not stamp an out-of-scope change as current: A cwd-derived scope parses the whole project but detects only its own directory, while the record leg re-hashed every graph file: a/src.txt was recorded current on the strength of a stat nothing consumed, so the next full build compared v2 against v2 forever (#288). - A build with --all-deps does not stamp an out-of-scope change as current: -a empties parse_scopes for the same reason cwd scoping does, so it reaches #288 by the same door: everything is parsed, only the scope is detected, everything is recorded. - A file added while building from a subdirectory is still built: The other side of #288's fix: carrying an unexamined file's recorded state forward must not become "record nothing", or a file first seen by a scoped build would never be built. - A stale output that cannot be deleted fails the build and keeps its record: Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. - clean does not count an empty directory it could not remove: The platform message already names the path; the caller must not repeat it. - A source and the out-of-tree output shadowing it are one record clean can read: The regression pin for comparing recorded paths as stored: this record holds one file as both a source and an output, and only their spellings tell them apart (#382). - A build refuses to overwrite a file the record does not attribute to a rule: Whether a previous build happened must not change the answer: the record is what the guard reads, and none of these three sequences gives it a claim on the file. - Turning a branch off keeps ownership of what it built: Ownership survives the branch going inactive only via the record's carry-forward (#369). - A stale output that cannot even be queried keeps its record: The rule lives in the readable root Tupfile so its directory stays authoritative; only the output's directory is locked, which is what reaches the exists() guard. - Source file content change triggers rebuild in variant build: scoped_build has: app -> lib cross-directory dep app/Tupfile is parsed first (alphabetically), references ../lib/foo.o This may create Ghost nodes, testing the ID contiguity fix - Source file content change triggers rebuild in variant build: Modify "42" to "99" - same size (2 chars), different content - Tupfile changes detected regardless of scope: A comment would not do: an edit leaving every command's text identical is correctly a no-op (#225), so the mutation has to change a command. - A file that cannot be hashed is named in a warning: The only signal a user gets when content hashing fails; it had no test at all (#204). - A file that cannot be hashed is named in a warning: The command must not read in.txt, or it fails before putup ever hashes it. - Order-only deps on generated outputs resolve correctly in variants: This tests the bug where order-only deps using plain paths (e.g., "include/foo.h") created duplicate File nodes instead of reusing Generated nodes. Pattern from busybox: : |> ... |> include/applets.h # output becomes build/include/applets.h : | include/applets.h |> ... # should resolve to build/include/applets.h Bug: the order-only dep created a File node at "include/applets.h" instead of reusing the Generated node at "build/include/applets.h". - Variant outputs are automatically mapped to build directory: This tests that output paths are automatically mapped to the variant directory. Pattern from busybox: Root Tupfile: : |> ... |> include/header.h # should become build/include/header.h src/Tupfile: | $(B)/include/header.h # references build/include/header.h Without automatic mapping, the output creates a node at "include/header.h" (source), but the dependency references "build/include/header.h" (variant) - creating two nodes. - Variant outputs are automatically mapped to build directory: If output was mapped incorrectly, there would be a ghost node at build/include/header.h that never gets satisfied - Cross-directory regular inputs work in variant builds: Similar to order-only test but with regular input dependency This tests that Ghost->Generated upgrade preserves edges aaa_consumer is parsed first (alphabetically), creates Ghost for ../zzz_producer/helper.c zzz_producer is parsed later, upgrades Ghost to Generated The edge from aaa_consumer's command to the generated file must be preserved - Self-host test via shell fixture: Regression guard: when a source's already-tracked header is edited to transitively include a new header, pup must record the new transitive header and rebuild on subsequent edits to it. Fixed by binding dep-scan command dirty-status to its parent compile (collect_affected_commands). - Subdir inherits from parent when no local config: NO build/sub/deep/tup.config - Root config used when no intermediate configs: NO build/sub/tup.config - should inherit from root - Empty subdir config does not block parent merge: Empty — parent vars merge through - Configure uses root tup.config only: NO build/configs/tup.config - Configure does not create .pup directory: Do NOT call init - no .pup directory exists - Configure handles config rule depending on non-config rule: Bug: passing only config-output commands as a filter ignores their dependencies. If a config rule depends on an intermediate file produced by a non-config rule, the dependency is not run, causing the config rule to fail. - A configure that cannot write tup.config does not report creating it: A file where the directory must go beats the exists() guard on tup.config itself, and needs no permission bits, so the scenario runs under root too. - Config selection persists across multiple builds: Use simple_c fixture which has no config-generating rules - (7 sites: permission/skip guards at former lines 3472, 3554, 3605, 3925, 4503, 8413, 8464): Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. - Scoped build must not delete out-of-scope outputs section: issue #122 - A build aborted before a command could run does not record it as done: Self-validating: the point of the scenario is that an abort leaves commands it meant to run un-run, so it must witness one before asserting what follows. - A build that cannot save its record does not report success: Renaming onto a directory fails for root too, so this needs no permission bits and no skip guard, unlike the scenarios that revoke write access to a directory. - A failed command's consumer runs once the command succeeds: The consumer's output already exists holding V1, so nothing schedules the consumer except propagation from the producer's re-run. Routing the retry through forced_cmds instead of changed_outputs leaves final.txt at V1 forever, which is what this pins. - index_shape: Determinism of construction, not of reload: two builds that never saw each other's work must record the same project. Answering this by reading the code is what #298's consult had to do. - Two builds of one tree record the same thing: A record whose sections are identical but whose lengths are not would mean the masking above is hiding a difference rather than excluding a timestamp; a shape that covers only the header would mean it is comparing almost nothing. - Two builds of one tree record the same thing: The weaker property the incremental suite leans on, asserted here because it costs one build: a run that does nothing must not rewrite what the record says either. - A damaged record says so instead of rebuilding in silence: Without this the shadow guard speaks first: an output on disk with no readable record is the #291 refusal, which would pass this scenario for the wrong reason. - A record too short to hold a header says so instead of rebuilding in silence: Below sizeof(RawHeader) + sizeof(RawFooter): above it the declared-layout row answers first and this would pin the wrong rejection. - A record too short to hold a header says so instead of rebuilding in silence: Without this the shadow guard speaks first, as in the layout scenario above. - Distcleaning keeps a record whose files it could not remove: The removal failure names the file on its own, so only the keep message witnesses that the record was kept because of it. - A glob's %f order does not depend on the build directory's name: Pins both halves and the canonical order: equality alone would still hold if the filesystem half stopped contributing entirely. - Out-of-tree, a generated file shadowing a source is one glob match and the source survives: Out-of-tree the output lands under the build root, so the committed file is shadowed rather than overwritten -- confusing, but not data loss, which is why #194's rejection is limited to in-tree builds. What #191 guarantees here is that the file is one match rather than two. - A glob consumer of a deleted stale output settles after the healing build: The heal in build 2 is #212 and is deliberately better than tup, which runs the consumer zero times; only the third build is the defect (#213). - A rule still naming a deleted stale output is rejected rather than re- run: The first rebuild is #212's routed heal; the defect is the build after it, which re-detects the file putup itself deleted (#213). - Removing a group member re-runs the commands that consume the group: The rule vanishes with its glob match, never by a Tupfile edit: an edit rebuilds a surviving member, and a rebuilt member reaches the consumer over the live graph, masking the removal (#169). - Removing a group member re-runs the commands that consume the group: Names the consuming command too, which an unannotated rule did not (#229). - Removing a group member re-runs the commands that consume the group: The removed foreach instance, not the pattern its sibling also matches. - Removing a group member schedules nothing when the group's only consumer is guarded off: Edges into a guard-unsatisfied command are dropped when the index is written, so the walk finding nothing here is the correct answer and not a missed route. - Env var change in a conditional rebuilds the affected branch: Both branches of `ifeq ($(TUP_PLATFORM),...)` produce the same output file with distinct content; the platform string does not appear in either command's text. So the only signal that an env-driven branch flip occurred is a sticky edge from the env Variable node to the guarded commands (the condition_env_vars path, symmetric to condition_config_vars). Without it, change detection sees no changed file and no changed identity, reports "Nothing to do", and the output stays stale. - Imported env vars persist across builds: MY_VAR not in environment (EnvGuard out of scope) - Exported env var consumed via subprocess environment triggers rebuild: The command consumes MY_GREETING through the inherited environment (bare $VAR in the shell), NOT via $(VAR) substitution. So the rendered command string is byte-identical across values: a string-keyed change detector cannot see the change. Correctness requires the command's identity to fold in the values of the vars it depends on (here, the exported MY_GREETING). - ?= soft assignment - = takes precedence: Use ?\?= in strings to avoid trigraph interpretation (??= -> #) - Cross-directory groups in 3-tree builds: Mirrors the GCC example pattern: root Tuprules.tup: S = $(TUP_CWD); LIB_DIR = gcc gcc/Tuprules.tup: S ?= $(TUP_CWD); LIB_DIR ?= .; macros use $(S)/$(LIB_DIR)/ gcc/Tupfile: produces , consumes via macros - Config tree inside source tree: main.c lives in source_dir, not source_dir/tupfiles, so a rule for its object exists only if the glob matched it there. - Cross-project order-only dependency resolution: This tests the bug where order-only deps using $(B) paths in 3-tree builds create Ghost nodes instead of resolving to existing Generated nodes. Pattern from busybox: root/Tupfile: : |> ... |> include/autoconf.h # output at build/include/autoconf.h applets/Tupfile: : main.c | $(B)/include/autoconf.h |> ... The bug manifests when: 1. source and output are in completely different filesystem trees 2. build_root_name has N levels of "../" (e.g., "../../../../tmp/build") 3. From a subdirectory, $(B) normalizes to N-1 levels of "../" because one "../" cancels with the subdirectory name 4. strip_build_prefix() fails to match the different prefix depths Example with busybox: source = /home/user/src/busybox output = /tmp/build build_root_name = ../../../../tmp/build (5 levels up from source) From applets/: $(B) = TUP_VARIANT_OUTPUTDIR/.. = ../../../../tmp/build/applets/.. = ../../../tmp/build (4 levels) strip_build_prefix("../../../tmp/build/include/x.h", "../../../../tmp/build") FAILS - prefixes don't match due to depth difference! - Cross-project order-only dependency resolution: Create asymmetric setup: source at 3 levels deep, output at 1 level source_root = workdir/a/b/c/source (3 dirs deep) output_root = workdir/out (1 dir deep) build_root_name = relative(out, a/b/c/source) = ../../../../out (4 ../) From consumer/: $(B) expands and normalizes to ../../../out (3 ../) The prefix mismatch causes strip_build_prefix to fail - Sibling directory inputs work with incremental variant builds: This tests the command string matching between graph and index. Pattern from spos: Tupfile at include/generated/ referencing ../data.txt Bug: Index uses std::filesystem::relative() while graph uses make_source_relative() These produce different paths for cross- directory references. - The object of a compile that runs elsewhere is reported instead of scanned wrongly: The scan runs from the Tupfile's directory, so a source word taken from an invocation that ran in sub/ resolves against a same-named file here -- deps recorded for a file the rule never compiled (#356). - The object of a compile that runs elsewhere is reported instead of scanned wrongly: One is standing next to it: a.o's compile is the covered prefix. The report's unit moved to the object, so its sentence has to speak about the object. - An object no scanned invocation writes is reported beside its scanned sibling: %o one directory down expands to '../../build/src/lib/a.o' -- the word resolves against the rule's own directory and back into the variant, which a root-level rule never shows. - A compile-shaped rule with no dependency scan is reported: The scan is declined correctly — putup cannot reproduce the prefix's shell state — but declining in silence leaves a rule whose headers are never recorded (#352). - A depfile flag the compile never carried hides no unscanned object: The suppression exists for a compile that writes its own depfile; read across the whole command text it was defeated by any word spelling one, including in a later invocation (#357). - Scoped build detects header changes outside scope: Reproducer for the real-world bug noted in CLAUDE.md: "Header-dep tracking can miss across variants — editing a widely-included SDK header may re-link with stale .o files and produce a binary newer than the header but functionally older." Differs from the single-scope test above in two ways: - multi-arg scoped invocation (mirrors `pup src/fubon ... ios` pattern) - multiple TUs in different scope dirs all transitively reach the same out-of-scope header, AND a downstream link rule consumes their .o files The strict assertion checks that ALL three .o files (alpha.o, beta.o, main.o) are present AND the binary's hash changes after the header bump, catching both the "noop" mode and the "rebuild some-but-not-all + relink with stale .o" mode of the bug. - Scoped build with multiple scopes detects out-of-scope header changes: Capture the linked binary's content so we can detect stale-link bugs even if pup claims "rebuilt". - Scoped build with multiple scopes detects out-of-scope header changes: Strict check: the linked binary must differ from the pre-edit version. If pup recompiled only some .o files and re-linked with stale others, the binary would be byte-identical or only partially updated — either way still buggy. - Scoped build with multiple scopes detects out-of-scope header changes: Same shape as above but with the build done out-of-tree under build/, invoked with -B. Variants share the source tree but have independent indices — a fix in one variant's index must not let the other re-link with stale .o. - Out-of-tree variant build picks up out-of-scope header changes: Probes the scoped-initial-build axis: when the FIRST build is already scoped (rather than full), is the index populated correctly enough that a subsequent header edit propagates through the linked binary? - Scoped rebuild after a SCOPED initial multi-scope build: Probes the repeated-scoped-invocation axis: full build, then a no-op scoped pass, then a real edit + scoped rebuild. Mirrors the production pattern where developers run the same scoped command many times across a session before the edit that exposes the bug. - 3-tree: group pattern %o must include build root prefix: Reproduces GCC BSP pattern where: 1. Library archive uses order-only group: % in command 2. Consumer links the library via $(B)/$(LIB_DIR)/libmath.a The group pattern forces has_group_pattern=true, so final_instruction becomes cmd_text (parse- time %o expansion). Output PathIds must be BuildRoot-grounded so materialize_path() prepends the build root prefix. Without grounding, %o becomes "libmath.a" (bare filename) instead of "../../build/zzz_lib/libmath.a". - 3-tree: group pattern %o must include build root prefix: The fixture's display is "AR %o", so the word after it is the rendered %o: a bare "libmath.a" is the bug, meaning the archive was written to the source tree. - 3-tree: group pattern %o must include build root prefix: Bounded to the line: %o is the last word on it, and the next line begins "[build]", which would satisfy the check below on its own. - An output declared by both conditional branches stays tracked: Scoped build must not delete out-of-scope outputs (issue #122) - An output declared by both conditional branches stays tracked: A two- directory project (alpha, beta) plus a root-level rule, fully built in build/. Each rule just copies its source, so outputs are trivially checkable. - A glob over generated files is path-ordered and stable across builds: The generated files exist on disk from here on, so a filesystem glob can now see what only the graph could see during the first build. - A continuation without a space before it builds and is scanned: Upstream tup rewrites `\`+newline to spaces, so the command runs as `gcc -c foo.c`; keeping the two bytes made the shell splice them into `-cfoo.c` and hid the compile from the dep scanner. - A group directory prefix after an extra outputs section names the group: An unresolved group only warns, so the silence is what witnesses the prefix moved it. test/unit/test_index.cpp: - `pup::graph::add_condition_node` forward declaration: Graph-module- internal (defined in dag.cpp, not in public header) - `TEST_CASE("CommandEntry conversion")`: Distinct values: a roundtrip that swapped the two fields must fail. - `TEST_CASE("Serialized edge section is independent of edge insertion order")`: Ties on from (edges 4->cmd1 / 4->cmd2) and on (from, to) (Implicit vs Sticky 4->cmd1) exercise every leg of the canonical-order comparator. - `TEST_CASE("Index ID contiguity requirement")`: This test documents a design constraint: IDs must be contiguous when stored in the index. The index format assigns IDs from array position on load (id = array_index + 1), so if there are gaps in stored IDs, parent_id references will be broken after round-trip. The build system ensures ID contiguity by storing ALL node types (including Ghost, Variable, Group) rather than skipping them. This test verifies the consequence of violating this constraint. - `TEST_CASE("A record whose declared layout does not fit the file is refused")`: Reading it as empty was the old behaviour: an out-of- bounds section became a zero-length span, so a damaged record loaded as the record of a project that had produced nothing -- and the source-file guard, which reads exactly that, went quiet and let a rule overwrite a checked-in file (#293). - `TEST_CASE("A file that does not carry the index magic is damage")`: No re-signing: the magic check runs before the checksum, so this is the arm under test. - static_asserts before `TEST_CASE("An operand offset that wraps makes the record unreadable")`: Both wrap tests discriminate only because the position they wrap onto holds something: a wrap landing on zeroes would be rejected for the wrong reason, not for the wrap the test is about. - `TEST_CASE("A name offset that wraps makes the record unreadable")`: Planted in this entry's `size`, which nothing reads back, so the wrapped read finds a name. - `TEST_CASE("A record whose entry carries a type this putup cannot name is unreadable")`: The recovery read builds the same entries, so it must not answer for this record either. - `TEST_CASE("A name offset that wraps makes the record unreadable")`: The record that names it is unreadable, so the recovery read must not answer for it either. - `TEST_CASE("A record whose edge carries a link type this putup cannot name is unreadable")`: Damage in a section the recovery read never looks at leaves the recorded paths readable. - `TEST_CASE("A record whose edge carries link type zero is unreadable")`: LinkType starts at 1, so zero is as unnameable as anything past the last enumerator. - `TEST_CASE("An empty recorded string is a value rather than a failure")`: env is semantics-bearing and empty here, which is exactly the value a failure must not mimic. - `TEST_CASE("A record names the paths it recorded even at a version too old to trust")`: The window is a handful of integers, so it is enumerated rather than sampled. - `TEST_CASE("A record that classifies one path two ways is unreadable")`: The three lists partition the file table, so one path in two of them proves the record holds two entries for it -- a state its own writer forbids (#382). - `TEST_CASE("A record that names one path twice with one type is unreadable")`: The same rule from inside a single list: two entries for one path, whichever list they land in. Crafted bytes are not bound by the writer's funnel (#382). - `TEST_CASE("A record that names one path once in each of two directories is readable")`: The positive control for the rejection above: same basename, different paths, which is what an out-of-tree build records for a source and the output that shadows it. - `TEST_CASE("One flipped bit anywhere in a record makes it unreadable")`: The mirror of the recovery-path property above, for the path every build takes: a record that is not the one putup wrote is not evidence, whichever door it is read through (#294). - `TEST_CASE("Keying a record's file table by path keeps every addressable entry and only those")`: Same entries, each once: an index may record two nodes at one path, and dropping either would make the map disagree with the table it is derived from. - `TEST_CASE("Keying the index under construction by path answers the same however it was filled")`: The walks that build this map also query it, so what it answers cannot depend on how far along the filling was — the insertion order is the graph's, not the caller's choice. test/unit/test_eval.cpp: - TEST_CASE("Evaluator pattern expansion"): For foreach rules, all_inputs has just one element (the current input) - TEST_CASE("Evaluator pattern expansion - multiple inputs"): For non- foreach rules, all_inputs has all input files - TEST_CASE("TUP_VARIANT_OUTPUTDIR expansion - no variant"): Without variant, TUP_VARIANT_OUTPUTDIR should be "." / This matches tup behavior where outputs go to current directory (Based on tup test t8108-variant-outputdir.sh) - TEST_CASE("TUP_VARIANT_OUTPUTDIR expansion - in-tree variant"): For in-tree variant build (e.g., build/ directory): From sub/dir Tupfile, TUP_VARIANT_OUTPUTDIR = ../../build/sub/dir; this is the relative path from source sub/dir to output build/sub/dir - TEST_CASE("TUP_VARIANTDIR vs TUP_VARIANT_OUTPUTDIR"): TUP_VARIANTDIR is the relative path to the variant's version of the *included file's* directory; TUP_VARIANT_OUTPUTDIR is the relative path to the variant's version of the *current Tupfile's* directory. From tup test t8108: TUP_CWD = ../../rules (relative to included file), TUP_VARIANTDIR = ../../build/rules (variant's rules directory), TUP_VARIANT_OUTPUTDIR = ../../build/sub/dir (variant's output for this Tupfile) Ref: https://github.com/typeless/putup/pull/475 Co-Authored-By: Claude Fable 5.1 --- include/pup/core/instruction.hpp | 2 +- include/pup/graph/dag.hpp | 17 +- include/pup/index/entry.hpp | 4 +- include/pup/index/format.hpp | 7 +- include/pup/parser/eval.hpp | 6 +- src/cli/cmd_build.cpp | 162 +--------- src/core/instruction.cpp | 4 +- src/graph/builder.cpp | 259 +--------------- src/graph/dag.cpp | 64 +--- src/index/entry.cpp | 11 +- src/index/reader.cpp | 37 +-- src/index/writer.cpp | 32 +- src/parser/eval.cpp | 58 +--- test/unit/test_e2e.cpp | 516 +------------------------------ test/unit/test_eval.cpp | 32 +- test/unit/test_index.cpp | 130 +------- test/unit/test_instruction.cpp | 2 +- 17 files changed, 80 insertions(+), 1263 deletions(-) diff --git a/include/pup/core/instruction.hpp b/include/pup/core/instruction.hpp index f4cbd3ae..4aaeb987 100644 --- a/include/pup/core/instruction.hpp +++ b/include/pup/core/instruction.hpp @@ -163,4 +163,4 @@ auto fold_instruction(Instruction const& atoms, Site const& site) -> StringId return buf.intern(global_pool()); } -} // namespace pup +} diff --git a/include/pup/graph/dag.hpp b/include/pup/graph/dag.hpp index fded8a15..398a2e34 100644 --- a/include/pup/graph/dag.hpp +++ b/include/pup/graph/dag.hpp @@ -75,13 +75,10 @@ struct CommandNode { SortedIdVec exported_vars = {}; ///< Env vars to export to command (interned StringIds) - // For generated rules (auto-generated from pattern matching) std::optional generated_output = {}; ///< Output specification OutputAction output_action = {}; ///< What to do with output NodeId parent_command = INVALID_NODE_ID; ///< Parent command for InjectImplicitDeps - // Condition guards - command executes only if ALL guards are satisfied - // For nested conditionals, this accumulates all enclosing conditions Vec guards = {}; }; @@ -118,12 +115,8 @@ struct Graph { NodeIdArenaIndex edges_to_index; NodeIdArenaIndex edges_from_index; - // Node lookup indices Vec dir_children; ///< Per-directory name→NodeId index (indexed by parent dir) - // Structured path algebra - // mutable: memoized interning cache, written on the (const) read path. - // Single-threaded access only — not safe to share across threads. mutable PathPool paths; ///< Interning trie of (parent PathId, basename StringId) entries SortedPairVec path_to_node; ///< Resolve: PathId → NodeId (reverse of FileNode::path_id) @@ -343,7 +336,7 @@ inline constexpr auto parsed_consumers = static_cast(consumers & ~ /// The half only the index carries: a discovered dependency is never parsed from a Tupfile, /// so its consumer has no graph edge to cascade over and must be routed on every change. inline constexpr auto discovered_consumers = link_type_bit(LinkType::Implicit); -} // namespace edge_mask +} /// Visit neighbor ids by direction and type mask without materializing a Vec. /// The allocation-free form for hot traversals (topo sort, reachability). @@ -491,16 +484,10 @@ auto set_build_root_name(Graph& graph, std::string_view name) -> void; [[nodiscard]] auto get_build_root_name(Graph const& graph) -> std::string_view; -// ============================================================================= -// BuildGraph - thin data carrier (replaces BuildGraph over time) -// ============================================================================= - /// Simple aggregate holding the graph and its path cache. /// Replaces BuildGraph as a thin data carrier with no methods. struct BuildGraph { Graph graph; - // mutable: memoization written by get_full_path on the (const) read path. - // Single-threaded access only — not safe to share across threads. mutable PathCache path_cache; }; @@ -544,4 +531,4 @@ auto collect_upstream_files( /// Set build root name and clear path cache auto set_build_root_name(BuildGraph& state, std::string_view name) -> void; -} // namespace pup::graph +} diff --git a/include/pup/index/entry.hpp b/include/pup/index/entry.hpp index f00466ce..671b2457 100644 --- a/include/pup/index/entry.hpp +++ b/include/pup/index/entry.hpp @@ -222,12 +222,10 @@ class Index final { Vec commands_ = {}; Vec edges_ = {}; - // Edge indices (indices into edges_ vector) Arena32 edge_arena_; NodeIdArenaIndex edges_from_index_; NodeIdArenaIndex edges_to_index_; - // Index save time (nanoseconds since epoch) for racy-clean detection std::int64_t save_time_ns_ = 0; [[nodiscard]] @@ -263,4 +261,4 @@ auto files_by_path(Index const& index) -> FilesByPath; [[nodiscard]] auto get_command_string(Index const& index, CommandEntry const& cmd) -> StringId; -} // namespace pup::index +} diff --git a/include/pup/index/format.hpp b/include/pup/index/format.hpp index d5489297..6fd88fe6 100644 --- a/include/pup/index/format.hpp +++ b/include/pup/index/format.hpp @@ -210,11 +210,6 @@ static_assert( "a flag's value is on-disk format, not implementation: changing one needs an INDEX_VERSION bump" ); -// Widening this record adds a category of recorded state, and a category is only useful once all -// three of its legs exist: written here, compared where staleness is decided, and routed so the -// commands that depend on it are scheduled. A category with a missing leg is a silent wrong build, -// which is the shape #189 catalogues — so the size is fixed deliberately, to stop a new field -// reaching the index before someone has answered for all three. static_assert( sizeof(RawCommandEntry) == 88, "RawCommandEntry changed size: bump INDEX_VERSION, and wire the new state's three legs " @@ -358,4 +353,4 @@ inline auto set_node_flags(RawFileEntry& entry, NodeFlags flags) -> void entry.flags_high = static_cast(value >> 8); } -} // namespace pup::index +} diff --git a/include/pup/parser/eval.hpp b/include/pup/parser/eval.hpp index 09cdee17..415059ff 100644 --- a/include/pup/parser/eval.hpp +++ b/include/pup/parser/eval.hpp @@ -268,7 +268,7 @@ constexpr auto TUP_VARIANTDIR = "TUP_VARIANTDIR"; constexpr auto TUP_VARIANT_OUTPUTDIR = "TUP_VARIANT_OUTPUTDIR"; constexpr auto TUP_SRCDIR = "TUP_SRCDIR"; constexpr auto TUP_OUTDIR = "TUP_OUTDIR"; -constexpr auto CONFIG_ = "CONFIG_"; // Prefix for @() variables -} // namespace builtin_vars +constexpr auto CONFIG_ = "CONFIG_"; +} -} // namespace pup::parser +} diff --git a/src/cli/cmd_build.cpp b/src/cli/cmd_build.cpp index f89546f4..6dc21df4 100644 --- a/src/cli/cmd_build.cpp +++ b/src/cli/cmd_build.cpp @@ -197,7 +197,6 @@ auto collect_scope_crossing_inputs( auto& pool = pup::global_pool(); auto result = Vec {}; - // Find commands whose directory is in scope auto in_scope_cmds = pup::NodeIdMap32 {}; for (auto const& cmd : index.commands()) { auto const* dir_file = index.find_file_by_id(cmd.dir_id); @@ -210,10 +209,6 @@ auto collect_scope_crossing_inputs( } } - // Derived from link_role rather than enumerated here, so a new link type joins this - // bypass by being classified once instead of by every walk remembering it — the omission - // that made a declared source input invisible to a scoped build (#200, #189). Ordering - // is excluded by the role, not by hand: order-only means existence, not content. for (auto const& edge : index.edges()) { if (!pup::graph::in_mask(edge.type, pup::graph::edge_mask::inputs)) { continue; @@ -294,13 +289,10 @@ auto find_changed_files_with_implicit( auto& changed = scan.changed; auto& metrics = pup::thread_metrics(); - // Racy-clean threshold: files modified within 1 second of index save auto const save_time_ns = old_index.save_time_ns(); auto constexpr RACY_CLEAN_THRESHOLD_NS = std::int64_t { 1'000'000'000 }; for (auto const& file : old_index.files()) { - // A ghost recorded with no content is one whose file was absent when the record was - // written, not one nothing watches: its appearance is a change like any other (#386). auto const is_ghost = file.type == pup::NodeType::Ghost && !pup::is_empty(file.path); auto const was_absent = is_ghost && file.content_hash == pup::ZERO_HASH; if (file.type != pup::NodeType::File && file.type != pup::NodeType::Generated && !is_ghost) { @@ -310,13 +302,10 @@ auto find_changed_files_with_implicit( auto& pool = pup::global_pool(); auto file_path_sv = pool.get(file.path); - // Generated paths carry the build-root prefix; scopes are source-relative auto scope_path_sv = file.type == pup::NodeType::Generated ? pool.get(pup::strip_path_prefix(file_path_sv, build_root)) : file_path_sv; - // Skip files outside scopes (but always check Tupfiles, upstream deps, - // and implicit dependencies like headers from .d files) if (!scopes.empty() && !is_tupfile(file_path_sv) && !pup::is_path_in_any_scope(scope_path_sv, scopes) && !std::binary_search(upstream_files.begin(), upstream_files.end(), file_path_sv) @@ -324,25 +313,18 @@ auto find_changed_files_with_implicit( continue; } - // Excluded dirs are not parsed this run, so nothing can consume their - // changes; observing them would record state we have no authority over. if (excludes.is_ignored_dir(pup::path::parent(file_path_sv))) { continue; } ++metrics.files_checked; - // Past every skip above: this is the one place that decides the walk looked at a file, - // so it is the one place that can say so (#288). scan.examined.push_back(file.path); - // File resolution: - // All paths are now source-relative (generated files include build root, e.g., "build/program"). auto file_path = file_path_sv; auto path = pup::path::is_absolute(file_path) ? file_path : pool.get(pup::path::join(source_root, file_path)); ++metrics.stat_calls; auto stat_result = pup::platform::stat_file(path); - // Overlay: parse-time inputs live under config_root but are indexed source-relative. if (!stat_result && file.type == pup::NodeType::File && !config_root.empty() && config_root != source_root && !pup::path::is_absolute(file_path)) { @@ -355,16 +337,12 @@ auto find_changed_files_with_implicit( } if (!stat_result) { - // Absent then, absent now: nothing happened to report. if (was_absent) { continue; } - // No active rule produces this, so its *absence* is not a change; an appearance is, - // which is why this skip is reached only once the file is known to be gone. if (std::binary_search(inactive_outputs.begin(), inactive_outputs.end(), file.path, pup::handle_less)) { continue; } - // Accounted for already: deleted and routed (#213), or read absent by its reader (#281). if (pup::has_flag(file.flags, pup::NodeFlags::AbsenceRouted)) { continue; } @@ -376,7 +354,6 @@ auto find_changed_files_with_implicit( continue; } - // Size check (fast path) auto current_size = stat_result->size; if (current_size != file.size) { if (verbose) { @@ -387,19 +364,16 @@ auto find_changed_files_with_implicit( continue; } - // Stat cache: skip hash if size + mtime match and not racy-clean auto const current_mtime_ns = stat_result->mtime_ns; auto const cached_mtime_ns = file.mtime_ns; auto const is_racy_clean = save_time_ns > 0 && cached_mtime_ns >= save_time_ns - RACY_CLEAN_THRESHOLD_NS; if (!no_stat_cache && cached_mtime_ns != 0 && current_mtime_ns == cached_mtime_ns && !is_racy_clean) { - // Stat cache hit: size + mtime match, trust cached hash ++metrics.hashes_skipped; continue; } - // Content hash check (authoritative) if (file.content_hash != pup::ZERO_HASH) { auto hash_result = pup::sha256_file(path); if (!hash_result || *hash_result != file.content_hash) { @@ -410,7 +384,6 @@ auto find_changed_files_with_implicit( changed.push_back(pool.intern(file_path_sv)); } } else { - // ZERO_HASH indicates hash wasn't computed - treat as changed to be safe if (verbose) { print(" Changed (no hash): {}\n", file_path_sv); } @@ -556,7 +529,6 @@ auto create_implicit_file( .parent_id = parent_id, .src_id = 0, .type = type, - // The run that reported this dep already saw it absent, so only its return is news (#281). .flags = present ? pup::NodeFlags::None : pup::NodeFlags::AbsenceRouted, .name = pool.intern(pup::path::filename(rel_sv)), .path = rel_path, @@ -569,7 +541,7 @@ auto create_implicit_file( return file_id; } -} // namespace +} auto CarriedState::carry_for(StringId path) const -> pup::index::FileEntry const* { @@ -609,7 +581,6 @@ auto serialize_graph_nodes( case pup::NodeType::Generated: { auto node_path = pup::graph::get_full_path(g, id, state.path_cache); if (node_path.empty()) { - // Dropping a slot would shift every later load-derived id (id == position + 1). index.add_file(pup::index::FileEntry { .id = id, .parent_id = pup::graph::get_parent_dir(g, id), @@ -632,7 +603,6 @@ auto serialize_graph_nodes( auto file_path = pup::global_pool().get((type == pup::NodeType::Generated) ? pup::path::join(output_root, fs_path) : pup::path::join(source_root, node_path)); - // Overlay: parse-time inputs live under config_root but are indexed source-relative. if (type == pup::NodeType::File && !config_root.empty() && config_root != source_root && !pup::platform::exists(file_path)) { auto config_path = pup::global_pool().get(pup::path::join(config_root, node_path)); @@ -667,8 +637,6 @@ auto serialize_graph_nodes( } auto& pool = pup::global_pool(); - // Records that this build already routed the file's absence, so the next one does - // not read the same stat failure as news. The slot has to stay: id == position + 1. auto entry_flags = std::binary_search(deleted_stale.begin(), deleted_stale.end(), pool.intern(node_path), pup::handle_less) ? node_flags | pup::NodeFlags::AbsenceRouted : node_flags; @@ -712,13 +680,9 @@ auto serialize_graph_nodes( break; } case pup::NodeType::Ghost: { - // A ghost that exists on disk but is produced by no rule is a - // foreign input (e.g. the variant's tup.config): record its content - // so change detection can see it. auto& pool = pup::global_pool(); auto node_path = pup::graph::get_full_path(g, id, state.path_cache); - // An out-of-authority output arrives here as a ghost; the record keeps its owner (#369). auto recorded_type = type; auto path_id = pup::StringId::Empty; auto content_hash = pup::Hash256 {}; @@ -755,8 +719,6 @@ auto serialize_graph_nodes( .mtime_ns = mtime_ns, .content_hash = content_hash, }); - // Like every other file-shaped arm: one path, one entry -- a discovered dependency - // naming this path must join this entry rather than mint a second one. if (!node_path.empty()) { path_to_id.insert(pool.intern(node_path), id); } @@ -765,7 +727,6 @@ auto serialize_graph_nodes( case pup::NodeType::Variable: case pup::NodeType::Group: case pup::NodeType::Root: { - // These node types must be in index to maintain consecutive ID sequence auto entry = pup::index::FileEntry { .id = id, .parent_id = pup::graph::get_parent_dir(g, id), @@ -783,7 +744,6 @@ auto serialize_graph_nodes( case pup::NodeType::Command: case pup::NodeType::Condition: case pup::NodeType::Phi: - // Unreachable: all_nodes yields only file-space ids past the is_command filter. break; } } @@ -973,10 +933,6 @@ auto process_implicit_deps( was_generated(ctx.prior_generated, rel_path) ? pup::NodeType::Generated : pup::NodeType::File ); - // Only when nothing orders this command after the file. Ordering is transitive — - // a codegen emitting one declared and one discovered output orders its consumer - // through the declared one — so this asks reachability, not adjacency: enumerating - // path shapes is how the two previous attempts at this taxed correct builds. auto ordered_after_dep = [&](pup::NodeId file_id, pup::NodeId consumer) -> bool { for (auto i = std::size_t { 0 }; i < ctx.ordered_memo_deps.size(); ++i) { if (ctx.ordered_memo_deps[i] == file_id) { @@ -1194,10 +1150,6 @@ auto preserve_old_implicit_edges( } } - // Command ids are positional and shift across builds (e.g. when an earlier-created - // command is removed), so the old edge's `to` id cannot be trusted to mean the same - // command; re-resolve each carried edge's command through the same join every other - // consumer uses. auto const new_lookup = index_command_lookup(ctx.index); for (auto const& edge : old_index.edges()) { @@ -1205,8 +1157,6 @@ auto preserve_old_implicit_edges( continue; } - // If the command is gone, drop the edge. If it survived and ran, the branch below - // drops it too: whatever that run reported is now the whole truth, empty included. auto const* old_cmd = old_index.find_command_by_id(edge.to); if (!old_cmd) { continue; @@ -1272,8 +1222,6 @@ auto is_dir_authoritative( if (contains_id(parsed_dirs, dir_str_id)) { return true; } - // A dir under a pruned nested-project root has a Tupfile this run never - // saw; its absence from available_dirs does not mean it was deleted. if (pup::is_path_in_any_scope(dir_path, pruned_dirs)) { return false; } @@ -1344,7 +1292,6 @@ auto merge_out_of_scope_commands( return new_id; }; - // Content that moved, not content that vanished (#247): the record stops claiming currency, not ownership. auto dep_state_changed = [&](pup::NodeId old_id) -> bool { auto const* old_file = old_index.find_file_by_id(old_id); if (!old_file || pup::is_empty(old_file->path)) { @@ -1368,13 +1315,11 @@ auto merge_out_of_scope_commands( return true; } } - // Sole witness for non-operand inputs (order-only, group, implicit, sticky). for (auto const* edge : old_index.edges_to(cmd.id)) { if (dep_state_changed(edge->from)) { return true; } } - // An extra output is owned by its edge alone, so the operand loop above cannot see it (#370). for (auto const* edge : old_index.edges_from(cmd.id)) { auto const* file = old_index.find_file_by_id(edge->to); if (file && file->type == pup::NodeType::Generated && dep_state_changed(edge->to)) { @@ -1392,12 +1337,8 @@ auto merge_out_of_scope_commands( if (find_joined(new_lookup, index_command_address(old_index, cmd))) { continue; } - // Marked, not dropped: dropping retracts which outputs this command owns along with the - // claim that they are current, and only the second is in doubt (#241). auto must_rerun = cmd.must_rerun || any_dep_changed(cmd); - // Omitted, not dropped (#243): an unresolvable operand is one this record cannot - // describe, but the outputs that did resolve are still owned by nothing else. auto unresolved = false; auto new_dir_id = pup::NodeId { 0 }; if (cmd.dir_id != pup::NodeId { 0 }) { @@ -1431,8 +1372,6 @@ auto merge_out_of_scope_commands( auto new_inputs = resolve_operands(cmd.inputs); auto new_order_only_inputs = resolve_operands(cmd.order_only_inputs); auto new_outputs = resolve_operands(cmd.outputs); - // An operand it could not carry means the record no longer describes what ran, so it - // keeps its outputs but stops claiming they are current. must_rerun = must_rerun || unresolved; auto new_cmd_id = pup::node_id::make_command(static_cast(ctx.index.commands().size()) + 1); @@ -1453,7 +1392,6 @@ auto merge_out_of_scope_commands( } for (auto const& edge : old_index.edges()) { - // Implicit edges are carried by preserve_old_implicit_edges instead. if (edge.type == pup::LinkType::Implicit) { continue; } @@ -1543,8 +1481,6 @@ auto propagate_command_effect( if (output_path_sv.empty()) { continue; } - // Set before push_path decides: a caller that dedups an already-changed path has still - // found an output, and forcing the command as well would be wrong. has_output_path = true; push_path(pup::global_pool().intern(output_path_sv)); } @@ -1636,7 +1572,6 @@ auto build_index( } std::sort(carried.refreshed.begin(), carried.refreshed.end(), pup::handle_less); - // Serialize file/directory nodes from the build graph auto [index, path_to_id] = serialize_graph_nodes(state, source_root, config_root, output_root, deleted_stale, carried, prior_generated); auto cmd_remap = serialize_command_nodes(state, index, path_to_id, must_rerun_cmds); @@ -1687,19 +1622,12 @@ auto build_index( .prior_generated = prior_generated, }; - // Process discovered implicit dependencies from compiler output process_implicit_deps(discovered_deps, ctx); - // After the discovered deps, so a merge-created copy of an old entry cannot shadow the - // fresh one this build just stat'd — a carried NodeFlags::AbsenceRouted would then discharge a - // later real deletion as already routed, and the consumer would never run (#237). Still - // before preserve_old_implicit_edges, which re-attaches carried edges by identity and so - // must see the merged records; that, not the ordering against the deps, is the constraint. if (old_index) { merge_out_of_scope_commands(*old_index, parse_scopes, excludes, parsed_dirs, available_dirs, pruned_dirs, ctx); } - // Preserve implicit edges from the old index for commands that weren't rebuilt if (old_index) { preserve_old_implicit_edges(*old_index, executed_cmds, ctx); } @@ -1766,29 +1694,13 @@ auto reject_shadowed_sources( auto& pool = pup::global_pool(); auto build_root_name = pup::graph::get_build_root_name(g); - // Only in-tree builds can destroy anything, now that no output may leave the build - // hierarchy (#385): out-of-tree, %o resolves under the build root, so a rule whose output - // path collides with a committed file writes beside it rather than over it. The collision - // is still confusing there -- the source becomes unreadable through that path -- but it is - // not data loss, and rejecting on it fails builds that cannot hurt anyone (a second build - // dir sees the first one's artifacts). if (!build_root_name.empty()) { return {}; } - // What the previous build's record settles. `Known`: a path it recorded as a source File is - // a source whatever is on disk now, and a path it recorded while producing nothing at it is - // owned by nobody (#389); absence stays undecidable -- a generated file it does not mention - // is what a scoped build leaves behind for out-of-scope outputs -- which is why the test - // below is positive. `NeverBuilt`: nothing we produced can be on disk yet, so anything - // sitting at an output's path is a source. `Lost`: a build happened here and putup cannot - // tell either way, which is the only case that says so out loud. auto const known = prior.kind == pup::index::PriorPaths::Kind::Known; auto shadowed = pup::Vec {}; - // Guard-satisfied producers only, like every other command walk here: an inactive - // conditional branch declares outputs it will never write, and rejecting on those - // fails projects that build fine. for (auto cmd_id : pup::graph::all_nodes(g)) { if (!pup::node_id::is_command(cmd_id) || !pup::graph::is_guard_satisfied(g, cmd_id)) { continue; @@ -1799,15 +1711,10 @@ auto reject_shadowed_sources( continue; } auto rel_sv = pool.get(pup::strip_path_prefix(full_path_sv, build_root_name)); - // The two-stage configure design has a rule produce the tup.config that the same - // build then reads as configuration. Match the whole basename: a suffix test also - // exempts anything merely ending in those characters, e.g. mytup.config. if (rel_sv == "tup.config" || rel_sv.ends_with("/tup.config")) { continue; } - // On disk in either input tree. Without this the printed remedy -- delete the file - // and try again -- would not clear the error, because the index still records it. auto abs_sv = pool.get(pup::path::join(source_root, rel_sv)); auto on_disk = pup::platform::exists(abs_sv); if (!on_disk && !config_root.empty() && config_root != source_root) { @@ -1869,7 +1776,6 @@ auto reject_unresolved_ghosts( auto build_root_name = pup::graph::get_build_root_name(g); for (auto id : pup::graph::nodes_of_type(g, pup::NodeType::Ghost)) { - // Guard-satisfied consumers only, like every other command walk here (#386). auto consumed = false; pup::graph::edges_for_each( g, id, pup::graph::EdgeDirection::Forward, pup::graph::edge_mask::consumers, [&](pup::NodeId consumer) { consumed = consumed || pup::graph::is_guard_satisfied(g, consumer); } @@ -1892,8 +1798,6 @@ auto reject_unresolved_ghosts( auto err = pup::Buf {}; err.fmt("Missing input file (unresolved ghost): {}\n", path_sv); - // -a only pulls in rules this build left out of scope; when nothing was left out, - // offering it sends the user round the same failure (#222). err.append(all_deps_would_help ? " Hint: try building with -a to include upstream dependencies" : " Hint: no rule in this build produces it — the rule that did may have been removed"); return pup::make_error(pup::ErrorCode::ParseError, err.view()); } @@ -1931,9 +1835,6 @@ auto detect_new_commands( auto const joined = find_joined(old_lookup, address); auto const* previous = joined ? idx.find_command_by_id(*joined) : nullptr; - // A record that is not evidence outranks the signature: the command must run again even - // when nothing about it changed, and its outputs must be treated as changed so consumers - // of whatever it half-wrote, or never wrote, are rescheduled too. auto signature = pup::graph::compute_command_signature(g, id, state.path_cache); auto must_rerun = previous != nullptr && previous->must_rerun; if (must_rerun) { @@ -1975,8 +1876,6 @@ auto reconcile_input_set( auto const& g = state.graph; auto result = RoutingDelta {}; - // Every typed path, so "left the graph" below means gone, not merely not-a-source: - // a generated file the user deleted is still a graph node and must not read as removed. auto graph_paths = pup::Vec {}; auto new_sources = pup::Vec {}; for (auto id : pup::graph::all_nodes(g)) { @@ -2027,8 +1926,6 @@ auto reconcile_input_set( } ); } - // Explicit comparator: libc++ extern-templates std::__sort for unsigned int*, - // so the default form links against a libc++ we deliberately do not have. std::sort(orphaned.begin(), orphaned.end(), [](pup::NodeId a, pup::NodeId b) { return a < b; }); orphaned.erase(std::unique(orphaned.begin(), orphaned.end()), orphaned.end()); @@ -2070,15 +1967,10 @@ auto remove_stale_outputs( auto deleted = pup::Vec {}; auto retired_commands = false; for (auto const& cmd : idx.commands()) { - // Only delete outputs of a command whose directory we have authoritative - // knowledge of this run; anything else is preserved. if (!is_dir_authoritative(idx, cmd.dir_id, parse_scopes, excludes, parsed_dirs, available_dirs, pruned_dirs)) { continue; } - // Staleness is per file, not per command: a rule that drops one of its outputs - // still joins through the ones it kept, so asking only "did this command survive" - // would leave the dropped file owned by nothing and never delete it. for (auto const* edge : idx.edges_from(cmd.id)) { auto const* file = idx.find_file_by_id(edge->to); if (!file || file->type != pup::NodeType::Generated) { @@ -2088,7 +1980,6 @@ auto remove_stale_outputs( continue; } - // Paths now include build root (e.g., "build/program") auto file_path_sv = pup::global_pool().get(file->path); auto abs_path = pup::global_pool().get(pup::path::join(source_root, file_path_sv)); if (pup::platform::exists(abs_path)) { @@ -2111,7 +2002,6 @@ auto remove_stale_outputs( if (!find_joined_command(join, idx, cmd)) { retired_commands = true; if (verbose) { - // Not graph::command_label: the command has left the graph, but the index keeps its operands, so a pattern shared by a foreach still names one command. auto label = pup::is_empty(cmd.display) ? pup::index::get_command_string(idx, cmd) : cmd.display; auto const* dir = idx.find_file_by_id(cmd.dir_id); auto dir_sv = dir ? pup::global_pool().get(dir->path) : std::string_view {}; @@ -2197,11 +2087,6 @@ auto build_single_variant( } auto scopes = compute_build_scopes(opts, *layout); - // Only scope parsing when explicit targets are given. - // CWD-derived scoping should still parse all Tupfiles so that - // out-of-scope Tupfile changes are detected for incremental builds. - // When -a is set, always parse all Tupfiles so that cross-directory - // producers are discovered and ghost nodes get resolved. auto parse_scopes = (opts.targets.empty() || opts.include_all_deps) ? pup::Vec {} : scopes; @@ -2220,8 +2105,6 @@ auto build_single_variant( }; auto result = build_context(opts, ctx_opts); - // build_context loads the old index as part of its work; that span is timed - // separately, so discount it here to keep the phases disjoint. pup::thread_metrics().parse_time = std::chrono::duration_cast(pup::SteadyClock::now() - variant_start) - pup::thread_metrics().index_load_time; if (!result) { @@ -2232,11 +2115,8 @@ auto build_single_variant( auto& ctx = *result; auto const refused_dir_count = std::size_t { ctx.refused_dirs().size() }; auto& bs = ctx.graph(); - // No early exit on an empty graph: having nothing to run is not having nothing to clean up (#231). auto num_commands = std::size_t { pup::graph::nodes_of_type(bs.graph, pup::NodeType::Command).size() }; - // One line, not one per rule: a warning that fires on every rule of a green build teaches - // everyone to scroll past warnings. The per-object findings live in `parse`. if (auto unscanned = check_unscanned_compiles(bs.graph, bs.path_cache); !unscanned.empty()) { vprint( variant_name, @@ -2264,9 +2144,6 @@ auto build_single_variant( auto index_path = pup::global_pool().get(ctx.layout().index_path()); auto const* old_idx_ptr = ctx.old_index(); - // A record too old for read_index still says which paths it recorded as sources; the version - // gate retracts its currency, not that (#291). Nothing recovered this way is ever loaded as - // `old_idx_ptr` -- it answers this one question and dies here. auto const prior = old_idx_ptr != nullptr ? pup::index::prior_paths(*old_idx_ptr) : pup::index::read_prior_paths(index_path); @@ -2282,23 +2159,16 @@ auto build_single_variant( return EXIT_FAILURE; } auto use_incremental = false; - // Carries "has not succeeded since it last failed": seeded from the previous index, cleared - // only by a successful run. A build in which the command does not run at all -- a target or - // scoped build -- must not forget it. auto must_rerun_cmds = pup::NodeIdMap32 {}; auto changed_files = pup::Vec {}; - // What the comparison below looked at, and so the only files this build may restate (#288). auto examined_files = pup::Vec {}; auto forced_cmds = pup::Vec {}; auto deleted_stale = pup::Vec {}; - // Outlives the block only so the ordering can be derived after the up-to-date exit below. auto join = CommandLookup {}; if (old_idx_ptr) { auto const& idx = *old_idx_ptr; - // Build the identity → NodeId map: the cross-build join key for commands. - // Must happen after parsing (operands set) but before incremental logic. auto cmd_index_start = pup::SteadyClock::now(); join = graph_command_lookup(bs); auto cmd_index_elapsed = pup::SteadyClock::now() - cmd_index_start; @@ -2309,8 +2179,6 @@ auto build_single_variant( upstream_files = pup::graph::collect_upstream_files(bs, scopes); } - // Always include implicit deps (headers from .d files) for in-scope - // commands, even if the headers live outside the scoped directories. auto scope_crossing_inputs = Vec {}; if (!scopes.empty()) { scope_crossing_inputs = collect_scope_crossing_inputs(idx, scopes); @@ -2334,8 +2202,6 @@ auto build_single_variant( } } - // Before detection, not after: a deleted output is a change like any other, and - // a consumer reaching it order-only is only notified if detection sees it gone. auto stale_start = pup::SteadyClock::now(); auto stale_result = remove_stale_outputs( idx, @@ -2422,7 +2288,6 @@ auto build_single_variant( } } - // A retired record is not "nothing to do": only the write below persists the retirement (#245). if (changed_files.empty() && forced_cmds.empty() && !retired_commands) { if (refused_dir_count > 0) { report_refused_dirs(variant_name, ctx.refused_dirs()); @@ -2482,7 +2347,6 @@ auto build_single_variant( if (use_tty_progress) { pup::exec::finalize_progress(prev_lines); } - // The command, not the display: "CC main.o" does not say which flags broke, and this is the only line a build prints of what actually ran. veprint(variant_name, "FAILED: {}\n", pool.get(job.command)); if (!pup::is_empty(job_result.output)) { auto output_sv = pool.get(job_result.output); @@ -2494,7 +2358,6 @@ auto build_single_variant( ? job_result.deps_for_command : job.id; if (job_result.success) { - // Recorded even when it discovered nothing: an empty report is a report, and treating it as silence carried dead edges forever (#224). executed_cmds.push_back(target_id); } @@ -2508,7 +2371,6 @@ auto build_single_variant( auto to_resolve = pup::path::is_absolute(dep_sv) ? dep_sv : pool.get(pup::path::join(working_dir_sv, dep_sv)); - // Dropping a dep loses an edge no later build re-derives, so neither arm is gated on -v. auto resolved_result = pup::platform::canonical(to_resolve); if (!resolved_result) { eprint("Warning: Skipping dependency '{}': {}\n", dep_sv, resolved_result.error().msg()); @@ -2524,7 +2386,6 @@ auto build_single_variant( } deps.push_back(pool.intern(rel_sv)); } else { - // Recorded, not ignored: tup drops out-of-tree deps unless --full-deps, putup tracks them (DESIGN.md). deps.push_back(*resolved_result); } } @@ -2537,30 +2398,23 @@ auto build_single_variant( } }); - scheduler.on_progress([&](std::size_t /* done */, std::size_t total) { + scheduler.on_progress([&](std::size_t, std::size_t total) { progress.total = total; }); - // Identify config-generating commands to exclude from regular build - // (config rules should only run during 'pup configure') auto config_cmds = find_config_commands(bs, pup::global_pool().get(ctx.layout().source_root)); auto config_cmd_ids = NodeIdMap32 {}; for (auto const& cfg : config_cmds) { config_cmd_ids.set(cfg.cmd_id, 1); - // Nothing here will ever run a config rule, so a needing-to-run record on one is undischargeable. must_rerun_cmds.remove(cfg.cmd_id); } - // Past the up-to-date exit, not with the rest of the incremental work: this is one pair per - // recorded discovery, and a build that schedules nothing would derive all of them to run none. auto injected_ordering = old_idx_ptr ? collect_discovered_ordering(*old_idx_ptr, join) : pup::Vec {}; auto start = pup::SteadyClock::time_point { pup::SteadyClock::now() }; - // Composable filter: layer independent concerns, intersect when combined auto filter = BuildFilter {}; - // Same pairs the scheduler gets, but kept on contradiction: over-routing costs a run, not the build. if (use_incremental) { filter.intersect_with(pup::graph::collect_affected_commands(bs.graph, changed_files, forced_cmds, injected_ordering)); } @@ -2577,7 +2431,6 @@ auto build_single_variant( filter.intersect_with(std::move(scope_cmds)); } - // Exclude config-generating commands (they run during configure, not build) if (!config_cmds.empty()) { auto non_config = pup::NodeIdMap32 {}; for (auto id : pup::graph::all_nodes(bs.graph)) { @@ -2588,7 +2441,6 @@ auto build_single_variant( filter.intersect_with(std::move(non_config)); } - // A command this build means to run is unverified until it succeeds: an abort strands or kills it silently (#304). for (auto id : pup::graph::all_nodes(bs.graph)) { if (!node_id::is_command(id) || !pup::graph::is_guard_satisfied(bs.graph, id)) { continue; @@ -2596,7 +2448,6 @@ auto build_single_variant( if (auto const* intended = filter.ptr(); intended != nullptr && !intended->contains(id)) { continue; } - // Nothing here will ever run a config rule, so a mark on one is undischargeable (as above). if (config_cmd_ids.contains(id)) { continue; } @@ -2606,7 +2457,6 @@ auto build_single_variant( auto build_result = scheduler.build(bs, filter.ptr(), injected_ordering); auto end = pup::SteadyClock::time_point { pup::SteadyClock::now() }; auto duration = std::chrono::milliseconds { std::chrono::duration_cast(end - start) }; - // The scheduler times its own job-list construction; the rest of the span is execution. pup::thread_metrics().exec_time = std::chrono::duration_cast(end - start) - pup::thread_metrics().job_list_time; @@ -2629,10 +2479,8 @@ auto build_single_variant( vprint(variant_name, "Nothing to do.\n"); } } else if (stats.failed_jobs > 0) { - // Ahead of the dry-run branch so that a scheduler which ever fails a dry job reports it. vprint(variant_name, "Build completed: {} commands ({} failed) in {}ms\n", stats.completed_jobs, stats.failed_jobs, duration.count()); } else if (opts.dry_run) { - // "Would run", matching clean's "Would remove": a dry run has completed nothing. vprint(variant_name, "Would run: {} commands\n", stats.completed_jobs); } else { vprint(variant_name, "Build completed: {} commands in {}ms\n", stats.completed_jobs, duration.count()); @@ -2645,7 +2493,6 @@ auto build_single_variant( auto final_index = std::optional {}; auto index_saved = true; if (!opts.dry_run) { - // Persisting a partial failure is safe because must_rerun records it, not because a failed command's outputs are missing. auto output_root_str = pup::global_pool().get(ctx.layout().output_root); auto index_rebuild_start = pup::SteadyClock::time_point { pup::SteadyClock::now() }; auto index = pup::index::Index { build_index( @@ -2692,15 +2539,14 @@ auto build_single_variant( } } - // A build owes a record of what it ran: without one the next build cannot tell it happened. return stats.failed_jobs > 0 || !index_saved || refused_dir_count > 0 ? EXIT_FAILURE : EXIT_SUCCESS; } -} // anonymous namespace +} auto cmd_build(Options const& opts) -> int { return for_each_variant(opts, build_single_variant, "Building"); } -} // namespace pup::cli +} diff --git a/src/core/instruction.cpp b/src/core/instruction.cpp index 97cde9ec..02624eb4 100644 --- a/src/core/instruction.cpp +++ b/src/core/instruction.cpp @@ -114,7 +114,7 @@ auto is_unnumbered_flag(char letter) -> bool || letter == 'o' || letter == 'O' || letter == 'd'; } -} // namespace +} auto InstructionBuilder::flush() -> void { @@ -289,4 +289,4 @@ auto parse_instruction(std::string_view text) -> Result return builder.take(); } -} // namespace pup +} diff --git a/src/graph/builder.cpp b/src/graph/builder.cpp index 20fbf4ae..f07b5ecc 100644 --- a/src/graph/builder.cpp +++ b/src/graph/builder.cpp @@ -41,17 +41,12 @@ namespace pup::graph { -// Graph-module-internal functions (defined in dag.cpp, not in public header) auto get_file_node(Graph& graph, NodeId id) -> FileNode*; auto get_command_node(Graph& graph, NodeId id) -> CommandNode*; auto add_condition_node(Graph& graph, ConditionNode node) -> Result; auto get_condition_node(Graph const& graph, NodeId id) -> ConditionNode const*; namespace { -// --------------------------------------------------------------------------- -// §1 — Inline helpers -// --------------------------------------------------------------------------- - auto str(StringId id) -> std::string_view { return global_pool().get(id); } auto intern(std::string_view s) -> StringId { return global_pool().intern(s); } @@ -73,10 +68,6 @@ struct ScopeGuard { template ScopeGuard(F) -> ScopeGuard; -// --------------------------------------------------------------------------- -// §2 — Path primitives -// --------------------------------------------------------------------------- - /// Strip trailing slashes from a path string auto strip_trailing_slashes(std::string_view s) -> std::string_view { @@ -214,10 +205,6 @@ auto request_demand_driven_parse( } } -// --------------------------------------------------------------------------- -// §3 — Path transform context -// --------------------------------------------------------------------------- - /// Context for transforming paths to Tupfile-relative coordinates. /// /// Commands execute from the Tupfile's source directory, so all paths in commands @@ -338,21 +325,6 @@ auto transform_output_path( return pup::make_source_relative(out, str(tc.source_to_root), str(tc.current_dir_id)); } -// --------------------------------------------------------------------------- -// §4 — Node resolution and creation -// --------------------------------------------------------------------------- - -// ============================================================================ -// Node-Traversal Path Resolution -// ============================================================================ -// Path resolution uses graph traversal instead of string manipulation: -// - ".." → walk to parent node -// - "name" → find/create child node -// -// This naturally unifies input and output path resolution because both traverse -// to the same node when the paths are equivalent (e.g., $(B)/include/header.h -// from an input and the variant-mapped output both resolve to the same node). - /// Was this path generated outside this parse — by a previous round, or by a directory this /// build never parsed? Both lists are canonical and sorted, so the lookup is the same shape as /// the graph's own. @@ -371,15 +343,8 @@ auto resolve_input_node( std::string_view path ) -> Result { - // Input paths are already source-relative from expand_inputs() which normalizes them - // by combining with current_dir. No further normalization needed here. - - // Track whether the path originally had the build prefix or pointed to output_root. - // This indicates the path should reference a generated file, not a source file. auto had_build_prefix = false; - // For variant builds, paths like "build/include/header.h" (from $(B)/include/header.h) - // already have the build root prefix. Strip it to get source-relative paths. auto build_root_name = get_build_root_name(ctx.state->graph); auto normalized_path = global_pool().get(pup::strip_path_prefix(path, build_root_name)); if (normalized_path != path) { @@ -396,49 +361,35 @@ auto resolve_input_node( } } - // A seeded match names a file a later directory will generate; without this it would - // resolve to a source-side Ghost and the producer's output would become a second node. if (!had_build_prefix && is_seeded_generated(ctx, normalized_path)) { had_build_prefix = true; } - // With BUILD_ROOT_ID model: - // - Source files are under SOURCE_ROOT_ID (0) at source-relative paths - // - Generated/Ghost files are under BUILD_ROOT_ID at source-relative paths - auto& pool = global_pool(); - // First check if node exists under BUILD_ROOT_ID (generated files) if (auto build_path = ctx.state->graph.paths.find_path(normalized_path, pool, PathId::BuildRoot)) { if (auto const* hit = ctx.state->graph.path_to_node.find(to_underlying(*build_path))) { return NodeId { *hit }; } } - // If path had build prefix, it's referencing a generated file. Even if a source file - // exists at the same path, create a Ghost node under BUILD_ROOT_ID so it can be - // upgraded to Generated when the output rule is processed. if (had_build_prefix) { auto path_id = ctx.state->graph.paths.intern_path(normalized_path, pool, PathId::BuildRoot); return ensure_file_node(ctx.state->graph, path_id, NodeType::Ghost); } - // Check under SOURCE_ROOT_ID (source files) if (auto source_path = ctx.state->graph.paths.find_path(normalized_path, pool, PathId::SourceRoot)) { if (auto const* hit = ctx.state->graph.path_to_node.find(to_underlying(*source_path))) { return NodeId { *hit }; } } - // Node doesn't exist - check filesystem to determine type auto source_path = pool.get(pup::path::join(str(ctx.options.source_root), normalized_path)); if (pup::platform::exists(source_path)) { auto path_id = ctx.state->graph.paths.intern_path(normalized_path, pool, PathId::SourceRoot); return ensure_file_node(ctx.state->graph, path_id, NodeType::File); } - // In 3-tree builds, files may live in config_root (alongside Tupfiles) rather than - // source_root. Check config_root as a fallback for source file resolution. if (!is_empty(ctx.options.config_root) && str(ctx.options.config_root) != str(ctx.options.source_root)) { auto config_path_sv = pool.get(pup::path::join(str(ctx.options.config_root), normalized_path)); if (pup::platform::exists(config_path_sv)) { @@ -447,14 +398,12 @@ auto resolve_input_node( } } - // Check if file exists in build directory (e.g., tup.config, or already-generated files) auto build_path_sv = pool.get(pup::path::join(str(ctx.options.output_root), normalized_path)); if (pup::platform::exists(build_path_sv)) { auto path_id = ctx.state->graph.paths.intern_path(normalized_path, pool, PathId::BuildRoot); return ensure_file_node(ctx.state->graph, path_id, NodeType::Ghost); } - // File doesn't exist anywhere - create Ghost node under BUILD_ROOT_ID auto path_id = ctx.state->graph.paths.intern_path(normalized_path, pool, PathId::BuildRoot); return ensure_file_node(ctx.state->graph, path_id, NodeType::Ghost); } @@ -479,7 +428,6 @@ auto get_or_create_group_node( return *cached; } - // Get or create parent directory node auto dir_path_id = ctx.state->graph.paths.intern_path(directory, global_pool(), PathId::SourceRoot); auto parent_id_result = ensure_file_node(ctx.state->graph, dir_path_id, NodeType::Directory); if (!parent_id_result) { @@ -487,8 +435,6 @@ auto get_or_create_group_node( } auto parent_id = *parent_id_result; - // Check if group node already exists in graph (e.g., from previous Tupfile) - // Group nodes are stored with angle-bracket name like "" auto gb = Buf {}; gb += '<'; gb += name; @@ -499,7 +445,6 @@ auto get_or_create_group_node( return *existing; } - // Create new group node auto node = FileNode { .type = NodeType::Group, .name = intern(group_basename), @@ -558,12 +503,10 @@ auto create_command_node( auto cmd_id = *cmd_id_result; - // Add sticky edges from Tupfile and included files to this command for (auto src_id : ctx.sticky_sources) { (void)add_edge(ctx.state->graph, src_id, cmd_id, LinkType::Sticky); } - // Add sticky edges from used config variables (fine-grained dependency tracking) 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]); @@ -572,7 +515,6 @@ auto create_command_node( } } - // Add sticky edges from condition config variables (phi-node model) auto const* ccv = ctx.condition_config_vars.data(); for (std::size_t i = 0, n = ctx.condition_config_vars.size(); i < n; ++i) { auto const* node_id = state.config_var_nodes.find(ccv[i]); @@ -581,8 +523,6 @@ auto create_command_node( } } - // Add sticky edges from condition env variables (symmetric to condition_config_vars - // — so an env-driven guard flip changes every guarded command's identity). auto const* cev = ctx.condition_env_vars.data(); for (std::size_t i = 0, n = ctx.condition_env_vars.size(); i < n; ++i) { auto const* node_id = state.imported_env_var_nodes.find(cev[i]); @@ -591,7 +531,6 @@ auto create_command_node( } } - // Add sticky edges from used imported env variables (fine-grained dependency tracking) auto const* uev = ctx.used_env_vars.data(); for (std::size_t i = 0, n = ctx.used_env_vars.size(); i < n; ++i) { auto const* node_id = state.imported_env_var_nodes.find(uev[i]); @@ -600,12 +539,6 @@ auto create_command_node( } } - // Add sticky edges from exported env variables. An exported var reaches the - // command's subprocess environment (read as bare $VAR) and so affects its output - // even when it never appears in the command text. Recording it as a sticky edge - // makes that dependency explicit and folds its value into the command identity, - // so a change to the var triggers a rebuild. (process_export guarantees every - // exported var has a value node.) auto const* exv = ctx.exported_vars.data(); for (std::size_t i = 0, n = ctx.exported_vars.size(); i < n; ++i) { auto const* node_id = state.imported_env_var_nodes.find(exv[i]); @@ -617,10 +550,6 @@ auto create_command_node( return cmd_id; } -// --------------------------------------------------------------------------- -// §5 — Condition model -// --------------------------------------------------------------------------- - /// Check if all guards in the condition stack are satisfied (current context is active) /// Returns true if no guards or all guards match their expected polarity auto is_context_active(BuilderContext const& ctx) -> bool @@ -704,10 +633,6 @@ auto format_condition_expr(parser::EvalContext& eval, parser::Conditional const& return buf.intern(global_pool()); } -// --------------------------------------------------------------------------- -// §6 — Glob expansion -// --------------------------------------------------------------------------- - /// One entry of a rule's expanded input list; not every entry names a file. struct RuleInput { enum class Kind { File, @@ -756,8 +681,6 @@ auto expand_glob_pattern( } } - // Unconditional: the generated half of the match set would otherwise depend on - // how far the parse fixpoint has progressed. auto pattern_dir = pup::path::parent(path); auto abs_pattern_dir_sv = pool.get(pup::path::normalize(pool.get(pup::path::join(str(ctx.current_dir), pattern_dir)))); request_demand_driven_parse(*ctx.eval, abs_pattern_dir_sv); @@ -776,8 +699,6 @@ auto expand_glob_pattern( } } - // A file this round has not reached yet, but a previous round proved the project - // generates. Without it the match set is a function of where the parse has got to. for (auto seeded : ctx.options.generated_seed) { if (glob.matches(pool.get(seeded))) { matches.push_back(RuleInput { RuleInput::Kind::File, seeded }); @@ -788,7 +709,6 @@ auto expand_glob_pattern( match.glob = pattern_id; } - // Sorting and deduping across two path spaces would order by where the build tree lives. std::ranges::sort(matches, {}, [&pool](RuleInput const& in) { return pool.get(in.value); }); matches.erase( std::unique(matches.begin(), matches.end(), [](RuleInput const& a, RuleInput const& b) { return a.value == b.value; }), @@ -829,7 +749,6 @@ auto apply_exclusions( for (auto excl_id : *paths) { auto excl = pool.get(excl_id); if (ctx.options.expand_globs && parser::has_glob_chars(excl)) { - // Matched against the merged list, not re-expanded against disk: generated matches are not on disk. auto pattern_id = is_empty(ctx.current_dir) ? pup::path::normalize(excl) : pup::path::normalize(pool.get(pup::path::join(str(ctx.current_dir), excl))); auto glob = parser::Glob { pool.get(pattern_id) }; for (auto it = result.begin(); it != result.end();) { @@ -853,10 +772,6 @@ auto apply_exclusions( } } -// --------------------------------------------------------------------------- -// §7 — Input/output expansion -// --------------------------------------------------------------------------- - /// Generated command text is not a Tupfile template: every byte is literal except a /// % marker, which pass 2 still has to resolve. auto atoms_from_generated_text(std::string_view text) -> Instruction @@ -1038,7 +953,6 @@ auto expand_outputs( auto full_output_path_sv = pool.get(pup::path::normalize(pool.get(pup::path::join(str(ctx.current_dir), output_path_sv)))); - // Guards do not gate this, and an absolute path is rejected rather than re-rooted (#385). auto const absolute = pup::path::is_absolute(full_output_path_sv); if (absolute || full_output_path_sv == ".." || full_output_path_sv.starts_with("../")) { auto msg = Buf {}; @@ -1168,11 +1082,6 @@ auto expand_inputs( return TokenizedInputs { std::move(result), token }; } -// --------------------------------------------------------------------------- -// §8 — Include/import/export -// --------------------------------------------------------------------------- - -// Forward declaration (mutual recursion: include_single_file → process_statement → process_conditional → process_statement) auto process_statement( BuilderContext& ctx, Builder& state, @@ -1249,9 +1158,6 @@ auto include_single_file( bool is_rules ) -> Result { - // A file joins each conditional context at most once: the same include in - // the same guard stack is redundant, but a later include under different - // guards is a distinct contribution (issue #91). auto context_key = Buf {}; context_key.fmt("{}", include_path); for (auto const& guard : ctx.condition_stack) { @@ -1346,10 +1252,6 @@ auto apply_pending_weak_assignments(BuilderContext& ctx, Builder& state) -> void ctx.pending_weak_assignments.clear(); } -// --------------------------------------------------------------------------- -// §9 — Directive processors -// --------------------------------------------------------------------------- - /// Find or create a `NodeType::Variable` node under the env-var directory for the /// given env var name and value. Updates the existing node's name and content_hash /// if the value changed. Returns the node id, or nullopt if the env-var directory @@ -1457,13 +1359,9 @@ auto process_export( parser::Export const& exp ) -> Result { - // Per tup manual: "adds the environment variable VARIABLE to the export - // list for future :-rules" auto var_name_sv = str(exp.var_name); ctx.exported_vars.insert(to_underlying(intern(var_name_sv))); - // An exported var reaches the command's subprocess environment, so its value - // is part of the command's identity even when the Tupfile never reads it. auto name_buf = Buf {}; name_buf += var_name_sv; auto const* env_val = pup::platform::get_env(name_buf.c_str()); @@ -1479,38 +1377,27 @@ auto process_import( parser::Import const& imp ) -> Result { - // Per tup manual: "sets a variable inside the Tupfile that has the value - // of the environment variable" auto var_name_sv = str(imp.var_name); auto value_id = StringId::Empty; - // Need null-terminated string for getenv auto name_buf = Buf {}; name_buf += var_name_sv; - // 1. Try environment first if (auto const* env_val = pup::platform::get_env(name_buf.c_str())) { value_id = intern(env_val); - } - // 2. If the author wrote `?= default`, that default is the explicit - // revert-target for the env-unset case and wins over the cache. - else if (imp.default_value) { + } else if (imp.default_value) { auto expanded = parser::expand(*ctx.eval, *imp.default_value); if (!expanded) { return pup::unexpected(expanded.error()); } value_id = *expanded; - } - // 3. No env, no default — fall back to the cached value from the previous - // build so plain `import VAR` stays stable across shell sessions that - // forget to re-export the variable. - else if (auto it = std::lower_bound( - state.options.cached_env_vars.begin(), - state.options.cached_env_vars.end(), - var_name_sv, - [](auto const& p, std::string_view k) { return str(p.first) < k; } - ); - it != state.options.cached_env_vars.end() && str(it->first) == var_name_sv) { + } else if (auto it = std::lower_bound( + state.options.cached_env_vars.begin(), + state.options.cached_env_vars.end(), + var_name_sv, + [](auto const& p, std::string_view k) { return str(p.first) < k; } + ); + it != state.options.cached_env_vars.end() && str(it->first) == var_name_sv) { value_id = it->second; } @@ -1522,7 +1409,6 @@ auto process_import( ctx.vars->set(var_name_sv, value_sv); } - // Track this as an imported variable for fine-grained dependency tracking auto var_name_id = to_underlying(intern(var_name_sv)); state.imported_var_names.insert(var_name_id); @@ -1559,19 +1445,14 @@ auto process_assignment( parser::Assignment const& assign ) -> Result { - // Evaluate the variable name (may contain variable refs like foo-$(BAR)) auto name = parser::expand(*ctx.eval, assign.name); if (!name) { return pup::unexpected(name.error()); } - // Save current tracking state and clear for value expansion - // This lets us capture which config/env vars are used in the RHS - // SortedIdVec moved-from state is empty, so no explicit clear() needed auto saved_config_vars = std::move(ctx.used_config_vars); auto saved_env_vars = std::move(ctx.used_env_vars); - // Evaluate the value - callbacks will populate used_*_vars auto value = parser::expand(*ctx.eval, assign.value); if (!value) { ctx.used_config_vars = std::move(saved_config_vars); @@ -1579,19 +1460,14 @@ auto process_assignment( return pup::unexpected(value.error()); } - // Capture the dependencies from RHS expansion. The value also depends on - // every condition guarding this assignment: reaching it required those - // config/env reads. auto captured_config_deps = std::move(ctx.used_config_vars); auto captured_env_deps = std::move(ctx.used_env_vars); captured_config_deps.merge_from(ctx.condition_config_vars); captured_env_deps.merge_from(ctx.condition_env_vars); - // Restore tracking state ctx.used_config_vars = std::move(saved_config_vars); ctx.used_env_vars = std::move(saved_env_vars); - // Config variables are read-only (loaded from tup.config), so only Regular and Node are writable auto* db = ctx.eval->vars; if (assign.var_kind == parser::VarRef::Kind::Node) { db = ctx.eval->node_vars; @@ -1711,26 +1587,18 @@ auto process_conditional( parser::Conditional const& cond ) -> Result { - // Save and clear used_*_vars to capture which vars the condition uses. - // Env vars are tracked symmetrically with config vars so that an env-driven - // guard flip (e.g. `ifeq ($(TUP_PLATFORM),win32)`) folds into the identity of - // every command in the conditional block. auto saved_config_vars = std::move(ctx.used_config_vars); auto saved_used_env_vars = std::move(ctx.used_env_vars); - // Evaluate condition value - this may use config vars like @(MODE) or env vars auto condition_true = parser::evaluate_condition(*ctx.eval, cond); - // Capture vars used in the condition expression auto condition_vars = std::move(ctx.used_config_vars); auto condition_env_vars_used = std::move(ctx.used_env_vars); ctx.used_config_vars = std::move(saved_config_vars); ctx.used_env_vars = std::move(saved_used_env_vars); - // Save condition_*_vars, then merge in condition-specific vars auto saved_condition_vars = std::move(ctx.condition_config_vars); auto saved_condition_env_vars = std::move(ctx.condition_env_vars); - // Rebuild: copy saved entries + merge condition_vars auto const* d = saved_condition_vars.data(); for (std::size_t i = 0, n = saved_condition_vars.size(); i < n; ++i) { ctx.condition_config_vars.insert(d[i]); @@ -1746,8 +1614,6 @@ auto process_conditional( ctx.condition_env_vars = std::move(saved_condition_env_vars); }); - // Guards exist only for conditions that can flip without a Tupfile edit - // (config or env reads recorded): static conditionals are textual like tup. auto is_static = condition_vars.empty() && condition_env_vars_used.empty(); if (is_static) { auto active = is_context_active(ctx); @@ -1764,7 +1630,6 @@ auto process_conditional( return {}; } - // Create condition node for phi-node model auto cond_expr = format_condition_expr(*ctx.eval, cond); auto cond_node = ConditionNode { .expression = cond_expr, @@ -1777,7 +1642,6 @@ auto process_conditional( } auto cond_id = *cond_id_result; - // Helper to process a branch with given polarity auto process_branch = [&]( Vec> const& body, bool polarity, @@ -1786,9 +1650,6 @@ auto process_conditional( ctx.condition_stack.push_back(Guard { .condition = cond_id, .polarity = polarity }); auto pop_guard = ScopeGuard([&] { ctx.condition_stack.pop_back(); }); - // Macro defs and assignments in an inactive branch serve only that - // branch's guarded rules; the active world after endif must not see - // them. auto saved_macros = is_active ? decltype(ctx.macros) {} : ctx.macros; auto restore_macros = ScopeGuard([&] { if (!is_active) { @@ -1826,13 +1687,11 @@ auto process_conditional( return {}; }; - // Process THEN branch (polarity=true means "condition must be true") auto then_result = process_branch(cond.then_body, true, condition_true); if (!then_result) { return pup::unexpected(then_result.error()); } - // Process ELSE branch (polarity=false means "condition must be false") auto else_result = process_branch(cond.else_body, false, !condition_true); if (!else_result) { return pup::unexpected(else_result.error()); @@ -1867,10 +1726,6 @@ auto process_include( return include_single_file(ctx, state, include_root, str(*resolved), false); } -// --------------------------------------------------------------------------- -// §10 — Rule expansion -// --------------------------------------------------------------------------- - /// Store deferred order-only edges from groups to a command. /// Groups can't be resolved to edges immediately because group membership /// may grow as more Tupfiles are parsed. Edges are materialized later by @@ -1909,7 +1764,6 @@ auto process_generated_rules( continue; } - // Create edges from inputs to generated command and collect operands auto gen_input_ids = Vec {}; for (auto input_id_val : gen_rule.inputs) { auto input_id = resolve_input_node(ctx, pool.get(input_id_val)); @@ -1919,7 +1773,6 @@ auto process_generated_rules( } } - // Create order-only file edges (group refs filtered out upstream) for (auto oi_id : gen_rule.order_only_inputs) { auto oi = pool.get(oi_id); if (is_order_only_group_reference(oi) || parser::has_glob_chars(oi)) { @@ -1931,15 +1784,10 @@ auto process_generated_rules( } } - // Propagate parent's group dependencies to this generated command add_deferred_group_edges(state, deferred_groups, *gen_cmd_id); - // Add edge from generated command to parent command (dep-scan runs before compile) (void)add_edge(ctx.state->graph, *gen_cmd_id, parent_cmd_id); - // Store generated rule info and operands on the node. - // outputs intentionally left empty: generated rules are dep-scan commands - // whose output is captured via generated_output, not %o expansion. if (auto* node = get_command_node(ctx.state->graph, *gen_cmd_id)) { node->generated_output = gen_rule.outputs.empty() ? GeneratedOutput {} : gen_rule.outputs[0]; node->output_action = gen_rule.action; @@ -2032,25 +1880,18 @@ auto expand_rule( ), }; - // Early macro lookup - needed to process macro's order_only_inputs for demand-driven parsing auto macro_result = lookup_bang_macro(ctx, rule.command); if (!macro_result) { return pup::unexpected(macro_result.error()); } auto macro_ptr = *macro_result; - // Resolve effective fields: merge rule + macro once. - // - command/display: macro wins (it IS the command template) - // - outputs: macro fills in only if rule has none - // - groups: rule wins, macro is fallback - // - order-only inputs: concatenated (handled below via all_order_only) auto const& eff_command = macro_ptr ? macro_ptr->command : rule.command; auto const* eff_display = macro_ptr && macro_ptr->display ? &*macro_ptr->display : rule.display ? &*rule.display : nullptr; auto const& eff_outputs = macro_ptr && rule.outputs.empty() ? macro_ptr->outputs : rule.outputs; - // Unioned, not fallback: a macro's extra outputs are side effects its caller cannot restate (tup parse_bang_rule_internal). auto eff_extra_outputs = rule.extra_outputs; if (macro_ptr) { for (auto const& pattern : macro_ptr->extra_outputs) { @@ -2073,24 +1914,16 @@ auto expand_rule( : macro_ptr && macro_ptr->output_order_only_group_dir ? &*macro_ptr->output_order_only_group_dir : nullptr; - // Pre-resolve order-only group references so % can expand them in commands - // This handles cross-directory groups like: | ../include/ |> cat % - // Stores known group names (sorted); the resolver constructs % on the fly. auto rule_order_only_group_names = SortedIdVec {}; - // Track group NodeIds for deferred edge creation - // Groups are first-class nodes; edges created after all Tupfiles are parsed auto deferred_group_ids = NodeIdMap32 {}; auto deferred_group_vec = Vec {}; - // Merge rule + macro order-only inputs once (reused for group pre-resolution and expansion) auto all_order_only = rule.order_only_inputs; if (macro_ptr && !macro_ptr->order_only_inputs.empty()) { all_order_only.insert(all_order_only.end(), macro_ptr->order_only_inputs.begin(), macro_ptr->order_only_inputs.end()); } - // Check all inputs (regular + order-only) for group references. - // In tup, references are always order-only even when in the inputs section. auto all_inputs = Vec {}; all_inputs.insert(all_inputs.end(), rule.inputs.begin(), rule.inputs.end()); all_inputs.insert(all_inputs.end(), all_order_only.begin(), all_order_only.end()); @@ -2145,9 +1978,6 @@ auto expand_rule( } } - // Override resolve_order_only_group for this rule's command expansion. - // All % patterns are preserved literally for deferred resolution. - // ScopeGuard ensures restoration even on early returns. auto original_resolver = std::move(ctx.eval->resolve_order_only_group); auto resolver_guard = ScopeGuard([&] { ctx.eval->resolve_order_only_group = std::move(original_resolver); }); ctx.eval->resolve_order_only_group = [&rule_order_only_group_names, &deferred_group_ids, &deferred_group_vec, &ctx, &state](std::string_view name @@ -2186,7 +2016,7 @@ auto expand_rule( auto display = StringId::Empty; auto instruction_pattern = Instruction {}; - auto outputs = expand_outputs(ctx, eff_outputs, flags, /*primary=*/nullptr); + auto outputs = expand_outputs(ctx, eff_outputs, flags, nullptr); if (!outputs) { return pup::unexpected(outputs.error()); } @@ -2196,8 +2026,6 @@ auto expand_rule( return pup::unexpected(extra_outputs.error()); } - // Expand command with actual outputs for %o substitution. - // Also capture instruction (after variable expansion, before pattern substitution). auto cmd_result = expand_command(ctx, eff_command, flags, *outputs, &instruction_pattern); if (!cmd_result) { return pup::unexpected(cmd_result.error()); @@ -2265,7 +2093,6 @@ auto expand_rule( .working_dir = intern(str(ctx.current_dir)), }; - // Use scanner_registry (new modular approach) if available, fall back to pattern_registry auto generated_rules = Vec {}; if (ctx.options.scanner_registry && !ctx.options.scanner_registry->empty()) { auto tokens = tokenize_command(cmd_text); @@ -2276,7 +2103,6 @@ auto expand_rule( process_generated_rules(ctx, state, generated_rules, *cmd_id, deferred_group_vec); - // Create edges from inputs to command and collect operand NodeIds auto input_ids = Vec {}; for (auto const& inp : operands) { if (inp.kind == RuleInput::Kind::GroupRef) { @@ -2298,7 +2124,6 @@ auto expand_rule( input_ids.push_back(*input_id); } - // Create edges from command to outputs and collect operand NodeIds auto output_ids = Vec {}; auto output_operand_tokens = Vec {}; auto all_declared = Vec {}; @@ -2312,9 +2137,7 @@ auto expand_rule( auto const primary_count = outputs->ids.size(); for (auto i = std::size_t { 0 }; i < all_declared.size(); ++i) { auto output_path = all_declared[i]; - // An extra output is owned exactly like a primary, minus the %o operands and {group} (tup.1 extra-outputs). auto const is_extra = i >= primary_count; - // Generated means "produced by a rule this configuration runs" (#386). auto output_id = ensure_file_node( ctx.state->graph, output_path, is_context_active(ctx) ? NodeType::Generated : NodeType::Ghost ); @@ -2322,14 +2145,10 @@ auto expand_rule( return pup::unexpected(output_id.error()); } - // Check for duplicate output - another command already produces this file auto output_inputs = get_inputs(ctx.state->graph, *output_id); if (!output_inputs.empty()) { for (auto existing_id : output_inputs) { if (node_id::is_command(existing_id)) { - // Check if this is a phi-node case (complementary guards) - // Allow multiple commands producing the same output if they have - // mutually exclusive guards (same condition, opposite polarity) auto const* existing_cmd = get_command_node(ctx.state->graph, existing_id); if (existing_cmd && are_guards_mutually_exclusive(existing_cmd->guards, ctx.condition_stack)) { continue; @@ -2356,13 +2175,11 @@ auto expand_rule( output_operand_tokens.push_back(outputs->tokens[i]); } - // Add to output group {name} if specified if (eff_output_group && !is_extra && is_context_active(ctx)) { auto gkey = to_underlying(*eff_output_group); ctx.groups.get_or_create(gkey).push_back(*output_id); } - // Add to order-only group if specified if (eff_output_oo_group && is_context_active(ctx)) { auto dir = StringId::Empty; @@ -2379,7 +2196,6 @@ auto expand_rule( auto group_id_result = get_or_create_group_node(ctx, state, str(dir), str(*eff_output_oo_group)); if (group_id_result) { - // Add edge: file → group (file is member of group) (void)add_edge(ctx.state->graph, *output_id, *group_id_result, LinkType::Group); } } @@ -2404,7 +2220,6 @@ auto expand_rule( order_only_ids.push_back(*file_id); } - // Store explicit operands on the command node for expand_instruction() if (auto* cmd = get_command_node(ctx.state->graph, *cmd_id)) { cmd->inputs = TokenList::grouped(std::move(input_ids), operand_tokens, token_count); cmd->order_only_inputs = TokenList::grouped( @@ -2415,8 +2230,6 @@ auto expand_rule( ); } - // Create order-only edges from the pre-expanded paths - // Skip group references (deferred edge creation) and glob patterns (not valid paths) for (auto oi : macro_order_only_paths) { auto oi_sv = str(oi); if (is_order_only_group_reference(oi_sv) || parser::has_glob_chars(oi_sv)) { @@ -2466,20 +2279,13 @@ auto process_rule( parser::Rule const& rule ) -> Result { - // Apply any pending weak assignments (??=) before expanding commands - // This ensures ??= assignments that precede rules take effect apply_pending_weak_assignments(ctx, state); - // Expand input patterns auto inputs = Result { expand_inputs(ctx, rule.inputs) }; if (!inputs) { return pup::unexpected(inputs.error()); } - // Skip rules where input pattern evaluated to empty (tup behavior) - // - rule.inputs.empty() means no input pattern was specified (": |> cmd") - // - inputs->empty() means the pattern(s) evaluated to no files - // Only skip if pattern was specified but produced nothing prune_duplicate_inputs(inputs->operands); auto const only_patterns = std::ranges::all_of(inputs->operands, [](RuleInput const& inp) { @@ -2500,7 +2306,6 @@ auto process_rule( } } - // Foreach rule: create one command per file, include patterns for %g for (auto const& file : files) { auto iter_inputs = patterns; iter_inputs.push_back(file); @@ -2510,7 +2315,6 @@ auto process_rule( } } } else { - // Normal rule: single command for all inputs auto result = expand_rule(ctx, state, rule, inputs->operands, inputs->token_count); if (!result) { return pup::unexpected(result.error()); @@ -2566,11 +2370,7 @@ auto process_statement( return {}; } -} // anonymous namespace - -// --------------------------------------------------------------------------- -// §11 — Public API -// --------------------------------------------------------------------------- +} auto make_builder(BuilderOptions opts) -> Builder { @@ -2586,10 +2386,6 @@ auto add_tupfile( Builder& state ) -> Result { - // Compute current_dir relative to config_root (where Tupfiles live) - // In 3-tree builds, config_root differs from source_root, but the directory - // structure mirrors the source tree, so this relative path is used for both - // config lookup and source file glob expansion. auto const& tupfile_root = is_empty(state.options.config_root) ? str(state.options.source_root) : str(state.options.config_root); @@ -2608,8 +2404,6 @@ auto add_tupfile( .current_file = tupfile.filename, }; - // Create Tupfile node and add to sticky_sources for dependency tracking - // For 3-tree builds, store relative to config_root (Tupfile's actual location) auto tupfile_rel = global_pool().get(pup::path::relative(tupfile_filename_sv, tupfile_root)); auto tupfile_path_id = ctx.state->graph.paths.intern_path(tupfile_rel, global_pool(), PathId::SourceRoot); auto tupfile_node_result = ensure_file_node(ctx.state->graph, tupfile_path_id, NodeType::File); @@ -2617,11 +2411,7 @@ auto add_tupfile( ctx.sticky_sources.push_back(*tupfile_node_result); } - // Create Variable nodes for fine-grained config dependency tracking - // Each config variable becomes a node so commands only depend on variables they use - // Only create nodes once (first Tupfile); subsequent Tupfiles reuse existing nodes if (eval.config_vars && state.config_var_nodes.empty()) { - // Get config directory for Variable node parent (typically the -B directory) auto config_dir_id = NodeId { 0 }; if (!is_empty(state.options.config_path)) { auto config_parent = pup::path::parent(str(state.options.config_path)); @@ -2661,14 +2451,10 @@ auto add_tupfile( } } - // Create/find virtual $ directory for imported env vars (like tup's env_dt) - // Only initialize once; subsequent Tupfiles reuse existing nodes if (state.env_var_dir_id == INVALID_NODE_ID) { - // Check if $ directory already exists in graph (from same build session) if (auto existing = find_by_dir_name(build_state.graph, NodeId { 0 }, "$")) { state.env_var_dir_id = *existing; } else { - // Create new $ directory under root auto env_dir_node = FileNode { .type = NodeType::Directory, .name = intern("$"), @@ -2681,9 +2467,6 @@ auto add_tupfile( } } - // Toolchain fingerprint: CONFIG_TRACKED_TOOLS lists tools the build's outputs - // depend on that appear in no rule's inputs. Their resolved (path, size, mtime) - // fold into every command's identity, so an in-place tool upgrade rebuilds. if (state.toolchain_node_id == INVALID_NODE_ID && eval.config_vars) { auto tracked = eval.config_vars->get("TRACKED_TOOLS"); if (!tracked.empty()) { @@ -2710,18 +2493,12 @@ auto add_tupfile( ctx.sticky_sources.push_back(state.toolchain_node_id); } - // Thread string pool into EvalContext for StringId lookups eval.string_pool = &global_pool(); - // Set up callback to track which config variables are used during expansion eval.on_config_var_used = [&ctx](std::string_view name) { ctx.used_config_vars.insert(to_underlying(intern(name))); }; - // Set up callback to track which imported env variables are used during expansion. - // Also ensure an env Variable node exists for any env var that resolves from the - // environment, so TUP_PLATFORM/TUP_ARCH (auto-resolved, not via `import`) get the - // same tracking node as explicitly imported vars and fold into command identity. eval.imported_vars = &state.imported_var_names; eval.on_env_var_used = [&ctx, &state](std::string_view name) { ctx.used_env_vars.insert(to_underlying(intern(name))); @@ -2732,11 +2509,9 @@ auto add_tupfile( } }; - // Wire up transitive dependency trackers for variable tracking eval.var_config_deps = &state.var_config_deps; eval.var_env_deps = &state.var_env_deps; - // Set up resolve_group callback for {group} pattern expansion eval.resolve_group = [&ctx](std::string_view name ) -> Vec { auto gkey = to_underlying(intern(name)); @@ -2754,9 +2529,6 @@ auto add_tupfile( return paths; }; - // Set up resolve_order_only_group callback for % pattern expansion in commands - // This is for local group references (no directory prefix) - uses current directory - // Groups are first-class nodes; lookup via graph edges (file → group) eval.resolve_order_only_group = [&ctx, &state](std::string_view name ) -> Vec { auto dir = is_empty(ctx.current_dir) ? std::string_view { "." } : str(ctx.current_dir); @@ -2794,10 +2566,8 @@ auto add_tupfile( } } - // Apply pending weak assignments (??=) - last wins apply_pending_weak_assignments(ctx, state); - // Copy errors and warnings for (auto& err : ctx.errors) { state.errors.push_back(std::move(err)); } @@ -2914,9 +2684,6 @@ auto finalize_graph( { auto& g = build_state.graph; - // Pass 1: Create graph edges and accumulate members per (command, group_name). - // Same-named groups from different directories contribute to the same replacement. - // Key: packed (command_id << 32 | interned_group_name) auto pack_key = [](NodeId cmd, std::uint32_t name_id) -> std::uint64_t { return (static_cast(cmd) << 32) | name_id; }; @@ -2954,7 +2721,6 @@ auto finalize_graph( } } - // Pass 2: Replace % patterns with the full accumulated member lists. for (auto const& [key, members] : accumulated) { auto command_id = static_cast(key >> 32); auto name_id = static_cast(key & 0xFFFFFFFF); @@ -3042,13 +2808,10 @@ auto finalize_graph( state.deferred_edges.clear(); - // The configure pass runs before tup.config exists, so rendered text does not yet - // distinguish anything and a key collision there means nothing; it schedules only the - // config-generating rules anyway. Last, because pass 2 rewrites command text. if (!state.options.reject_empty_commands) { return {}; } return reject_ambiguous_keys(build_state); } -} // namespace pup::graph +} diff --git a/src/graph/dag.cpp b/src/graph/dag.cpp index a7730ecc..8ce148ae 100644 --- a/src/graph/dag.cpp +++ b/src/graph/dag.cpp @@ -26,7 +26,6 @@ namespace pup::graph { -// Forward declarations for internal node accessors (no longer in public header) auto get_file_node(Graph const& graph, NodeId id) -> FileNode const*; auto get_file_node(Graph& graph, NodeId id) -> FileNode*; auto get_command_node(Graph const& graph, NodeId id) -> CommandNode const*; @@ -38,22 +37,18 @@ auto make_graph() -> Graph { auto graph = Graph {}; - // Reserve BUILD_ROOT_ID (1) for the build root node. - // All Generated/Ghost nodes will be parented under this node. - // The build root's filesystem location is determined at build time - // (source_root for in-tree, output_root for variant builds). - graph.files.resize(2); // Index 0 unused, index 1 = build root + graph.files.resize(2); graph.dir_children.resize(2); graph.files[1] = FileNode { .id = BUILD_ROOT_ID, .type = NodeType::Directory, - .name = StringId::Empty, // Name set by set_build_root_name() + .name = StringId::Empty, .parent_dir = SOURCE_ROOT_ID, .path_id = PathId::BuildRoot, }; graph.path_to_node.insert(to_underlying(PathId::SourceRoot), SOURCE_ROOT_ID); graph.path_to_node.insert(to_underlying(PathId::BuildRoot), BUILD_ROOT_ID); - graph.next_file_id = 2; // Start regular nodes at ID 2 + graph.next_file_id = 2; return graph; } @@ -96,9 +91,6 @@ auto add_file_node(Graph& graph, FileNode node) -> Result auto const id = graph.next_file_id++; node.id = id; - // Populate path_id from parent's path_id + this node's name. - // Nodes under BUILD_ROOT_ID get BuildRoot-grounded PathIds. - // Nodes under SOURCE_ROOT_ID (0) get SourceRoot-grounded PathIds. if (!is_empty(node.name)) { auto parent_path = PathId::SourceRoot; if (node.parent_dir != 0) { @@ -128,8 +120,6 @@ auto add_file_node(Graph& graph, FileNode node) -> Result auto ensure_file_node(Graph& graph, PathId path_id, NodeType type) -> Result { - // Recursion terminates at a root sentinel. - // BuildRoot/Ungrounded → BUILD_ROOT_ID, SourceRoot → SOURCE_ROOT_ID. if (is_root(path_id)) { return path_id == PathId::SourceRoot ? SOURCE_ROOT_ID : BUILD_ROOT_ID; } @@ -145,9 +135,6 @@ auto ensure_file_node(Graph& graph, PathId path_id, NodeType type) -> Result Result Result*field : def; } -} // namespace +} template<> auto get(Graph const& graph, NodeId id) -> NodeType @@ -636,7 +620,6 @@ auto node_count(Graph const& graph) -> std::size_t return file_count + cmd_count; } -// Includes all edge types: Normal, Sticky, OrderOnly. auto edge_count(Graph const& graph) -> std::size_t { return graph.edges.size(); @@ -794,8 +777,6 @@ auto set_build_root_name(Graph& graph, std::string_view name) -> void if (!is_empty(name_id)) { graph.dir_children[0].insert(to_underlying(name_id), BUILD_ROOT_ID); } - // path_id stays PathId::BuildRoot (set in make_graph). - // The name is for display only (get_full_path), not for PathId identity. } auto get_build_root_name(Graph const& graph) -> std::string_view @@ -810,7 +791,7 @@ auto path_extension(std::string_view name) -> std::string_view return pup::path::bare_extension(name); } -} // namespace +} /// Core expansion logic parameterized on the path resolver. template @@ -1082,14 +1063,9 @@ auto compute_command_key(Graph const& graph, NodeId cmd_id, PathCache& cache) -> state = sha256_update(state, pool.get(expand_instruction(graph, cmd_id, cache))); - // Command text is Tupfile-relative, so the same rule in sibling directories renders - // identically; without the directory those distinct rules share one key. state = sha256_update(state, std::span { &SEP, 1 }); state = sha256_update(state, pool.get(get(graph, cmd_id))); - // A dep-scan command is output-less and drops its parent's -o, so two compiles of one - // source with equal flags render byte-identical scans; whose deps they inject is the - // only thing that tells them apart. One level: a parent is rule-authored, so has none. if (auto parent = get_parent_command(graph, cmd_id); parent != INVALID_NODE_ID) { auto parent_key = compute_command_key(graph, parent, cache); state = sha256_update(state, std::span { &SEP, 1 }); @@ -1105,15 +1081,11 @@ auto compute_command_signature(Graph const& graph, NodeId cmd_id, PathCache& cac auto state = sha256_init(); auto constexpr SEP = std::byte { 0 }; - // Base: the fully-expanded command text (instruction + operand paths + in-text vars). state = sha256_update(state, pool.get(expand_instruction(graph, cmd_id, cache))); state = sha256_update(state, std::span { &SEP, 1 }); state = sha256_update(state, pool.get(get(graph, cmd_id))); - // An output the text never names -- every extra output, a %o-less primary -- would otherwise - // leave identity unchanged when the declaration changes; sorted for insertion independence. - // A dep-scan command's data-flow edge to its parent shares this walk and names no file. auto output_paths = Vec {}; for (auto out_id : get_outputs(graph, cmd_id)) { auto path = get_full_path(graph, out_id, cache); @@ -1128,11 +1100,6 @@ auto compute_command_signature(Graph const& graph, NodeId cmd_id, PathCache& cac state = sha256_update(state, path); } - // Fold in (name, value-hash) of each Variable node reached via a Sticky edge. - // This captures vars that affect output without appearing in the rendered text — - // exported env vars the subprocess reads as $VAR, config vars gating an export, etc. - // Sorted and deduped by name so identity is independent of edge insertion order and - // tolerant of duplicate sticky edges (the graph permits them). auto vars = Vec> {}; for (auto var_id : edges_where(graph, cmd_id, EdgeDirection::Backward, edge_mask::sticky)) { if (get(graph, var_id) != NodeType::Variable) { @@ -1155,10 +1122,6 @@ auto compute_command_signature(Graph const& graph, NodeId cmd_id, PathCache& cac return sha256_finalize(state); } -// ============================================================================= -// BuildGraph free functions -// ============================================================================= - auto make_build_graph() -> BuildGraph { return BuildGraph { .graph = make_graph(), .path_cache = {} }; @@ -1170,10 +1133,6 @@ auto set_build_root_name(BuildGraph& state, std::string_view name) -> void graph::clear_path_cache(state.path_cache); } -// ============================================================================= -// Graph algorithms (moved from scheduler — these operate on Graph, not Scheduler) -// ============================================================================= - auto collect_required_commands(Graph const& graph, Vec const& target_ids) -> NodeIdMap32 { auto visited = NodeIdMap32 {}; @@ -1268,7 +1227,6 @@ auto collect_affected_commands( } }); - // A contradictory pair only adds a command here: the set only grows, so no cycle check. for (auto const *hop = std::lower_bound(discovered_consumers.begin(), discovered_consumers.end(), id, [](auto const& h, NodeId k) { return h.first < k; }); hop != discovered_consumers.end() && hop->first == id; ++hop) { @@ -1279,18 +1237,10 @@ auto collect_affected_commands( } } - // Joined after the cascade, not before: a forced command runs, but nothing about it - // says its consumers must, and seeding it upstream would say exactly that. for (auto id : forced) { affected.set(id, 1); } - // InjectImplicitDeps siblings (dep-scan commands) have no graph outputs, - // so the cascade above can't reach them. They must run whenever their - // parent compile runs — otherwise newly-introduced transitive includes - // are never re-discovered, and the parent's run drops the recorded ones - // with nothing to re-report them (#228). Walk all commands and attach any - // whose parent_command was just marked affected. for (auto cmd_id : nodes_of_type(graph, NodeType::Command)) { auto parent = get_parent_command(graph, cmd_id); if (parent != INVALID_NODE_ID && affected.contains(parent)) { @@ -1364,7 +1314,7 @@ auto walk_upstream_from_scope( return result; } -} // anonymous namespace +} auto collect_scope_with_upstream_commands( Graph const& graph, @@ -1404,4 +1354,4 @@ auto collect_upstream_files( return upstream; } -} // namespace pup::graph +} diff --git a/src/index/entry.cpp b/src/index/entry.cpp index 44f264d4..ede50e02 100644 --- a/src/index/entry.cpp +++ b/src/index/entry.cpp @@ -224,20 +224,17 @@ auto Index::compute_paths() -> void return; } - // File IDs are 1-based contiguous: files_[i].id == i + 1. - // Use direct indexing instead of a hash map. assert(files_[0].id == 1 && "compute_paths requires 1-based contiguous IDs"); assert(files_[n - 1].id == static_cast(n) && "compute_paths requires 1-based contiguous IDs"); auto computed = Vec {}; computed.resize(n); - auto chain = Vec {}; // reusable ancestor stack + auto chain = Vec {}; for (std::size_t i = 0; i < n; ++i) { if (computed[i]) { continue; } - // Walk the parent chain upward, collecting unresolved ancestors chain.clear(); auto idx = i; for (;;) { @@ -255,7 +252,6 @@ auto Index::compute_paths() -> void break; } - // Cycle detection: check if parent_idx is already in our chain auto is_cycle = false; for (auto a : chain) { if (a == parent_idx) { @@ -269,7 +265,6 @@ auto Index::compute_paths() -> void idx = parent_idx; } - // Resolve paths top-down (chain is bottom-up, so iterate in reverse) auto& pool = global_pool(); for (auto it = chain.rbegin(); it != chain.rend(); ++it) { auto& file = files_[*it]; @@ -444,7 +439,7 @@ class IndexSite final { std::string_view m_source_to_root; }; -} // namespace +} auto get_command_string(Index const& index, CommandEntry const& cmd) -> StringId { @@ -492,4 +487,4 @@ auto files_by_path(Index const& index) -> FilesByPath return result; } -} // namespace pup::index +} diff --git a/src/index/reader.cpp b/src/index/reader.cpp index 7daa2c6f..b48a2267 100644 --- a/src/index/reader.cpp +++ b/src/index/reader.cpp @@ -81,8 +81,6 @@ auto declared_layout_fits(std::size_t file_size, RawHeader const& hdr) -> bool && fits(hdr.edge_offset, std::uint64_t { hdr.edge_count } * sizeof(RawEdge)) && fits(hdr.operand_table_offset, std::uint64_t { hdr.command_count } * sizeof(std::uint32_t)) && fits(hdr.string_offset, hdr.string_table_size) - // Operand data is the only section whose size the header does not carry: it runs from - // its own offset to the string table. && hdr.operand_data_offset <= hdr.string_offset; } @@ -99,7 +97,6 @@ auto open_index_in_window(std::string_view path, std::uint32_t min_version) -> R result.file = std::move(*file_result); - // Damage before version: neither a sub-header file nor foreign magic is a record of any version (#383). if (result.file.size() < sizeof(RawHeader) + sizeof(RawFooter)) { return make_error(ErrorCode::IndexDamaged, "Index file too small"); } @@ -114,8 +111,6 @@ auto open_index_in_window(std::string_view path, std::uint32_t min_version) -> R if (!declared_layout_fits(result.file.size(), *hdr)) { return make_error(ErrorCode::IndexDamaged, "Index sections do not fit the file"); } - // What makes the bytes worth reading at all: that they are the ones a putup wrote, not a file - // that merely starts with the right four and lays its sections out plausibly (#294). if (!index_verify_checksum(result)) { return make_error(ErrorCode::IndexChecksumMismatch, "Build record failed its checksum"); } @@ -123,7 +118,7 @@ auto open_index_in_window(std::string_view path, std::uint32_t min_version) -> R return result; } -} // namespace +} auto open_index(std::string_view path) -> Result { @@ -138,14 +133,11 @@ auto read_index(IndexFile const& f) -> Result auto index = Index {}; - // Read save_time_ns from header auto const* hdr = index_header(f); if (hdr) { index.set_save_time_ns(hdr->save_time_ns); } - // A field this read cannot reproduce faithfully makes the whole record unreadable rather than - // a weaker claim in its place: an empty name or operand set is something callers act on (#381). auto files = index_raw_files(f); for (auto i = std::size_t { 0 }; i < files.size(); ++i) { auto const& raw = files[i]; @@ -160,10 +152,8 @@ auto read_index(IndexFile const& f) -> Result index.add_file(*entry); } - // Compute paths from parent chain (after all files loaded) index.compute_paths(); - // Read command entries (v8: instruction + operands) auto commands = index_raw_commands(f); for (auto i = std::size_t { 0 }; i < commands.size(); ++i) { auto const& raw = commands[i]; @@ -193,7 +183,6 @@ auto read_index(IndexFile const& f) -> Result index.add_command(std::move(*command)); } - // Read edges auto edges = index_raw_edges(f); for (auto const& raw : edges) { auto edge = EdgeEntry::from_raw(raw); @@ -203,7 +192,6 @@ auto read_index(IndexFile const& f) -> Result index.add_edge(*edge); } - // Build edge indices for O(1) lookup index.build_edge_indices(); return index; @@ -226,8 +214,6 @@ auto prior_paths(Index const& index) -> PriorPaths if (pup::is_empty(file.path)) { continue; } - // Total by construction: a reader that decides whether to overwrite a file must not - // consume a projection that drops an entry type silently (#389). switch (file.type) { case NodeType::File: result.sources.push_back(file.path); @@ -254,11 +240,6 @@ auto prior_paths(Index const& index) -> PriorPaths std::sort(result.generated.begin(), result.generated.end(), pup::handle_less); std::sort(result.unowned.begin(), result.unowned.end(), pup::handle_less); - // Each entry feeds one list, so a path appearing twice -- in one list or across two -- is a - // record holding two entries for one path, the state its own writer forbids, and this is the - // last check available before an irreversible action (#382). Compared as stored: an - // out-of-tree record legitimately holds one file as "a/bar.txt" and "build/a/bar.txt", which - // stripping a prefix would collapse. auto const repeats = [](Vec const& paths) { return std::adjacent_find(paths.begin(), paths.end()) != paths.end(); }; @@ -298,9 +279,6 @@ auto read_prior_paths(std::string_view path) -> PriorPaths return lost; } - // The file table and nothing else. Everything a version bump invalidates -- command - // identities, signatures, recorded currency -- stays unread, so it cannot reach a caller - // through this return type. auto recorded = Index {}; auto raw = index_raw_files(*file); for (auto i = std::size_t { 0 }; i < raw.size(); ++i) { @@ -354,25 +332,20 @@ auto index_get_semantic_string(IndexFile const& f, std::uint32_t offset) -> Resu return make_error(ErrorCode::IndexDamaged, "Index header unreadable"); } - // The string table's own end, not the file's: a string declared past the table but inside the - // file is still a string this record does not contain. auto const table_end = std::size_t { hdr->string_offset } + hdr->string_table_size; auto const string_start = std::size_t { hdr->string_offset } + offset; - // Length-prefixed strings: if (string_start + sizeof(std::uint16_t) > table_end) { return make_error(ErrorCode::IndexDamaged, "Recorded string starts outside the string table"); } auto data = std::span { f.file.data(), f.file.size() }; - // Read u16 length (little-endian) auto const* len_bytes = data.subspan(string_start, sizeof(std::uint16_t)).data(); auto const length = static_cast( static_cast(len_bytes[0]) | (static_cast(len_bytes[1]) << 8) ); - // Offset 0 is the table's empty entry, so this is a value the record states, not a failure. if (length == 0) { return std::string_view {}; } @@ -406,7 +379,6 @@ auto index_get_operands(IndexFile const& f, std::size_t cmd_index) -> Result { f.file.data(), f.file.size() }; - // Read offset from operand table auto table_pos = std::size_t { hdr->operand_table_offset } + cmd_index * sizeof(std::uint32_t); if (table_pos + sizeof(std::uint32_t) > f.file.size()) { return make_error(ErrorCode::IndexDamaged, "Operand table entry outside the file"); @@ -424,9 +396,6 @@ auto index_get_operands(IndexFile const& f, std::size_t cmd_index) -> Resultstring_offset }; auto record_pos = std::size_t { hdr->operand_data_offset } + offset; if (record_pos + OPERAND_RECORD_HEAD_WORDS * sizeof(std::uint32_t) > section_end) { @@ -480,7 +449,7 @@ auto index_get_operands(IndexFile const& f, std::size_t cmd_index) -> Result bool { @@ -499,4 +468,4 @@ auto index_verify_checksum(IndexFile const& f) -> bool return computed == footer->checksum; } -} // namespace pup::index +} diff --git a/src/index/writer.cpp b/src/index/writer.cpp index 5cdce8fa..d5effce8 100644 --- a/src/index/writer.cpp +++ b/src/index/writer.cpp @@ -65,7 +65,6 @@ class StringTable final { auto StringTable::add(std::string_view str) -> Result { - // Empty strings get offset 0, which has a zero-length entry if (str.empty()) { if (data_.empty()) { data_.push_back(0); @@ -74,27 +73,23 @@ auto StringTable::add(std::string_view str) -> Result return 0; } - // Check for duplicate via StringPool auto str_id = pool_.intern(str); auto const* existing = offsets_.find(to_underlying(str_id)); if (existing) { return *existing; } - // Validate string length fits in u16 if (str.size() > MAX_STRING_LENGTH) { return make_error( ErrorCode::InvalidArgument, "String exceeds 64KB limit" ); } - // Ensure we have the empty string entry at offset 0 if (data_.empty()) { data_.push_back(0); data_.push_back(0); } - // Check table size limit (u32 offset max) auto const entry_size = sizeof(std::uint16_t) + str.size(); if (data_.size() > MAX_U32 - entry_size) { return make_error( @@ -104,12 +99,10 @@ auto StringTable::add(std::string_view str) -> Result auto offset = static_cast(data_.size()); - // Write length prefix (u16, little-endian) auto const length = static_cast(str.size()); data_.push_back(static_cast(length & 0xFF)); data_.push_back(static_cast(length >> 8)); - // Write string data data_.insert(data_.end(), str.begin(), str.end()); offsets_.insert(to_underlying(str_id), offset); @@ -145,7 +138,7 @@ auto build_header( }; } -} // namespace +} auto write_index( std::string_view path, @@ -162,7 +155,6 @@ auto write_index( auto serialize_index(Index const& index) -> Result> { - // Load derives ids from position, so a gap silently rewires every later edge. for (auto i = std::size_t { 0 }; i < index.files().size(); ++i) { if (index.files()[i].id != static_cast(i + 1)) { return make_error>( @@ -182,7 +174,6 @@ auto serialize_index(Index const& index) -> Result> auto strings = StringTable {}; - // Identity is key/signature, not this text (#360): mark and shorten rather than fail the record. auto add_display = [&strings](std::string_view text) -> Result { if (text.size() <= MAX_STRING_LENGTH) { return strings.add(text); @@ -196,7 +187,6 @@ auto serialize_index(Index const& index) -> Result> return strings.add(projected.view().substr(0, MAX_STRING_LENGTH)); }; - // Build file entries and collect strings auto file_entries = Vec {}; file_entries.reserve(index.files().size()); @@ -209,8 +199,6 @@ auto serialize_index(Index const& index) -> Result> file_entries.push_back(file.to_raw(*name_offset)); } - // Build command entries and collect strings - // v8: Use instruction_pattern instead of fully-expanded command auto command_entries = Vec {}; command_entries.reserve(index.commands().size()); @@ -230,12 +218,10 @@ auto serialize_index(Index const& index) -> Result> command_entries.push_back(cmd.to_raw(*instruction_offset, *display_offset, *env_offset)); } - // v8: Build operand offset table and operand data auto operand_table = Vec {}; auto operand_data = Vec {}; operand_table.reserve(index.commands().size()); - // The 4GB guard below is the only bound on these, and only while each counted operand costs >=1 byte of this stream (#365). auto push_u32 = [&operand_data](std::uint32_t value) { operand_data.push_back(static_cast(value & 0xFF)); operand_data.push_back(static_cast((value >> 8) & 0xFF)); @@ -273,7 +259,6 @@ auto serialize_index(Index const& index) -> Result> } } - // Build edge entries auto edge_entries = Vec {}; edge_entries.reserve(index.edges().size()); @@ -281,12 +266,10 @@ auto serialize_index(Index const& index) -> Result> edge_entries.push_back(edge.to_raw()); } - // Canonical order: edge bytes must not depend on job-completion order (#120). std::sort(edge_entries.begin(), edge_entries.end(), [](RawEdge const& a, RawEdge const& b) { return std::tie(a.from_id, a.to_id, a.type) < std::tie(b.from_id, b.to_id, b.type); }); - // Calculate offsets (all u32, check for overflow) auto const file_size_64 = file_entries.size() * sizeof(RawFileEntry); auto const command_size_64 = command_entries.size() * sizeof(RawCommandEntry); auto const edge_size_64 = edge_entries.size() * sizeof(RawEdge); @@ -318,42 +301,34 @@ auto serialize_index(Index const& index) -> Result> auto const footer_offset = string_offset + string_size; auto const total_size = static_cast(total_size_64); - // Get current time for racy-clean detection auto now = pup::SystemClock::now(); auto save_time_ns = std::chrono::duration_cast( now.time_since_epoch() ) .count(); - // Build header auto header = build_header( index, strings, file_offset, command_offset, edge_offset, operand_table_offset, operand_data_offset, string_offset, save_time_ns ); - // Allocate result buffer auto result = Vec {}; result.resize(total_size); auto output = std::span { result }; - // Write header std::memcpy(output.subspan(0, sizeof(header)).data(), &header, sizeof(header)); - // Write file entries if (!file_entries.empty()) { std::memcpy(output.subspan(file_offset, file_size).data(), file_entries.data(), file_size); } - // Write command entries if (!command_entries.empty()) { std::memcpy(output.subspan(command_offset, command_size).data(), command_entries.data(), command_size); } - // Write edge entries if (!edge_entries.empty()) { std::memcpy(output.subspan(edge_offset, edge_size).data(), edge_entries.data(), edge_size); } - // Write operand offset table (v8) if (!operand_table.empty()) { std::memcpy( output.subspan(operand_table_offset, operand_table_size).data(), @@ -362,7 +337,6 @@ auto serialize_index(Index const& index) -> Result> ); } - // Write operand data (v8) if (!operand_data.empty()) { std::memcpy( output.subspan(operand_data_offset, operand_data_size).data(), @@ -371,12 +345,10 @@ auto serialize_index(Index const& index) -> Result> ); } - // Write string table if (string_size > 0) { std::memcpy(output.subspan(string_offset, string_size).data(), strings.data().data(), string_size); } - // Compute and write checksum auto checksum = sha256(output.first(footer_offset)); auto footer = RawFooter { .checksum = checksum }; std::memcpy(output.subspan(footer_offset, sizeof(footer)).data(), &footer, sizeof(footer)); @@ -384,4 +356,4 @@ auto serialize_index(Index const& index) -> Result> return result; } -} // namespace pup::index +} diff --git a/src/parser/eval.cpp b/src/parser/eval.cpp index 1206b18d..eaaf6ed3 100644 --- a/src/parser/eval.cpp +++ b/src/parser/eval.cpp @@ -28,10 +28,6 @@ namespace pup::parser { -// ============================================================================= -// VarDb -// ============================================================================= - VarDb::VarDb(VarDb const& other) { for (auto [key, value] : other.entries_) { @@ -115,13 +111,8 @@ auto VarDb::clear() -> void entries_.clear(); } -// ============================================================================= -// VarContext lookup functions -// ============================================================================= - auto lookup_var_with_bank(VarContext const& ctx, std::string_view name, VarRef::Kind kind) -> VarLookupResult { - // Context-computed special variables (highest priority) - NOT overridable if (name == builtin_vars::TUP_CWD && !ctx.tup_cwd.empty()) { return { ctx.tup_cwd, VarBank::Builtin }; } @@ -138,17 +129,13 @@ auto lookup_var_with_bank(VarContext const& ctx, std::string_view name, VarRef:: return { ctx.tup_outdir, VarBank::Builtin }; } - // TUP_PLATFORM and TUP_ARCH: env > config > context > default - // For Config kind, env still takes priority, then config is checked. if (name == builtin_vars::TUP_PLATFORM) { if (auto const* env = pup::platform::get_env("TUP_PLATFORM"); env && *env) { return { std::string_view { env }, VarBank::Env }; } - // For Config kind or if config has the value, use config if (ctx.config && ctx.config->contains(name)) { return { ctx.config->get(name), VarBank::Config }; } - // Only use context value for Regular kind (not Config) if (kind != VarRef::Kind::Config && !ctx.tup_platform.empty()) { return { ctx.tup_platform, VarBank::Builtin }; } @@ -158,18 +145,15 @@ auto lookup_var_with_bank(VarContext const& ctx, std::string_view name, VarRef:: if (auto const* env = pup::platform::get_env("TUP_ARCH"); env && *env) { return { std::string_view { env }, VarBank::Env }; } - // For Config kind or if config has the value, use config if (ctx.config && ctx.config->contains(name)) { return { ctx.config->get(name), VarBank::Config }; } - // Only use context value for Regular kind (not Config) if (kind != VarRef::Kind::Config && !ctx.tup_arch.empty()) { return { ctx.tup_arch, VarBank::Builtin }; } return { std::string_view { pup::ARCH }, VarBank::Builtin }; } - // Kind-specific lookup switch (kind) { case VarRef::Kind::Config: if (ctx.config && ctx.config->contains(name)) { @@ -184,9 +168,7 @@ auto lookup_var_with_bank(VarContext const& ctx, std::string_view name, VarRef:: break; case VarRef::Kind::Regular: - // Regular variables have priority over config if (ctx.vars && ctx.vars->contains(name)) { - // Check if this is an imported env var if (ctx.imported_vars && ctx.string_pool) { auto id = ctx.string_pool->find(name); if (!pup::is_empty(id) && ctx.imported_vars->contains(pup::to_underlying(id))) { @@ -195,7 +177,6 @@ auto lookup_var_with_bank(VarContext const& ctx, std::string_view name, VarRef:: } return { ctx.vars->get(name), VarBank::Regular }; } - // Fall back to config (tup behavior: CONFIG_* accessible via $()) if (ctx.config && ctx.config->contains(name)) { return { ctx.config->get(name), VarBank::Config }; } @@ -210,10 +191,6 @@ auto lookup_var(VarContext const& ctx, std::string_view name, VarRef::Kind kind) return lookup_var_with_bank(ctx, name, kind).value; } -// ============================================================================= -// Internal helper functions -// ============================================================================= - namespace { /// Create VarContext from EvalContext for lookup functions @@ -243,9 +220,6 @@ auto expand_var(EvalContext& ctx, VarRef const& ref) -> Result auto name_sv = pool.get(ref.name); auto [value, bank] = lookup_var_with_bank(var_ctx, name_sv, ref.kind); - // Dependency tracking based on which bank was used. An @() reference is - // a config read even when the variable is undefined — defining it later - // must count as a change. if ((bank == VarBank::Config || ref.kind == VarRef::Kind::Config) && ctx.on_config_var_used) { auto config_name = name_sv; if (config_name.starts_with(builtin_vars::CONFIG_)) { @@ -254,15 +228,11 @@ auto expand_var(EvalContext& ctx, VarRef const& ref) -> Result ctx.on_config_var_used(config_name); } - // TUP_PLATFORM/TUP_ARCH fall back to compiled-in defaults when neither - // env nor config provides them; setting the env var later must count as - // a change, so the fallback still records an env read. if (bank == VarBank::Builtin && ctx.on_env_var_used && (name_sv == builtin_vars::TUP_PLATFORM || name_sv == builtin_vars::TUP_ARCH)) { ctx.on_env_var_used(name_sv); } - // Propagate transitive config var dependencies for regular variables if (ref.kind == VarRef::Kind::Regular && bank == VarBank::Regular && ctx.var_config_deps && ctx.on_config_var_used && ctx.string_pool) { auto name_id = ctx.string_pool->find(name_sv); @@ -276,12 +246,10 @@ auto expand_var(EvalContext& ctx, VarRef const& ref) -> Result } } - // Track imported env variable usage if (bank == VarBank::Env && ctx.on_env_var_used) { ctx.on_env_var_used(name_sv); } - // Propagate transitive env var dependencies for regular variables if (ref.kind == VarRef::Kind::Regular && bank == VarBank::Regular && ctx.var_env_deps && ctx.on_env_var_used && ctx.string_pool) { auto name_id = ctx.string_pool->find(name_sv); @@ -298,11 +266,7 @@ auto expand_var(EvalContext& ctx, VarRef const& ref) -> Result return value ? pool.intern(*value) : StringId::Empty; } -} // namespace - -// ============================================================================= -// Free functions -// ============================================================================= +} auto expand(EvalContext& ctx, Expression const& expr) -> Result { @@ -322,8 +286,6 @@ auto expand(EvalContext& ctx, Expression const& expr) -> Result } } - // Recursively expand any variable references that were embedded in literals - // (e.g., from escaped quotes like \"$(VAR)\") return expand(ctx, buf.view()); } @@ -334,31 +296,25 @@ auto expand(EvalContext& ctx, std::string_view text) -> Result auto pos = std::size_t { 0 }; while (pos < text.size()) { - // Look for variable references auto dollar = text.find('$', pos); auto at = text.find('@', pos); auto amp = text.find('&', pos); - // Find the earliest variable reference auto next = std::min({ dollar, at, amp }); if (next == std::string_view::npos) { - // No more variable references buf.append(text.substr(pos)); break; } - // Add text before the variable buf.append(text.substr(pos, next - pos)); - // Handle $$ escape -> literal $ for shell commands if (text[next] == '$' && next + 1 < text.size() && text[next + 1] == '$') { buf.append('$'); pos = next + 2; continue; } - // Check for variable reference pattern: X(name) if (next + 1 < text.size() && text[next + 1] == '(') { auto close = text.find(')', next + 2); if (close != std::string_view::npos) { @@ -382,7 +338,6 @@ auto expand(EvalContext& ctx, std::string_view text) -> Result } } - // Not a variable reference, just add the character buf.append(text[next]); pos = next + 1; } @@ -501,7 +456,7 @@ struct ParseSite { } }; -} // namespace +} auto expand_pattern_atoms( EvalContext& ctx, @@ -749,30 +704,25 @@ auto expand_path( return result; } - // Expand the path expression auto path_result = expand(ctx, pattern.path); if (!path_result) { return pup::unexpected(path_result.error()); } - // Split result by whitespace - variables may contain multiple files auto expanded = pool.get(*path_result); auto start = std::size_t { 0 }; while (start < expanded.size()) { - // Skip whitespace while (start < expanded.size() && (expanded[start] == ' ' || expanded[start] == '\t')) { ++start; } if (start >= expanded.size()) { break; } - // Find end of token auto end = start; while (end < expanded.size() && expanded[end] != ' ' && expanded[end] != '\t') { ++end; } if (end > start) { - // Normalize path to remove // and resolve . and .. components auto path_str = expanded.substr(start, end - start); result.push_back(pup::path::normalize(path_str)); } @@ -795,7 +745,7 @@ auto record_config_definedness_read(EvalContext& ctx, std::string_view name) -> ctx.on_config_var_used(name); } -} // namespace +} auto evaluate_condition(EvalContext& ctx, Conditional const& cond) -> bool { @@ -843,4 +793,4 @@ auto evaluate_condition(EvalContext& ctx, Conditional const& cond) -> bool return false; } -} // namespace pup::parser +} diff --git a/test/unit/test_e2e.cpp b/test/unit/test_e2e.cpp index 0d721ab1..f33d1a9d 100644 --- a/test/unit/test_e2e.cpp +++ b/test/unit/test_e2e.cpp @@ -27,9 +27,6 @@ using namespace pup::test; -// The assertion most incremental scenarios rest on, so it has to be exact: a substring test -// for "0 commands" also accepts every multiple of ten, and a quiescence check that can pass -// while a build ran hides the very defects those scenarios exist to catch (#234). TEST_CASE("is_noop accepts only a build that ran nothing", "[e2e-fixture]") { auto result_with = [](std::string out) { return PupResult { .exit_code = 0, .stdout_output = std::move(out), .stderr_output = {} }; }; @@ -50,10 +47,6 @@ TEST_CASE("is_noop accepts only a build that ran nothing", "[e2e-fixture]") } } -// ============================================================================= -// Build Verification Tests -// ============================================================================= - SCENARIO("Building a simple C project", "[e2e][build]") { GIVEN("an initialized simple_c project") @@ -83,8 +76,8 @@ SCENARIO("Building a simple C project", "[e2e][build]") WHEN("built again without changes") { - (void)f.build(); // first build - auto result = f.build(); // second build + (void)f.build(); + auto result = f.build(); THEN("nothing is rebuilt") { @@ -324,8 +317,6 @@ SCENARIO("Percent flags inside display text expand against the rule", "[e2e][bui SCENARIO("An unterminated caret is a parse error, not a shell error", "[e2e][build][display]") { - // Upstream rejects this at parse time; falling through left the ^ in the command text, - // so the rule died mid-build as "sh: ^: not found" — blamed on the tool, not the typo (#217). GIVEN("a rule whose display annotation is never closed") { auto f = E2EFixture { "glob_mixed_space" }; @@ -351,8 +342,6 @@ SCENARIO("An unterminated caret is a parse error, not a shell error", "[e2e][bui SCENARIO("An upstream caret flag is rejected, not rendered as a label", "[e2e][build][display]") { - // tup reads the non-space run after ^ as flags (t, o); putup implements neither, and - // printing "t" as the rule's label honours nothing and refuses nothing (#217). GIVEN("a rule using upstream's caret-flag form") { auto f = E2EFixture { "glob_mixed_space" }; @@ -375,7 +364,6 @@ SCENARIO("An upstream caret flag is rejected, not rendered as a label", "[e2e][b SCENARIO("A failing command is reported by its command line, not its display", "[e2e][build][display]") { - // The display names the step, not what broke, and this is the only line a build prints of what actually ran. GIVEN("an annotated rule whose command fails") { auto f = E2EFixture { "glob_mixed_space" }; @@ -427,7 +415,6 @@ SCENARIO("A failing config rule is reported by its command line, not its display SCENARIO("A bang macro's display wins over one written on the rule", "[e2e][build][display][bang]") { - // Outputs and groups resolve rule-over-macro; display is the one field that goes the other way, because the macro owns the command the display names. GIVEN("a macro carrying a display, applied by a rule that also carries one") { auto f = E2EFixture { "glob_mixed_space" }; @@ -511,7 +498,6 @@ SCENARIO("Order-only groups ensure build ordering", "[e2e][groups]") THEN("percent-group pattern expands in command body") { - // % in command body should expand to config.h auto content = f.read_file("headers.txt"); REQUIRE(content.find("config.h") != std::string::npos); } @@ -537,8 +523,6 @@ SCENARIO("Order-only groups ensure build ordering", "[e2e][groups]") WHEN("the generator script is modified to output a new value") { (void)f.build({ "-j1" }); - // Modify gen_config.sh which outputs config.h - // This tests: gen_config.sh -> config.h -> (order-only) -> main.o -> program f.write_file("gen_config.sh", "#!/bin/sh\necho '#define CONFIG_VALUE 99'\n"); auto result = f.build({ "-j1" }); @@ -772,7 +756,6 @@ SCENARIO("Cross-directory order-only groups", "[e2e][groups]") THEN("percent-group expands cross-directory group in command") { - // % should expand to ../include/version.h auto content = f.read_file("src/headers.txt"); REQUIRE(content.find("VERSION") != std::string::npos); } @@ -812,10 +795,6 @@ SCENARIO("Bang macro order-only groups trigger demand-driven parsing", "[e2e][bu { GIVEN("a project with a bang macro that references an order-only group in another directory") { - // This test verifies that order-only group references embedded in bang macros - // correctly trigger demand-driven parsing of the directory containing the group. - // Bug: the group reference in !cc = | $(TOROOT)/include/ |> ... - // was not triggering parsing of include/Tupfile before looking up the group. auto f = E2EFixture { "groups_bang_macro_cross_dir" }; REQUIRE(f.init().success()); @@ -866,12 +845,6 @@ SCENARIO("Bang macro order-only groups work in variant builds", "[e2e][variant][ { GIVEN("a project with a bang macro that references an order-only group across directories") { - // This test verifies that order-only group references in bang macros work correctly - // in variant builds. The bug was that DEP (implicit dep scanning) commands were not - // inheriting the order-only edges from their parent compile commands when using - // bang macros with TOROOT-based group references like: - // !cc = | $(TOROOT)/include/ |> ... - // This caused DEP commands to run before headers were generated. auto f = E2EFixture { "groups_bang_macro_variant" }; f.mkdir("build"); f.write_file("build/tup.config", "CONFIG_CC=gcc\n"); @@ -920,7 +893,6 @@ SCENARIO("Groups defined in included files are visible", "[e2e][groups]") THEN("percent-group expands group from included file in command") { - // % should expand to config.h (defined in gen.tup) auto content = f.read_file("headers.txt"); REQUIRE(content.find("CONFIG_VALUE") != std::string::npos); } @@ -1033,8 +1005,6 @@ SCENARIO("Group references in regular inputs expand correctly", "[e2e][groups]") { GIVEN("a Tupfile with group reference in inputs section (before |)") { - // This tests the spos pattern: $(ROOT)/modules/ |> cat % - // Group references are order-only even when in regular inputs section auto f = E2EFixture { "groups_in_inputs" }; REQUIRE(f.init().success()); @@ -1055,7 +1025,6 @@ SCENARIO("Group references in regular inputs expand correctly", "[e2e][groups]") THEN("percent-group in command expands to all group members") { - // % should expand to mod1.header and mod2.header auto content = f.read_file("output/headers.txt"); REQUIRE(content.find("mod1") != std::string::npos); REQUIRE(content.find("mod2") != std::string::npos); @@ -1140,10 +1109,6 @@ SCENARIO("Dollar-dollar escapes to literal dollar in shell commands", "[e2e][bui } } -// ============================================================================= -// Incremental Build Tests -// ============================================================================= - SCENARIO("Incremental rebuilds detect header changes", "[e2e][incremental]") { GIVEN("a built project with header dependencies") @@ -1276,7 +1241,6 @@ SCENARIO("Editing an output-less command re-runs it", "[e2e][incremental]") SCENARIO("A Tupfile edit that changes no command re-runs nothing", "[e2e][incremental][identity]") { - // Complement of "Editing an output-less command re-runs it". The rule needs an output: the Sticky route propagates a command's outputs, so an output-less one cannot exhibit this at all (#225). GIVEN("a built project whose rebuild is stable") { auto f = E2EFixture { "glob_mixed_space" }; @@ -1517,7 +1481,6 @@ SCENARIO("Fine-grained config variable tracking with indirect usage", "[e2e][inc THEN("rebuild occurs even though command uses $(MYFLAGS) not @(OPT)") { - // THIS IS THE KEY ASSERTION - currently fails REQUIRE(result.success()); REQUIRE_FALSE(result.is_noop()); } @@ -1561,7 +1524,6 @@ SCENARIO("SoftSet with config var only records deps when effective", "[e2e][incr { auto f = E2EFixture { "config_var_softset" }; f.mkdir("build"); - // Pre-set MYFLAGS so ?= is ineffective f.write_file("Tuprules.tup", "MYFLAGS = -DLEVEL=99\n"); f.write_file("build/tup.config", "CONFIG_OPT=1\nCONFIG_OTHER=x\n"); REQUIRE(f.pup({ "configure", "-B", "build" }).success()); @@ -1589,11 +1551,9 @@ SCENARIO("WeakSet with config var records deps only for winning assignment", "[e { auto f = E2EFixture { "config_var_weakset" }; f.mkdir("build"); - // OPT2=5 wins (last ??= assignment), OPT1=1 is ignored f.write_file("build/tup.config", "CONFIG_OPT1=1\nCONFIG_OPT2=5\n"); REQUIRE(f.pup({ "configure", "-B", "build" }).success()); REQUIRE(f.build({ "-B", "build" }).success()); - // Should use OPT2 (last wins) REQUIRE(f.run("build/program").stdout_output == "Level: 5\n"); REQUIRE(f.build({ "-B", "build" }).is_noop()); @@ -1635,8 +1595,6 @@ SCENARIO("Touch does not trigger unnecessary rebuild", "[e2e][incremental]") WHEN("a source file is touched without content change") { - // Touch updates mtime but not content - // Use absolute path to touch since it's not in workdir auto result = f.run("/usr/bin/touch", { "hello.c" }); REQUIRE(result.success()); @@ -1687,8 +1645,6 @@ SCENARIO("Partial failure with -k saves successful outputs", "[e2e][keep-going]" THEN("only the fixed command runs, not the already-successful one") { - // The output should show only bad.c being compiled, not good.c - // If good.c is recompiled, the bug is present REQUIRE(result2.stdout_output.find("good.c") == std::string::npos); REQUIRE(result2.stdout_output.find("bad.c") != std::string::npos); } @@ -1716,8 +1672,6 @@ SCENARIO("A build record that is not putup's own is refused out loud", "[e2e][in } REQUIRE(bytes.size() > 128); - // Mid-file: past the header putup already validates, ahead of the footer, so nothing - // but the checksum can notice. auto const pos = bytes.size() / 2; bytes[pos] = static_cast(bytes[pos] ^ 0x01); { @@ -1732,9 +1686,6 @@ SCENARIO("A build record that is not putup's own is refused out loud", "[e2e][in REQUIRE(result.stderr_output.find("failed its checksum") != std::string::npos); } - // With the record refused, nothing left says which files on disk this project - // produced, and #291's rule is that putup does not guess -- so the build stops and - // names them rather than treating its own outputs as checked-in sources. THEN("it refuses rather than claim outputs it can no longer prove are its own") { REQUIRE_FALSE(result.success()); @@ -1780,8 +1731,6 @@ SCENARIO("A build records one entry per path", "[e2e][index]") REQUIRE(std::ranges::any_of(paths, [](auto p) { return p.starts_with("/"); })); } - // The directory walk creates an entry and registers it for the next lookup; register - // anything but what it created and the next chain re-creates it (#325). THEN("no two entries answer to the same path") { auto const dup = std::adjacent_find(paths.begin(), paths.end()); @@ -1831,8 +1780,6 @@ SCENARIO("Implicit dependencies track header changes", "[e2e][incremental]") SCENARIO("A dependency outside the source tree is recorded rather than dropped", "[e2e][incremental][implicit]") { - // The arms that drop a discovered dep sit above an else that keeps the out-of-tree ones as - // absolute paths. Nothing pinned that, and #305 was filed on the assumption it drops them. GIVEN("a compile whose depfile names system headers") { auto f = E2EFixture { "glob_mixed_space" }; @@ -1865,8 +1812,6 @@ SCENARIO("A dependency outside the source tree is recorded rather than dropped", SCENARIO("A changed header re-runs the output-less command that read it", "[e2e][incremental][implicit]") { - // Routing a discovered dep pushes the reading command's outputs, so a command with none was - // reached and then dropped. A compile gate is the shape that has no outputs on purpose (#228). GIVEN("an output-less compile gate that reads a header") { auto f = E2EFixture { "glob_mixed_space" }; @@ -1908,9 +1853,6 @@ SCENARIO("A changed header re-runs the output-less command that read it", "[e2e] SCENARIO("A scoped build sees a declared input outside the scope change", "[e2e][incremental][scope]") { - // Detection skips out-of-scope files unless a bypass covers them, and the bypass admitted - // Implicit and Sticky edges but not Normal — so a plainly declared source input was the one - // kind of dependency a scoped build could not see (#200). GIVEN("a rule in a subdirectory declaring a source file above it") { auto f = E2EFixture { "glob_mixed_space" }; @@ -1941,9 +1883,6 @@ SCENARIO("A scoped build sees a declared input outside the scope change", "[e2e] SCENARIO("A command that raced its discovered dependency runs again", "[e2e][incremental][implicit]") { - // Nothing orders a consumer against a producer it only discovers, so it can read the file - // before it exists. The dep is then recorded from a post-run stat — as already satisfied — - // and no later build re-runs it, leaving the output permanently wrong (#274). GIVEN("a consumer that discovers a generated file with nothing ordering the two") { auto f = E2EFixture { "glob_mixed_space" }; @@ -1989,9 +1928,6 @@ SCENARIO("A command that raced its discovered dependency runs again", "[e2e][inc SCENARIO("A consumer ordered through a sibling output is not taxed for discovering the other", "[e2e][incremental][implicit]") { - // Codegen emitting one declared and one discovered output: the consumer is ordered through - // the declared one, so it cannot have raced the discovered one. Marking it anyway doubles - // every codegen rebuild — the shape a two-hop ordering check cannot see (#274). GIVEN("a generator producing a source the consumer declares and a header it only discovers") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2022,9 +1958,6 @@ SCENARIO("A consumer ordered through a sibling output is not taxed for discoveri SCENARIO("A discovered dependency orders its consumer on a later build", "[e2e][incremental][implicit]") { - // A discovery is index-only, so it orders nothing: every later build that runs both the - // producer and the consumer races them again, and the consumer is taxed with an extra run - // to catch up. The previous build's discovery is what the scheduler orders by now (#276). GIVEN("a settled project whose consumer only discovers what the other produces") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2075,11 +2008,6 @@ SCENARIO("A discovered dependency orders its consumer on a later build", "[e2e][ SCENARIO("A recorded discovery that the rules now contradict does not stall the build", "[e2e][incremental][implicit]") { - // The ordering carried from the last build is stale by construction: the rules can since - // have turned the discovered file's producer into a consumer of the discoverer's output. - // The rules are this build's truth, so the contradiction retracts the carried ordering — - // taking it as binding leaves both commands waiting for each other, and a scheduler with - // nothing runnable and nothing running reports the build complete having run neither (#276). GIVEN("a settled project where one command discovers what the other produces") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2139,8 +2067,6 @@ SCENARIO("A recorded discovery that the rules now contradict does not stall the SCENARIO("A discovery whose producing rule is gone orders nothing", "[e2e][incremental][implicit]") { - // Ordering carried from the last build names a producer by the file it produces, so a rule - // that has since stopped producing it names nothing and the consumer waits for no one (#276). GIVEN("a settled project whose consumer discovers a generated file") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2193,10 +2119,6 @@ SCENARIO("A discovery whose producing rule is gone orders nothing", "[e2e][incre SCENARIO("Ordering the scheduler did not enforce does not excuse a race", "[e2e][incremental][implicit]") { - // A carried ordering is only real for a pair this build actually scheduled: the consumer may - // be quiescent and absent from the job set, and then nothing enforced it. Crediting it anyway - // lets a command reach its own excuse through the missing one's outputs, and the race it did - // commit goes unmarked — the permanently wrong output #274 exists to prevent (#276). GIVEN("a settled project whose quiescent consumer bridges a producer to a third command") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2266,10 +2188,6 @@ SCENARIO("Ordering the scheduler did not enforce does not excuse a race", "[e2e] SCENARIO("A producer's own input change reaches the consumer that only discovered it", "[e2e][incremental][implicit]") { - // The affected cascade walks graph edges from the pre-build changed set, and a discovered - // dependency has none; the file is not known changed until its producer has run, and the - // index then stamps it from a post-run stat. So the consumer was never scheduled at all and - // its output stayed wrong while the build reported the tree up to date (#277). GIVEN("a settled project whose consumer only discovers what the other produces") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2343,9 +2261,6 @@ SCENARIO("A producer's own input change reaches the consumer that only discovere SCENARIO("A producer's input change reaches a chain of discovered consumers", "[e2e][incremental][implicit]") { - // Routing a discovered consumer makes its own outputs change, so whatever discovered those - // must follow. Expanding the recorded pairs once rather than inside the cascade's fixpoint - // would reach the first consumer and stop (#277). GIVEN("a settled chain where each link only discovers the one before it") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2406,9 +2321,6 @@ SCENARIO("A producer's input change reaches a chain of discovered consumers", "[ SCENARIO("A producer's input change reaches an output-less discovered consumer", "[e2e][incremental][implicit]") { - // The cascade marks the consumer command node itself, so a reader with no output path is - // reached the same way one with outputs is; routing through outputs instead would drop it, - // the shape #228 had on the comparison route (#284). GIVEN("a settled gate that declares no outputs and only discovered the generated header") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2421,8 +2333,6 @@ SCENARIO("A producer's input change reaches an output-less discovered consumer", REQUIRE(f.init().success()); REQUIRE(f.build().success()); - // The gate's dep scan must see the header to record it; introduced together they race - // and the discovery is silently empty. f.write_file("b/Tupfile", ": gate.c |> gcc -c %f -o /dev/null |>\n"); f.write_file( "b/gate.c", @@ -2453,9 +2363,6 @@ SCENARIO("A producer's input change reaches an output-less discovered consumer", SCENARIO("A discovered consumer re-runs for a producer that rewrites the same bytes", "[e2e][incremental][implicit]") { - // Membership routes, not content: the consumer re-runs because its producer ran, exactly as - // a declared consumer does. Pinned so the pessimism is a decision rather than a surprise, and - // so that it costs one run per producer run and not one per build (#277). GIVEN("a settled project whose producer writes constant output from a changing input") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2505,10 +2412,6 @@ SCENARIO("A discovered consumer re-runs for a producer that rewrites the same by SCENARIO("A dependency absent when it was recorded settles rather than re-running forever", "[e2e][incremental][implicit]") { - // A command may report reading a file that is not there -- a conditional include that - // resolved to nothing. Reading the same stat failure as news every build re-runs the command - // forever for output that cannot change, which is the loop the campaign exists to kill. tup - // records the absence as a ghost and settles, and re-runs only if the file appears (#281). GIVEN("a consumer whose dependency report names a file that does not exist") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2538,8 +2441,6 @@ SCENARIO("A dependency absent when it was recorded settles rather than re-runnin SCENARIO("A dependency that was absent when recorded still re-runs its reader when it appears", "[e2e][incremental][implicit]") { - // What settling must not cost: the absence is recorded as zero size, zero mtime and a zero - // hash, and a file arriving has to be read as a change against all three (#281). GIVEN("a settled consumer whose dependency report names a file that does not exist") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2579,7 +2480,6 @@ SCENARIO("A dependency that was absent when recorded still re-runs its reader wh WHEN("the named file appears empty") { - // Size and mtime match the sentinel zeros, so only the recorded hash separates it. f.write_file("h.txt", ""); REQUIRE(f.build().success()); @@ -2605,8 +2505,6 @@ SCENARIO("A dependency that was absent when recorded still re-runs its reader wh SCENARIO("A deleted dependency a command still reports re-runs it once", "[e2e][incremental][implicit]") { - // The deletion is a real change and must reach the reader, but the run that follows records - // the file as absent -- so the build after it has nothing new to say and must settle (#281). GIVEN("a settled consumer whose dependency is then deleted") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2651,9 +2549,6 @@ SCENARIO("A deleted dependency a command still reports re-runs it once", "[e2e][ SCENARIO("A recreated dependency does not carry its deletion mark forward", "[e2e][incremental][implicit]") { - // The merge copies an out-of-scope file's entry verbatim. Run before the discovered deps, - // that copy shadowed the fresh one, so a carried NodeFlags::AbsenceRouted discharged the next - // real deletion as "already routed" and the consumer never ran again (#237). GIVEN("a guarded producer, a consumer that discovers its output, and an out-of-scope declarer") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2672,10 +2567,6 @@ SCENARIO("A recreated dependency does not carry its deletion mark forward", "[e2 ); f.write_file("d/Tupfile", ": ../a/p.txt |> cp %f %o |> o.copy\n"); f.write_file("tup.config", "CONFIG_FOO=y\n"); - // Nothing orders these two commands on a first build — b/ depends on a/p.txt only by - // discovery — so c.o's content here is whichever won, and asserting it raced on CI. - // gen.sh writes the .d either way, so the dependency is recorded regardless, which is - // all the steps below need. REQUIRE(f.build().success()); WHEN("the guard is turned off, the output is recreated by hand, and then deleted again") @@ -2703,7 +2594,6 @@ SCENARIO("A recreated dependency does not carry its deletion mark forward", "[e2 SCENARIO("A build whose discovered dependency was deleted quiesces", "[e2e][incremental][implicit]") { - // The command re-runs and rediscovers nothing, which the carry logic could not tell from "did not run", so the dead edge and its file entry came back every build (#224). GIVEN("a rule whose command reports its own dependencies") { auto f = E2EFixture { "glob_mixed_space" }; @@ -2746,9 +2636,6 @@ SCENARIO("A build whose discovered dependency was deleted quiesces", "[e2e][incr SCENARIO("Implicit deps survive identical rules in sibling directories", "[e2e][incremental][identity]") { - // Command text is Tupfile-relative, so these two rules render the same string. - // If identity ignores the directory they collide, and one directory's header - // edges get attached to the other's command. auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" }; GIVEN("two directories whose rules and sources are byte-identical") @@ -2784,7 +2671,6 @@ SCENARIO("Implicit deps survive identical rules in sibling directories", "[e2e][ SCENARIO("A header the compile reads only under -O2 is tracked", "[e2e][incremental]") { - // A scan without the compile's flags resolves the other branch and records the wrong header. auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" }; GIVEN("a source whose include is gated on __OPTIMIZE__") @@ -2847,7 +2733,6 @@ SCENARIO("A dep scan that prints nothing fails the build", "[e2e][incremental]") SCENARIO("A dep scan that prints anything but its rule fails the build", "[e2e][incremental]") { - // Chatter parsed as dependencies never stats, so the command would re-run for ever. auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" }; GIVEN("a compiler whose scan prints a note before its rule") @@ -2875,7 +2760,6 @@ SCENARIO("A dep scan that prints anything but its rule fails the build", "[e2e][ SCENARIO("Implicit deps survive a flag whose path is a separate word", "[e2e][incremental]") { - // A flag's path reaches the scan whichever spelling carries it, or the scan has no input file. auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" }; GIVEN("a compile whose sysroot is spelled as two words") @@ -2911,7 +2795,6 @@ SCENARIO("Implicit deps survive a flag whose path is a separate word", "[e2e][in SCENARIO("Implicit deps cover every source of a multi-source command", "[e2e][incremental]") { - // gcc -M emits one rule per source; stopping at the first leaves b.h untracked and the program silently stale auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" }; GIVEN("a command compiling two sources with a header each") @@ -2946,13 +2829,6 @@ SCENARIO("Implicit deps cover every source of a multi-source command", "[e2e][in SCENARIO("Implicit deps survive command-id shift from a removed source", "[e2e][incremental][idshift]") { - // Implicit (header→command) edges discovered last build are carried forward for - // commands that don't rebuild. If that carry-forward keys on the command's array - // position (NodeId), removing a glob-matched source whose command was created - // earlier shifts every later command's id down — and the carried edge gets - // misattributed to whatever command now occupies the old id. A later edit to the - // header then rebuilds the wrong unit and leaves the real output stale. The - // carry-forward must key on the command's structural identity, stable across shifts. auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" }; GIVEN("a project where m_a.o has a discovered header dep and m_b.o is a removable unit") @@ -2965,8 +2841,6 @@ SCENARIO("Implicit deps survive command-id shift from a removed source", "[e2e][ WHEN("the earlier-created unit is removed (shifting m_a's command id down), then m_a.h changes") { - // No Tupfile edit: the glob drops m_b.c. m_a.c is not recompiled this build, - // so its m_a.h dependency must be carried forward — at the new command id. f.remove_file("m_b.c"); REQUIRE(f.build().success()); REQUIRE(f.run("program").stdout_output == "A1\n"); @@ -3015,7 +2889,7 @@ SCENARIO("New source file triggers rebuild", "[e2e][incremental]") THEN("the original file is not recompiled") { - REQUIRE_FALSE(result.is_noop()); // mul.o was built + REQUIRE_FALSE(result.is_noop()); } } } @@ -3123,9 +2997,6 @@ SCENARIO("A scoped build does not blind the next full build", "[e2e][incremental SCENARIO("A build run from a subdirectory does not stamp an out-of-scope change as current", "[e2e][incremental][scope]") { - // A cwd-derived scope parses the whole project but detects only its own directory, while the - // record leg re-hashed every graph file: a/src.txt was recorded current on the strength of a - // stat nothing consumed, so the next full build compared v2 against v2 forever (#288). GIVEN("two independent directories, settled") { auto f = E2EFixture { "scoped_out_of_scope_edit" }; @@ -3154,8 +3025,6 @@ SCENARIO("A build run from a subdirectory does not stamp an out-of-scope change SCENARIO("A build with --all-deps does not stamp an out-of-scope change as current", "[e2e][incremental][scope]") { - // -a empties parse_scopes for the same reason cwd scoping does, so it reaches #288 by the - // same door: everything is parsed, only the scope is detected, everything is recorded. GIVEN("two independent directories, settled") { auto f = E2EFixture { "scoped_out_of_scope_edit" }; @@ -3184,8 +3053,6 @@ SCENARIO("A build with --all-deps does not stamp an out-of-scope change as curre SCENARIO("A file added while building from a subdirectory is still built", "[e2e][incremental][scope]") { - // The other side of #288's fix: carrying an unexamined file's recorded state forward must not - // become "record nothing", or a file first seen by a scoped build would never be built. GIVEN("a settled project whose other directory globs its sources") { auto f = E2EFixture { "glob_mixed_space" }; @@ -3223,7 +3090,6 @@ SCENARIO("Removed source file triggers stale output cleanup", "[e2e][incremental { auto f = E2EFixture { "new_file_detection" }; - // Add second file f.write_file("mul.c", "int mul(int a, int b) { return a * b; }\n"); REQUIRE(f.init().success()); @@ -3469,7 +3335,6 @@ SCENARIO("A stale output that cannot be deleted fails the build and keeps its re if (!probe_ec) { std::filesystem::remove(dir / "probe", probe_ec); std::filesystem::permissions(dir, writable, std::filesystem::perm_options::replace); - // Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. WARN("cannot revoke write permission (running as root?): scenario not exercised"); return; } @@ -3551,7 +3416,6 @@ SCENARIO("clean does not count an empty directory it could not remove", "[e2e][c if (!probe_ec) { std::filesystem::remove(root / "probe", probe_ec); std::filesystem::permissions(root, writable, std::filesystem::perm_options::replace); - // Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. WARN("cannot revoke write permission (running as root?): scenario not exercised"); return; } @@ -3571,7 +3435,6 @@ SCENARIO("clean does not count an empty directory it could not remove", "[e2e][c REQUIRE(combined.find("Failed to remove directory") != std::string::npos); REQUIRE(combined.find("0 directories") != std::string::npos); REQUIRE_FALSE(result.success()); - // The platform message already names the path; the caller must not repeat it. auto const dir_name = std::string { "/out:" }; auto first = combined.find(dir_name); REQUIRE(first != std::string::npos); @@ -3602,7 +3465,6 @@ SCENARIO("distclean does not report a reset it could not perform", "[e2e][clean] if (!probe_ec) { std::filesystem::remove(root / "probe", probe_ec); std::filesystem::permissions(root, writable, std::filesystem::perm_options::replace); - // Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. WARN("cannot revoke write permission (running as root?): scenario not exercised"); return; } @@ -3654,8 +3516,6 @@ SCENARIO("clean leaves a source file an inactive branch merely declares", "[e2e] SCENARIO("A source and the out-of-tree output shadowing it are one record clean can read", "[e2e][clean][out-of-tree]") { - // The regression pin for comparing recorded paths as stored: this record holds one file as - // both a source and an output, and only their spellings tell them apart (#382). GIVEN("a build whose output shadows a committed source of the same name") { auto f = E2EFixture { "glob_mixed_space" }; @@ -3686,8 +3546,6 @@ SCENARIO("A source and the out-of-tree output shadowing it are one record clean SCENARIO("A build refuses to overwrite a file the record does not attribute to a rule", "[e2e][build][conditionals]") { - // Whether a previous build happened must not change the answer: the record is what the - // guard reads, and none of these three sequences gives it a claim on the file. GIVEN("a conditional branch whose output path holds a file no rule has produced") { auto f = E2EFixture { "glob_mixed_space" }; @@ -3875,7 +3733,6 @@ SCENARIO("A subdirectory an inactive branch would write into is not generated", SCENARIO("Turning a branch off keeps ownership of what it built", "[e2e][build][conditionals]") { - // Ownership survives the branch going inactive only via the record's carry-forward (#369). GIVEN("an output produced while its branch was active") { auto f = E2EFixture { "glob_mixed_space" }; @@ -3902,8 +3759,6 @@ SCENARIO("Turning a branch off keeps ownership of what it built", "[e2e][build][ SCENARIO("A stale output that cannot even be queried keeps its record", "[e2e][incremental][stale]") { - // The rule lives in the readable root Tupfile so its directory stays authoritative; - // only the output's directory is locked, which is what reaches the exists() guard. GIVEN("a removed rule whose output sits in a directory that cannot be read") { auto f = E2EFixture { "glob_mixed_space" }; @@ -3922,7 +3777,6 @@ SCENARIO("A stale output that cannot even be queried keeps its record", "[e2e][i if (!probe_ec) { std::filesystem::remove(dir / "probe", probe_ec); std::filesystem::permissions(dir, readable, std::filesystem::perm_options::replace); - // Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. WARN("cannot revoke directory permissions (running as root?): scenario not exercised"); return; } @@ -4140,7 +3994,7 @@ SCENARIO("Removed source file cleans stale output in variant build", "[e2e][incr REQUIRE(f.build({ "-B", "build" }).success()); REQUIRE(f.exists("build/add.o")); REQUIRE(f.exists("build/mul.o")); - REQUIRE_FALSE(f.exists("add.o")); // Not in source dir + REQUIRE_FALSE(f.exists("add.o")); WHEN("a source file is removed") { @@ -4161,9 +4015,6 @@ SCENARIO("Source file content change triggers rebuild in variant build", "[e2e][ { GIVEN("a variant build with cross-directory dependencies") { - // scoped_build has: app -> lib cross-directory dep - // app/Tupfile is parsed first (alphabetically), references ../lib/foo.o - // This may create Ghost nodes, testing the ID contiguity fix auto f = E2EFixture { "scoped_build" }; f.mkdir("build"); f.write_file("build/tup.config", ""); @@ -4181,7 +4032,6 @@ SCENARIO("Source file content change triggers rebuild in variant build", "[e2e][ WHEN("source file content is modified without size change") { - // Modify "42" to "99" - same size (2 chars), different content auto original = f.read_file("lib/foo.c"); auto pos = original.find("42"); REQUIRE(pos != std::string::npos); @@ -4200,10 +4050,6 @@ SCENARIO("Source file content change triggers rebuild in variant build", "[e2e][ } } -// ============================================================================= -// Scoped Build Tests -// ============================================================================= - SCENARIO("Scoped build skips changes outside scope", "[e2e][incremental][scope]") { GIVEN("a multi-directory project with cross-directory dependencies") @@ -4344,7 +4190,6 @@ SCENARIO("Explicit target path sets scope", "[e2e][incremental][scope]") THEN("only the lib scope is checked and rebuilt") { REQUIRE(result.success()); - // lib/foo.c change detected and rebuilt REQUIRE(result.stdout_output.find("foo.o") != std::string::npos); } } @@ -4361,14 +4206,12 @@ SCENARIO("Tupfile changes detected regardless of scope", "[e2e][incremental][sco WHEN("a Tupfile outside scope is modified") { - // A comment would not do: an edit leaving every command's text identical is correctly a no-op (#225), so the mutation has to change a command. f.write_file("app/Tupfile", ": main.c ../lib/foo.o |> gcc %f -o %o -DEXTRA=1 |> app\n"); auto result = f.run_pup_in_dir("lib", { "-v" }); THEN("the change is detected and triggers rebuild") { REQUIRE(result.success()); - // Tupfile change causes dependent app to rebuild REQUIRE_FALSE(result.is_noop()); } } @@ -4432,10 +4275,6 @@ SCENARIO("Scoped build with -a checks upstream deps (mma behavior)", "[e2e][incr } } -// ============================================================================= -// Cross-Directory Scoped Build with -a (all-deps) Tests -// ============================================================================= - SCENARIO("Fresh scoped build with -a succeeds for cross-directory deps", "[e2e][scope]") { GIVEN("a project with producer/consumer cross-directory dependencies") @@ -4484,12 +4323,10 @@ SCENARIO("Fresh scoped build WITHOUT -a fails for cross-directory deps", "[e2e][ SCENARIO("A file that cannot be hashed is named in a warning", "[e2e][incremental]") { - // The only signal a user gets when content hashing fails; it had no test at all (#204). GIVEN("a source file that cannot be read") { auto f = E2EFixture { "glob_mixed_space" }; f.write_file("in.txt", "hello\n"); - // The command must not read in.txt, or it fails before putup ever hashes it. f.write_file("Tupfile", ": in.txt |> echo done > %o |> out.o\n"); REQUIRE(f.init().success()); REQUIRE(f.build().success()); @@ -4500,7 +4337,6 @@ SCENARIO("A file that cannot be hashed is named in a warning", "[e2e][incrementa auto probe = std::ifstream { victim }; if (probe.good()) { std::filesystem::permissions(victim, readable, std::filesystem::perm_options::replace); - // Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. WARN("cannot revoke read permission (running as root?): scenario not exercised"); return; } @@ -4628,10 +4464,6 @@ SCENARIO("Incremental -a scoped build detects upstream changes", "[e2e][incremen } } -// ============================================================================= -// Clean/Distclean Tests -// ============================================================================= - SCENARIO("Clean removes generated files but preserves sources", "[e2e][clean]") { GIVEN("a built project") @@ -4724,7 +4556,7 @@ SCENARIO("Clean works for out-of-tree builds", "[e2e][clean][variant]") REQUIRE(f.pup({ "configure", "-B", "build" }).success()); REQUIRE(f.build({ "-B", "build" }).success()); REQUIRE(f.exists("build/hello.o")); - REQUIRE_FALSE(f.exists("hello.o")); // Not in source dir + REQUIRE_FALSE(f.exists("hello.o")); WHEN("pup clean -B build is executed") { @@ -4844,7 +4676,7 @@ SCENARIO("Distclean with no index still removes .pup", "[e2e][clean]") GIVEN("a project with .pup directory but no index") { auto f = E2EFixture { "distclean_no_index" }; - f.mkdir(".pup"); // Manually create .pup (simulates interrupted build) + f.mkdir(".pup"); REQUIRE(f.exists(".pup")); WHEN("pup distclean is executed") @@ -4917,10 +4749,6 @@ SCENARIO("Distclean works when running from build directory", "[e2e][clean][vari } } -// ============================================================================= -// Variant Build Tests -// ============================================================================= - SCENARIO("Out-of-tree builds place outputs in build directory", "[e2e][variant]") { GIVEN("a configured out_of_tree project") @@ -4980,7 +4808,6 @@ SCENARIO("Variant build with generated header has correct incremental behavior", WHEN("modifying the generated header content") { - // Modify the Tupfile to change the generated content f.write_file("Tupfile", "# Modified to generate different header\n" ": |> echo '#define VERSION 2' > %o |> version.h\n" "CFLAGS = -MD -I$(TUP_VARIANT_OUTPUTDIR)\n" @@ -5005,13 +4832,6 @@ SCENARIO("Variant build with generated header has correct incremental behavior", SCENARIO("Order-only deps on generated outputs resolve correctly in variants", "[e2e][variant][incremental]") { - // This tests the bug where order-only deps using plain paths (e.g., "include/foo.h") - // created duplicate File nodes instead of reusing Generated nodes. - // Pattern from busybox: - // : |> ... |> include/applets.h # output becomes build/include/applets.h - // : | include/applets.h |> ... # should resolve to build/include/applets.h - // Bug: the order-only dep created a File node at "include/applets.h" instead - // of reusing the Generated node at "build/include/applets.h". GIVEN("a variant build with output and order-only dep using same path") { auto f = E2EFixture { "variant_cross_dir_order_only" }; @@ -5037,12 +4857,6 @@ SCENARIO("Order-only deps on generated outputs resolve correctly in variants", " SCENARIO("Variant outputs are automatically mapped to build directory", "[e2e][variant]") { - // This tests that output paths are automatically mapped to the variant directory. - // Pattern from busybox: - // Root Tupfile: : |> ... |> include/header.h # should become build/include/header.h - // src/Tupfile: | $(B)/include/header.h # references build/include/header.h - // Without automatic mapping, the output creates a node at "include/header.h" (source), - // but the dependency references "build/include/header.h" (variant) - creating two nodes. GIVEN("a variant build with S/B convention like busybox") { auto f = E2EFixture { "variant_auto_output" }; @@ -5074,8 +4888,6 @@ SCENARIO("Variant outputs are automatically mapped to build directory", "[e2e][v THEN("rebuild is a no-op (no ghost nodes)") { - // If output was mapped incorrectly, there would be a ghost node - // at build/include/header.h that never gets satisfied auto rebuild = f.build({ "-B", "build" }); INFO("rebuild stdout: " << rebuild.stdout_output); REQUIRE(rebuild.success()); @@ -5087,11 +4899,6 @@ SCENARIO("Variant outputs are automatically mapped to build directory", "[e2e][v SCENARIO("Cross-directory regular inputs work in variant builds", "[e2e][variant]") { - // Similar to order-only test but with regular input dependency - // This tests that Ghost->Generated upgrade preserves edges - // aaa_consumer is parsed first (alphabetically), creates Ghost for ../zzz_producer/helper.c - // zzz_producer is parsed later, upgrades Ghost to Generated - // The edge from aaa_consumer's command to the generated file must be preserved GIVEN("a variant build with generated file as regular input from another dir") { auto f = E2EFixture { "variant_cross_dir_regular_input" }; @@ -5202,7 +5009,7 @@ SCENARIO("Subdirectory builds with cross-directory dependencies", "[e2e][variant WHEN("header in different directory is modified") { - (void)f.build(); // First build + (void)f.build(); f.write_file("include/config.h", "#define CONFIG_VALUE 100\n"); auto result = f.build(); @@ -5227,7 +5034,6 @@ SCENARIO("Variant-only files are found via generalized path resolution", "[e2e][ auto f = E2EFixture { "variant_fallback" }; f.mkdir("build"); f.write_file("build/tup.config", "# Variant config\n"); - // Create config.txt ONLY in the variant directory, not in source f.write_file("build/config.txt", "variant-only-content\n"); REQUIRE(f.pup({ "configure", "-B", "build" }).success()); @@ -5251,10 +5057,6 @@ SCENARIO("Variant-only files are found via generalized path resolution", "[e2e][ } } -// ============================================================================= -// Shell Fixture Tests (wrapped) -// ============================================================================= - SCENARIO("Conditionals test via shell fixture", "[e2e][shell]") { WHEN("the conditionals shell fixture runs") @@ -5282,10 +5084,6 @@ SCENARIO("Self-host test via shell fixture", "[e2e][shell]") } -// Regression guard: when a source's already-tracked header is edited to -// transitively include a new header, pup must record the new transitive -// header and rebuild on subsequent edits to it. Fixed by binding dep-scan -// command dirty-status to its parent compile (collect_affected_commands). SCENARIO("Transitive implicit-dep header tracking", "[e2e][shell][incremental]") { WHEN("the header_dep_transitive shell fixture runs") @@ -5301,10 +5099,6 @@ SCENARIO("Transitive implicit-dep header tracking", "[e2e][shell][incremental]") } } -// ============================================================================= -// Show Command Tests -// ============================================================================= - SCENARIO("Show graph shows only declared deps by default", "[e2e][show]") { GIVEN("a built implicit_deps project") @@ -5471,7 +5265,6 @@ SCENARIO("Show index dumps implicit-dep edges from the on-disk index", "[e2e][sh REQUIRE(result.success()); REQUIRE(result.stdout_output.find("Files:") != std::string::npos); REQUIRE(result.stdout_output.find("Commands with implicit/sticky deps:") != std::string::npos); - // No per-command section in summary mode REQUIRE(result.stdout_output.find("Commands (with implicit/sticky edges):") == std::string::npos); } } @@ -5608,7 +5401,6 @@ SCENARIO("Show script generates shell build script", "[e2e][show]") auto f = E2EFixture { "simple_c" }; REQUIRE(f.init().success()); - // Add required script config variables f.append_file("tup.config", "CONFIG_SCRIPT_PROLOGUE=#!/bin/sh\\nset -ex\\ncd \"$(dirname \"$0\")\"\n" "CONFIG_SCRIPT_RUN=(cd \"%DIR\" && %CMD)\n" @@ -5711,10 +5503,6 @@ SCENARIO("Show with unknown format fails", "[e2e][show]") } } -// ============================================================================= -// Layout Detection Tests -// ============================================================================= - SCENARIO("Layout detection finds build directory via tup.config", "[e2e][layout]") { GIVEN("a project with build/tup.config") @@ -5790,12 +5578,10 @@ SCENARIO("Layout detection prefers build/.pup with index over empty source .pup" auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" }; auto f = E2EFixture { "implicit_deps" }; - // Create empty .pup at source root (simulating stale directory) f.mkdir(".pup"); REQUIRE(f.exists(".pup")); REQUIRE_FALSE(f.exists(".pup/index")); - // Configure and build to build/ which creates build/.pup/index REQUIRE(f.pup({ "configure", "-B", "build" }).success()); REQUIRE(f.build({ "-B", "build" }).success()); REQUIRE(f.exists("build/.pup/index")); @@ -5824,17 +5610,12 @@ SCENARIO("Layout detection prefers build/.pup with index over empty source .pup" } } -// ============================================================================= -// Multi-Variant Build Tests -// ============================================================================= - SCENARIO("Multi-variant glob selection builds all matching variants", "[e2e][multi-variant]") { GIVEN("a project with multiple variant directories") { auto f = E2EFixture { "multi_variant" }; - // Create two variant directories with tup.config f.mkdir("build-debug"); f.mkdir("build-release"); f.write_file("build-debug/tup.config", "CONFIG_DEBUG=y\n"); @@ -6270,10 +6051,6 @@ SCENARIO("Multi-variant parse", "[e2e][multi-variant]") } } -// ============================================================================= -// Unified Target Tests -// ============================================================================= - SCENARIO("Unified targets - path-based variant selection", "[e2e][target]") { GIVEN("a project with build-debug and build-release variants") @@ -6451,15 +6228,12 @@ SCENARIO("Unified targets - single output file target", "[e2e][target]") WHEN("pup build-debug/hello is run after full build") { - // First do a full build so the output exists REQUIRE(f.build({ "-B", "build-debug" }).success()); REQUIRE(f.is_executable("build-debug/hello")); - // Remove the output to force rebuild f.remove_file("build-debug/hello"); REQUIRE_FALSE(f.exists("build-debug/hello")); - // Build just the single output auto result = f.pup({ "build-debug/hello" }); THEN("only that output is rebuilt") @@ -6508,11 +6282,9 @@ SCENARIO("Unified targets - B flag with output target", "[e2e][target]") WHEN("pup -B build-debug build-debug/hello rebuilds deleted output") { - // First do a full build REQUIRE(f.build({ "-B", "build-debug" }).success()); REQUIRE(f.is_executable("build-debug/hello")); - // Remove output and rebuild with -B + output target f.remove_file("build-debug/hello"); REQUIRE_FALSE(f.exists("build-debug/hello")); @@ -6527,10 +6299,6 @@ SCENARIO("Unified targets - B flag with output target", "[e2e][target]") } } -// ============================================================================= -// Target-based build (from scratch) tests -// ============================================================================= - SCENARIO("Target-based build from scratch", "[e2e][target]") { GIVEN("an initialized multi_file project") @@ -6598,10 +6366,6 @@ SCENARIO("Target-based build with variant", "[e2e][target][variant]") } } -// ============================================================================= -// Scoped tup.config tests -// ============================================================================= - SCENARIO("Subdir merges parent and local config", "[e2e][scoped-config]") { GIVEN("a project with root and sub configs defining different vars") @@ -6638,7 +6402,6 @@ SCENARIO("Subdir inherits from parent when no local config", "[e2e][scoped-confi f.mkdir("build/sub/deep"); f.write_file("build/tup.config", "CONFIG_ROOT_VAR=from_root\n"); f.write_file("build/sub/tup.config", "CONFIG_SUB_VAR=from_sub\n"); - // NO build/sub/deep/tup.config REQUIRE(f.pup({ "configure", "-B", "build" }).success()); WHEN("pup builds the project") @@ -6661,7 +6424,6 @@ SCENARIO("Root config used when no intermediate configs", "[e2e][scoped-config]" auto f = E2EFixture { "scoped_config" }; f.mkdir("build/sub"); f.write_file("build/tup.config", "CONFIG_ROOT_VAR=from_root\n"); - // NO build/sub/tup.config - should inherit from root REQUIRE(f.pup({ "configure", "-B", "build" }).success()); WHEN("pup builds the project") @@ -6684,7 +6446,7 @@ SCENARIO("Empty subdir config does not block parent merge", "[e2e][scoped-config auto f = E2EFixture { "scoped_config" }; f.mkdir("build/sub"); f.write_file("build/tup.config", "CONFIG_ROOT_VAR=from_root\n"); - f.write_file("build/sub/tup.config", ""); // Empty — parent vars merge through + f.write_file("build/sub/tup.config", ""); REQUIRE(f.pup({ "configure", "-B", "build" }).success()); WHEN("pup builds the project") @@ -6732,7 +6494,6 @@ SCENARIO("Multi-level config merge", "[e2e][scoped-config]") f.write_file("build/tup.config", "CONFIG_ROOT_VAR=from_root\n"); f.write_file("build/sub/tup.config", "CONFIG_SUB_VAR=from_sub\n"); f.write_file("build/sub/deep/tup.config", "CONFIG_DEEP_VAR=from_deep\n"); - // Custom Tupfile to test all three vars at the deep level f.write_file("sub/deep/Tupfile", ": |> echo \"@(ROOT_VAR)\" > %o |> root_from_deep.txt\n" ": |> echo \"@(SUB_VAR)\" > %o |> sub_from_deep.txt\n" @@ -6800,10 +6561,6 @@ SCENARIO("-D config overrides win over all config files", "[e2e][scoped-config]" } } -// ============================================================================= -// pup configure command tests -// ============================================================================= - SCENARIO("Configure executes config-generating rules only", "[e2e][configure]") { GIVEN("a project with config-generating and normal rules") @@ -6883,7 +6640,6 @@ SCENARIO("Configure uses root tup.config only", "[e2e][configure]") auto f = E2EFixture { "configure_cmd" }; f.mkdir("build"); f.write_file("build/tup.config", "CONFIG_MACHINE=board-xyz\n"); - // NO build/configs/tup.config REQUIRE(f.pup({ "configure", "-B", "build" }).success()); WHEN("pup configure runs") @@ -6909,7 +6665,6 @@ SCENARIO("Configure does not write index", "[e2e][configure]") f.mkdir("build"); f.write_file("build/tup.config", "CONFIG_MACHINE=board-xyz\n"); REQUIRE(f.pup({ "configure", "-B", "build" }).success()); - // Remove index if it was created by init f.remove_file("build/.pup/index"); REQUIRE_FALSE(f.exists("build/.pup/index")); @@ -6933,7 +6688,6 @@ SCENARIO("Configure does not create .pup directory", "[e2e][configure]") auto f = E2EFixture { "configure_cmd" }; f.mkdir("build"); f.write_file("build/tup.config", "CONFIG_MACHINE=board-xyz\n"); - // Do NOT call init - no .pup directory exists REQUIRE_FALSE(f.exists("build/.pup")); WHEN("pup configure runs") @@ -6965,7 +6719,6 @@ SCENARIO("Full two-stage build with pup configure", "[e2e][configure]") auto configure_result = f.pup({ "configure", "-B", "build" }); REQUIRE(configure_result.success()); - // Verify config content immediately after configure REQUIRE(f.exists("build/configs/tup.config")); auto config_content = f.read_file("build/configs/tup.config"); REQUIRE(config_content.find("hello-world") != std::string::npos); @@ -6982,10 +6735,6 @@ SCENARIO("Full two-stage build with pup configure", "[e2e][configure]") SCENARIO("Configure handles config rule depending on non-config rule", "[e2e][configure][deps]") { - // Bug: passing only config-output commands as a filter ignores their dependencies. - // If a config rule depends on an intermediate file produced by a non-config rule, - // the dependency is not run, causing the config rule to fail. - GIVEN("a project where config rule depends on intermediate file") { auto f = E2EFixture { "configure_deps" }; @@ -7001,8 +6750,6 @@ SCENARIO("Configure handles config rule depending on non-config rule", "[e2e][co { INFO("stdout: " << result.stdout_output); INFO("stderr: " << result.stderr_output); - // This test documents the expected behavior: - // Configure should run BOTH the intermediate rule AND the config rule REQUIRE(result.success()); REQUIRE(f.exists("build/configs/tup.config")); } @@ -7018,7 +6765,6 @@ SCENARIO("Configure works with empty .pup directory (no index)", "[e2e][configur f.mkdir("build"); f.write_file("build/tup.config", ""); - // Create empty .pup directory (simulating interrupted init or manual mkdir) f.mkdir("build/.pup"); REQUIRE(f.exists("build/.pup")); REQUIRE_FALSE(f.exists("build/.pup/index")); @@ -7066,8 +6812,6 @@ SCENARIO("A configure that cannot write tup.config does not report creating it", { auto f = E2EFixture { "simple_c" }; REQUIRE(f.init().success()); - // A file where the directory must go beats the exists() guard on tup.config itself, - // and needs no permission bits, so the scenario runs under root too. f.write_file("blocked", ""); WHEN("configure is run against it") @@ -7157,11 +6901,8 @@ SCENARIO("Config selection persists across multiple builds", "[e2e][configure]") { GIVEN("a project with config selected via tup.config") { - // Use simple_c fixture which has no config-generating rules auto f = E2EFixture { "simple_c" }; f.mkdir("build"); - // Write config that would be selected via environment variable - // (e.g., user sets CONFIG_BOARD=my-board based on env var) f.write_file("build/tup.config", "CONFIG_OPT=fast\nCONFIG_DEBUG=0\n"); WHEN("pup configure runs and then pup build runs multiple times") @@ -7171,20 +6912,17 @@ SCENARIO("Config selection persists across multiple builds", "[e2e][configure]") INFO("configure stderr: " << configure_result.stderr_output); REQUIRE(configure_result.success()); - // First build auto build1 = f.build({ "-B", "build" }); INFO("build1 stdout: " << build1.stdout_output); INFO("build1 stderr: " << build1.stderr_output); REQUIRE(build1.success()); REQUIRE(f.is_executable("build/hello")); - // Second build should succeed auto build2 = f.build({ "-B", "build" }); INFO("build2 stdout: " << build2.stdout_output); INFO("build2 stderr: " << build2.stderr_output); REQUIRE(build2.success()); - // Third build should be no-op (nothing changed) auto build3 = f.build({ "-B", "build" }); INFO("build3 stdout: " << build3.stdout_output); INFO("build3 stderr: " << build3.stderr_output); @@ -7193,7 +6931,6 @@ SCENARIO("Config selection persists across multiple builds", "[e2e][configure]") THEN("builds are stable and config persists") { - // Config file should still exist with original values auto config = f.read_file("build/tup.config"); REQUIRE(config.find("CONFIG_OPT=fast") != std::string::npos); REQUIRE(config.find("CONFIG_DEBUG=0") != std::string::npos); @@ -7210,7 +6947,6 @@ SCENARIO("Build skips config-generating rules", "[e2e][configure][build]") f.mkdir("build"); f.write_file("build/tup.config", "CONFIG_MACHINE=test-value\n"); - // Run configure first REQUIRE(f.pup({ "configure", "-B", "build" }).success()); auto config_after_configure = f.read_file("build/configs/tup.config"); REQUIRE(config_after_configure.find("test-value") != std::string::npos); @@ -7224,7 +6960,6 @@ SCENARIO("Build skips config-generating rules", "[e2e][configure][build]") INFO("stdout: " << result.stdout_output); INFO("stderr: " << result.stderr_output); REQUIRE(result.success()); - // Config should be unchanged from configure auto config_after_build = f.read_file("build/configs/tup.config"); REQUIRE(config_after_build == config_after_configure); } @@ -7432,10 +7167,6 @@ SCENARIO("configure handles mixed copy-rule + auto-gen configs", "[e2e][configur } } -// ============================================================================= -// Duplicate Output Detection Tests -// ============================================================================= - SCENARIO("Duplicate output detection", "[e2e][duplicate]") { GIVEN("a Tupfile with two rules producing the same output") @@ -7513,8 +7244,6 @@ SCENARIO("A build aborted before a command could run does not record it as done" f.write_file("gate", ""); REQUIRE_FALSE(f.build({ "-j1" }).success()); - // Self-validating: the point of the scenario is that an abort leaves commands - // it meant to run un-run, so it must witness one before asserting what follows. auto stranded = 0; for (auto i = 1; i <= 8; ++i) { if (f.read_file("o" + std::to_string(i) + ".txt") == "v1\n") { @@ -7596,8 +7325,6 @@ SCENARIO("A build that cannot save its record does not report success", "[e2e][i auto f = E2EFixture { "glob_mixed_space" }; f.write_file("Tupfile", ": src.txt |> cp %f %o |> out.txt\n"); f.write_file("src.txt", "ORIGINAL\n"); - // Renaming onto a directory fails for root too, so this needs no permission bits and - // no skip guard, unlike the scenarios that revoke write access to a directory. f.mkdir(".pup/index"); WHEN("the project is built") @@ -7647,9 +7374,6 @@ SCENARIO("A command that failed is re-run on the next build", "[e2e][incremental SCENARIO("A failed command's consumer runs once the command succeeds", "[e2e][incremental][failure]") { - // The consumer's output already exists holding V1, so nothing schedules the consumer except - // propagation from the producer's re-run. Routing the retry through forced_cmds instead of - // changed_outputs leaves final.txt at V1 forever, which is what this pins. GIVEN("a built producer/consumer pair whose producer then fails after writing partial output") { auto f = E2EFixture { "failed_command" }; @@ -7952,10 +7676,8 @@ auto index_shape(std::vector const& bytes) -> std::vector return shape; } -} // namespace +} -// Determinism of construction, not of reload: two builds that never saw each other's work must -// record the same project. Answering this by reading the code is what #298's consult had to do. SCENARIO("Two builds of one tree record the same thing", "[e2e][index][determinism]") { GIVEN("the same project built twice from scratch, in trees that share nothing") @@ -7980,9 +7702,6 @@ SCENARIO("Two builds of one tree record the same thing", "[e2e][index][determini REQUIRE(index_shape(a) == index_shape(b)); } - // A record whose sections are identical but whose lengths are not would mean the - // masking above is hiding a difference rather than excluding a timestamp; a shape - // that covers only the header would mean it is comparing almost nothing. THEN("they are the same size, and the comparison covers the record") { REQUIRE(a.size() == b.size()); @@ -7990,8 +7709,6 @@ SCENARIO("Two builds of one tree record the same thing", "[e2e][index][determini } } - // The weaker property the incremental suite leans on, asserted here because it costs one - // build: a run that does nothing must not rewrite what the record says either. WHEN("one of them is rebuilt with nothing to do") { auto const before = index_bytes(first); @@ -8154,7 +7871,6 @@ SCENARIO("A record distclean kept can still name the files it owns", "[e2e][clea WHEN("a putup whose window covers that record cleans the project") { - // The same bytes, a reader that accepts them: what keeping the record buys. stamp_index_version(f, pup::index::INDEX_VERSION); auto cleaned = f.clean(); @@ -8206,8 +7922,6 @@ SCENARIO("A damaged record says so instead of rebuilding in silence", "[e2e][inc f.write_file("src.txt", "ORIGINAL\n"); REQUIRE(f.build().success()); damage_index_layout(f); - // Without this the shadow guard speaks first: an output on disk with no readable record - // is the #291 refusal, which would pass this scenario for the wrong reason. f.remove_file("out.txt"); WHEN("the next build loads that record") @@ -8234,10 +7948,7 @@ SCENARIO("A record too short to hold a header says so instead of rebuilding in s f.write_file("Tupfile", ": src.txt |> cp %f %o |> out.txt\n"); f.write_file("src.txt", "ORIGINAL\n"); REQUIRE(f.build().success()); - // Below sizeof(RawHeader) + sizeof(RawFooter): above it the declared-layout row answers - // first and this would pin the wrong rejection. truncate_index(f, 40); - // Without this the shadow guard speaks first, as in the layout scenario above. f.remove_file("out.txt"); WHEN("the next build loads that record") @@ -8410,7 +8121,6 @@ SCENARIO("Distcleaning keeps a record whose files it could not remove", "[e2e][c if (!probe_ec) { std::filesystem::remove(dir / "probe", probe_ec); std::filesystem::permissions(dir, writable, std::filesystem::perm_options::replace); - // Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. WARN("cannot revoke write permission (running as root?): scenario not exercised"); return; } @@ -8425,8 +8135,6 @@ SCENARIO("Distcleaning keeps a record whose files it could not remove", "[e2e][c INFO("stdout: " << result.stdout_output); INFO("stderr: " << result.stderr_output); auto combined = result.stdout_output + result.stderr_output; - // The removal failure names the file on its own, so only the keep message - // witnesses that the record was kept because of it. REQUIRE(combined.find("Keeping the build record") != std::string::npos); REQUIRE(combined.find("rm -rf") != std::string::npos); REQUIRE(combined.find("Project reset complete") == std::string::npos); @@ -8461,7 +8169,6 @@ SCENARIO("Distcleaning out of tree keeps a record whose files it could not remov if (!probe_ec) { std::filesystem::remove(dir / "probe", probe_ec); std::filesystem::permissions(dir, writable, std::filesystem::perm_options::replace); - // Not SKIP: the suite is built -fno-exceptions, where Catch2 aborts the process instead. WARN("cannot revoke write permission (running as root?): scenario not exercised"); return; } @@ -8600,8 +8307,6 @@ SCENARIO("A glob's %f order does not depend on the build directory's name", "[e2 INFO("AAA: " << from_aaa); INFO("zz: " << from_zz); REQUIRE(from_aaa == from_zz); - // Pins both halves and the canonical order: equality alone would still - // hold if the filesystem half stopped contributing entirely. REQUIRE(from_aaa == "GEN\nKEEP\n"); } } @@ -8610,10 +8315,6 @@ SCENARIO("A glob's %f order does not depend on the build directory's name", "[e2 SCENARIO("Out-of-tree, a generated file shadowing a source is one glob match and the source survives", "[e2e][glob][pathspace]") { - // Out-of-tree the output lands under the build root, so the committed file is shadowed - // rather than overwritten -- confusing, but not data loss, which is why #194's rejection - // is limited to in-tree builds. What #191 guarantees here is that the file is one match - // rather than two. GIVEN("a generated file whose name also exists as a checked-in source") { auto f = E2EFixture { "glob_mixed_space" }; @@ -9116,8 +8817,6 @@ SCENARIO("Deleting a stale output re-runs its order-only consumer in the same bu SCENARIO("A glob consumer of a deleted stale output settles after the healing build", "[e2e][stale][order-only]") { - // The heal in build 2 is #212 and is deliberately better than tup, which runs the consumer - // zero times; only the third build is the defect (#213). GIVEN("a consumer that reaches a generated file through an order-only glob") { auto f = E2EFixture { "glob_mixed_space" }; @@ -9164,7 +8863,6 @@ SCENARIO("A glob consumer of a deleted stale output settles after the healing bu SCENARIO("A rule still naming a deleted stale output is rejected rather than re-run", "[e2e][stale][order-only]") { - // The first rebuild is #212's routed heal; the defect is the build after it, which re-detects the file putup itself deleted (#213). GIVEN("a consumer whose order-only input was generated by a rule that has been dropped") { auto f = E2EFixture { "glob_mixed_space" }; @@ -9197,7 +8895,6 @@ SCENARIO("A rule still naming a deleted stale output is rejected rather than re- SCENARIO("Removing a group member re-runs the commands that consume the group", "[e2e][stale][order-only][group]") { - // The rule vanishes with its glob match, never by a Tupfile edit: an edit rebuilds a surviving member, and a rebuilt member reaches the consumer over the live graph, masking the removal (#169). GIVEN("a consumer that depends on a group order-only and reads the directory for members") { auto f = E2EFixture { "glob_mixed_space" }; @@ -9224,9 +8921,7 @@ SCENARIO("Removing a group member re-runs the commands that consume the group", REQUIRE(f.read_file("b/listing.txt").find("two.o") == std::string::npos); REQUIRE(f.read_file("b/listing.txt").find("one.o") != std::string::npos); INFO("stdout: " << result.stdout_output); - // Names the consuming command too, which an unannotated rule did not (#229). REQUIRE(result.stdout_output.find("Removed input: a/two.o (ls ../a") != std::string::npos); - // The removed foreach instance, not the pattern its sibling also matches. REQUIRE(result.stdout_output.find("Removed command: cp two.txt two.o (in a)") != std::string::npos); } } @@ -9235,7 +8930,6 @@ SCENARIO("Removing a group member re-runs the commands that consume the group", SCENARIO("Removing a group member schedules nothing when the group's only consumer is guarded off", "[e2e][stale][order-only][group]") { - // Edges into a guard-unsatisfied command are dropped when the index is written, so the walk finding nothing here is the correct answer and not a missed route. GIVEN("a group whose only consumer sits in an inactive conditional") { auto f = E2EFixture { "glob_mixed_space" }; @@ -9326,10 +9020,6 @@ endif } } -// ============================================================================= -// Platform Conditional Tests -// ============================================================================= - SCENARIO("TUP_PLATFORM env var controls platform conditionals", "[e2e][platform]") { GIVEN("a Tupfile with platform-conditional rules") @@ -9348,7 +9038,6 @@ SCENARIO("TUP_PLATFORM env var controls platform conditionals", "[e2e][platform] REQUIRE(result.success()); REQUIRE(f.exists("posix.txt")); REQUIRE_FALSE(f.exists("win32.txt")); - // Content should be the platform name (linux, macosx, etc.) REQUIRE_FALSE(f.read_file("posix.txt").empty()); } } @@ -9380,12 +9069,6 @@ SCENARIO("TUP_PLATFORM env var controls platform conditionals", "[e2e][platform] SCENARIO("Env var change in a conditional rebuilds the affected branch", "[e2e][platform][incremental][platform-incremental]") { - // Both branches of `ifeq ($(TUP_PLATFORM),...)` produce the same output file with - // distinct content; the platform string does not appear in either command's text. - // So the only signal that an env-driven branch flip occurred is a sticky edge from - // the env Variable node to the guarded commands (the condition_env_vars path, - // symmetric to condition_config_vars). Without it, change detection sees no changed - // file and no changed identity, reports "Nothing to do", and the output stays stale. GIVEN("a project whose active branch is selected by an env-sourced $(TUP_PLATFORM) condition") { auto f = E2EFixture { "platform_conditional_incremental" }; @@ -9394,7 +9077,7 @@ SCENARIO("Env var change in a conditional rebuilds the affected branch", REQUIRE(f.init().success()); REQUIRE(f.build().success()); REQUIRE(f.read_file("result.txt") == "WINBUILD\n"); - REQUIRE(f.build().is_noop()); // stable under no change + REQUIRE(f.build().is_noop()); } WHEN("the env var flips, selecting the other branch") @@ -9449,7 +9132,6 @@ SCENARIO("Imported env vars persist across builds", "[e2e][import]") WHEN("rebuilding without env var set") { - // MY_VAR not in environment (EnvGuard out of scope) auto result = f.build(); THEN("cached value is used, no rebuild triggered") @@ -9504,11 +9186,6 @@ SCENARIO("Imported env vars with equals in value", "[e2e][import]") SCENARIO("Exported env var consumed via subprocess environment triggers rebuild", "[e2e][import][envdep]") { - // The command consumes MY_GREETING through the inherited environment (bare $VAR - // in the shell), NOT via $(VAR) substitution. So the rendered command string is - // byte-identical across values: a string-keyed change detector cannot see the - // change. Correctness requires the command's identity to fold in the values of - // the vars it depends on (here, the exported MY_GREETING). GIVEN("a project already built with an exported env var the shell reads from the environment") { auto f = E2EFixture { "env_dep_subprocess" }; @@ -9517,7 +9194,7 @@ SCENARIO("Exported env var consumed via subprocess environment triggers rebuild" REQUIRE(f.init().success()); REQUIRE(f.build().success()); REQUIRE(f.read_file("out.txt") == "hello\n"); - REQUIRE(f.build().is_noop()); // stable under no change + REQUIRE(f.build().is_noop()); } WHEN("the exported env var changes (command text stays byte-identical)") @@ -9717,10 +9394,6 @@ SCENARIO("Only commands using changed env var rebuild", "[e2e][import]") } } -// ============================================================================= -// Conditional Assignment Operators Tests -// ============================================================================= - SCENARIO("?= soft assignment - first wins", "[e2e][assignment]") { GIVEN("a Tupfile with multiple ?= assignments") @@ -9769,7 +9442,6 @@ SCENARIO("?= soft assignment - = takes precedence", "[e2e][assignment]") } } -// Use ?\?= in strings to avoid trigraph interpretation (??= -> #) SCENARIO("?\?= weak assignment - last wins", "[e2e][assignment]") { GIVEN("a Tupfile with multiple ?\?= assignments") @@ -9941,10 +9613,6 @@ SCENARIO("import ?= default reverts when env is unset on warm index", "[e2e][imp } } -// ============================================================================= -// Error Handling Tests -// ============================================================================= - SCENARIO("Cyclic dependency detection", "[e2e][error]") { GIVEN("a project with circular dependencies") @@ -9988,10 +9656,6 @@ SCENARIO("Missing include file detection", "[e2e][error]") } } -// ============================================================================= -// Out-of-Tree Configuration Tests (3-tree builds) -// ============================================================================= - SCENARIO("Out-of-tree configuration with separate source/config/build trees", "[e2e][out-of-tree-config]") { GIVEN("a project with separate source, config, and build directories") @@ -10001,7 +9665,6 @@ SCENARIO("Out-of-tree configuration with separate source/config/build trees", "[ auto config_dir = f.workdir() / "config"; auto build_dir = f.workdir() / "build"; - // Create build directory with tup.config f.mkdir("build"); f.write_file("build/tup.config", ""); @@ -10024,7 +9687,6 @@ SCENARIO("Out-of-tree configuration with separate source/config/build trees", "[ THEN("Tupfiles are discovered from config directory") { - // Should find Tupfiles in config/, not source/ REQUIRE(result.stdout_output.find("config/Tupfile") != std::string::npos); } } @@ -10058,7 +9720,6 @@ SCENARIO("Out-of-tree configuration with separate source/config/build trees", "[ THEN("source directory remains pristine") { - // No .pup or build artifacts in source REQUIRE_FALSE(f.exists("source/.pup")); REQUIRE_FALSE(f.exists("source/main.o")); } @@ -10112,10 +9773,6 @@ SCENARIO("Cross-directory groups in 3-tree builds", "[e2e][out-of-tree-config][g { GIVEN("a 3-tree project with cross-directory group references via variables") { - // Mirrors the GCC example pattern: - // root Tuprules.tup: S = $(TUP_CWD); LIB_DIR = gcc - // gcc/Tuprules.tup: S ?= $(TUP_CWD); LIB_DIR ?= .; macros use $(S)/$(LIB_DIR)/ - // gcc/Tupfile: produces , consumes via macros auto f = E2EFixture { "groups_cross_dir_3tree" }; auto source_dir = f.workdir() / "source"; auto config_dir = f.workdir() / "config"; @@ -10183,11 +9840,9 @@ SCENARIO("Out-of-tree configuration with TUP_SRCDIR and TUP_OUTDIR variables", " auto build_debug = f.workdir() / "build-debug"; auto build_release = f.workdir() / "build-release"; - // Create build directories f.mkdir("build-debug"); f.mkdir("build-release"); - // Create debug tup.config f.write_file("build-debug/tup.config", "CONFIG_CFLAGS=-g -O0\n"); f.write_file("build-release/tup.config", "CONFIG_CFLAGS=-O2\n"); @@ -10296,7 +9951,6 @@ SCENARIO("Config tree inside source tree", "[e2e][out-of-tree-config]") THEN("globs resolve against source root, not config root") { - // main.c lives in source_dir, not source_dir/tupfiles, so a rule for its object exists only if the glob matched it there. INFO("stdout: " << result.stdout_output); REQUIRE(result.stdout_output.find("main.o") != std::string::npos); } @@ -10352,37 +10006,10 @@ SCENARIO("Config tree inside source tree", "[e2e][out-of-tree-config]") SCENARIO("Cross-project order-only dependency resolution", "[e2e][out-of-tree-config][variant]") { - // This tests the bug where order-only deps using $(B) paths in 3-tree builds - // create Ghost nodes instead of resolving to existing Generated nodes. - // Pattern from busybox: - // root/Tupfile: : |> ... |> include/autoconf.h # output at build/include/autoconf.h - // applets/Tupfile: : main.c | $(B)/include/autoconf.h |> ... - // - // The bug manifests when: - // 1. source and output are in completely different filesystem trees - // 2. build_root_name has N levels of "../" (e.g., "../../../../tmp/build") - // 3. From a subdirectory, $(B) normalizes to N-1 levels of "../" - // because one "../" cancels with the subdirectory name - // 4. strip_build_prefix() fails to match the different prefix depths - // - // Example with busybox: - // source = /home/user/src/busybox - // output = /tmp/build - // build_root_name = ../../../../tmp/build (5 levels up from source) - // From applets/: $(B) = TUP_VARIANT_OUTPUTDIR/.. - // = ../../../../tmp/build/applets/.. = ../../../tmp/build (4 levels) - // strip_build_prefix("../../../tmp/build/include/x.h", "../../../../tmp/build") - // FAILS - prefixes don't match due to depth difference! GIVEN("a 3-tree project with asymmetric directory depths") { auto f = E2EFixture { "cross_project_order_only" }; - // Create asymmetric setup: source at 3 levels deep, output at 1 level - // source_root = workdir/a/b/c/source (3 dirs deep) - // output_root = workdir/out (1 dir deep) - // build_root_name = relative(out, a/b/c/source) = ../../../../out (4 ../) - // From consumer/: $(B) expands and normalizes to ../../../out (3 ../) - // The prefix mismatch causes strip_build_prefix to fail auto source_dir = f.workdir() / "a" / "b" / "c" / "source"; auto config_dir = f.workdir() / "a" / "b" / "c" / "config"; auto build_dir = f.workdir() / "out"; @@ -10392,7 +10019,6 @@ SCENARIO("Cross-project order-only dependency resolution", "[e2e][out-of-tree-co f.mkdir("out"); f.write_file("out/tup.config", ""); - // Copy fixture files to the asymmetric locations f.write_file("a/b/c/source/consumer/main.c", f.read_file("source/consumer/main.c")); f.write_file("a/b/c/config/Tupfile.ini", ""); f.write_file("a/b/c/config/Tuprules.tup", f.read_file("config/Tuprules.tup")); @@ -10437,10 +10063,8 @@ SCENARIO("Cross-project order-only dependency resolution", "[e2e][out-of-tree-co THEN("header generation is scheduled before compilation") { - // Find positions in output auto gen_pos = result.stdout_output.find("generated.h"); auto gcc_pos = result.stdout_output.find("gcc"); - // Header generation should appear before gcc compilation REQUIRE(gen_pos != std::string::npos); REQUIRE(gcc_pos != std::string::npos); REQUIRE(gen_pos < gcc_pos); @@ -10449,10 +10073,6 @@ SCENARIO("Cross-project order-only dependency resolution", "[e2e][out-of-tree-co } } -// ============================================================================= -// Show Var Command Tests -// ============================================================================= - SCENARIO("show var displays variable assignments", "[e2e][show][var]") { GIVEN("a project with variable assignments") @@ -10528,10 +10148,6 @@ SCENARIO("show var displays variable assignments", "[e2e][show][var]") } } -// ============================================================================= -// Phi-Node Model Tests (Conditional Stability) -// ============================================================================= - SCENARIO("Phi-node model processes both conditional branches", "[e2e][phi]") { GIVEN("a project with conditional compilation") @@ -10614,9 +10230,8 @@ endif WHEN("toggled to BUILD=release") { REQUIRE(f.pup({ "configure", "-B", "build" }).success()); - (void)f.build({ "-B", "build", "-j1" }); // First build with debug + (void)f.build({ "-B", "build", "-j1" }); - // Toggle to release f.write_file("build/tup.config", "CONFIG_BUILD=release\n"); auto result = f.build({ "-B", "build", "-j1" }); @@ -11369,10 +10984,6 @@ SCENARIO("Rules with empty input patterns are skipped", "[e2e][empty-input]") } } -// ============================================================================= -// Incremental Build Command String Mismatch Tests -// ============================================================================= - SCENARIO("include_rules includes all Tuprules.tup from root to leaf", "[e2e][build]") { GIVEN("a project with Tuprules.tup at root and in a subdirectory") @@ -11393,11 +11004,8 @@ SCENARIO("include_rules includes all Tuprules.tup from root to leaf", "[e2e][bui auto content = f.read_file("sub/result.txt"); INFO("result.txt: " << content); - // ROOT comes from root Tuprules.tup (TUP_CWD = .. from sub/) REQUIRE(content.find("..") != std::string::npos); - // CC = gcc (root sets it first, sub's ?= doesn't override) REQUIRE(content.find("gcc") != std::string::npos); - // SUBVAR comes from sub/Tuprules.tup REQUIRE(content.find("from_sub") != std::string::npos); } } @@ -11406,10 +11014,6 @@ SCENARIO("include_rules includes all Tuprules.tup from root to leaf", "[e2e][bui SCENARIO("Sibling directory inputs work with incremental variant builds", "[e2e][incremental][variant]") { - // This tests the command string matching between graph and index. - // Pattern from spos: Tupfile at include/generated/ referencing ../data.txt - // Bug: Index uses std::filesystem::relative() while graph uses make_source_relative() - // These produce different paths for cross-directory references. GIVEN("a variant build with sibling directory input") { auto f = E2EFixture { "sibling_dir_inputs" }; @@ -11438,10 +11042,6 @@ SCENARIO("Sibling directory inputs work with incremental variant builds", "[e2e] } } -// ============================================================================= -// Strict Convention Checker Tests -// ============================================================================= - SCENARIO("Check level controls convention enforcement", "[e2e][strict]") { GIVEN("a project with a component violating conventions") @@ -11503,9 +11103,6 @@ SCENARIO("Check level controls convention enforcement", "[e2e][strict]") SCENARIO("The object of a compile that runs elsewhere is reported instead of scanned wrongly", "[e2e][strict][depscan]") { - // The scan runs from the Tupfile's directory, so a source word taken from an invocation that - // ran in sub/ resolves against a same-named file here -- deps recorded for a file the rule - // never compiled (#356). GIVEN("a rule that compiles here and then again after a cd, with a same-named source in both") { auto f = E2EFixture { "glob_mixed_space" }; @@ -11533,8 +11130,6 @@ SCENARIO("The object of a compile that runs elsewhere is reported instead of sca THEN("it does not claim the command contains no reproducible compile") { - // One is standing next to it: a.o's compile is the covered prefix. The report's - // unit moved to the object, so its sentence has to speak about the object. INFO("stderr: " << result.stderr_output); REQUIRE(result.stderr_output.find("in this command") == std::string::npos); REQUIRE( @@ -11584,8 +11179,6 @@ SCENARIO("An object no scanned invocation writes is reported beside its scanned GIVEN("a scanned rule in a subdirectory whose output word points into a variant directory") { - // %o one directory down expands to '../../build/src/lib/a.o' -- the word resolves against - // the rule's own directory and back into the variant, which a root-level rule never shows. auto f = E2EFixture { "variant_config_input" }; f.mkdir("src/lib"); f.write_file("src/lib/a.c", "#include \"a.h\"\nint a(void){return 0;}\n"); @@ -11629,8 +11222,6 @@ SCENARIO("An object no scanned invocation writes is reported beside its scanned SCENARIO("A compile-shaped rule with no dependency scan is reported", "[e2e][strict][depscan]") { - // The scan is declined correctly — putup cannot reproduce the prefix's shell state — but - // declining in silence leaves a rule whose headers are never recorded (#352). GIVEN("a project with a scanned compile, an announced compile, an unscanned compile, and a self-depfiling compile") { auto f = E2EFixture { "unscanned_compile" }; @@ -11691,8 +11282,6 @@ SCENARIO("A compile-shaped rule with no dependency scan is reported", "[e2e][str SCENARIO("A depfile flag the compile never carried hides no unscanned object", "[e2e][strict][depscan]") { - // The suppression exists for a compile that writes its own depfile; read across the whole - // command text it was defeated by any word spelling one, including in a later invocation (#357). GIVEN("a rule that spells a depfile flag after the compile it could not scan") { auto f = E2EFixture { "glob_mixed_space" }; @@ -11769,10 +11358,6 @@ SCENARIO("Strict checker exempts the config-tree root in 3-tree builds", "[e2e][ } } -// ============================================================================= -// Scoped Build Implicit Dependency Tests -// ============================================================================= - SCENARIO("Scoped build detects header changes outside scope", "[e2e][incremental][scope]") { GIVEN("a project with headers outside the source scope") @@ -11780,7 +11365,6 @@ SCENARIO("Scoped build detects header changes outside scope", "[e2e][incremental auto f = E2EFixture { "scoped_implicit_dep" }; REQUIRE(f.init().success()); - // Initial build (full, unscoped) auto first = f.build(); INFO("first build stdout: " << first.stdout_output); INFO("first build stderr: " << first.stderr_output); @@ -11789,10 +11373,8 @@ SCENARIO("Scoped build detects header changes outside scope", "[e2e][incremental WHEN("header outside scope is modified and scoped build runs") { - // Modify the header (outside the src/ scope) f.write_file("include/header.h", "#define VERSION 2\n"); - // Scoped build: only src/ directory auto result = f.pup({ "src" }); THEN("the change is detected and the file is rebuilt") @@ -11806,20 +11388,6 @@ SCENARIO("Scoped build detects header changes outside scope", "[e2e][incremental } } -// Reproducer for the real-world bug noted in CLAUDE.md: -// "Header-dep tracking can miss across variants — editing a widely-included -// SDK header may re-link with stale .o files and produce a binary newer -// than the header but functionally older." -// -// Differs from the single-scope test above in two ways: -// - multi-arg scoped invocation (mirrors `pup src/fubon ... ios` pattern) -// - multiple TUs in different scope dirs all transitively reach the same -// out-of-scope header, AND a downstream link rule consumes their .o files -// -// The strict assertion checks that ALL three .o files (alpha.o, beta.o, -// main.o) are present AND the binary's hash changes after the header bump, -// catching both the "noop" mode and the "rebuild some-but-not-all + relink -// with stale .o" mode of the bug. SCENARIO("Scoped build with multiple scopes detects out-of-scope header changes", "[e2e][incremental][scope][multi-scope]") { @@ -11837,15 +11405,12 @@ SCENARIO("Scoped build with multiple scopes detects out-of-scope header changes" REQUIRE(f.exists("app/main.o")); REQUIRE(f.exists("app/app")); - // Capture the linked binary's content so we can detect stale-link bugs - // even if pup claims "rebuilt". auto const binary_before = f.read_file("app/app"); WHEN("the shared header is modified and the same scoped build re-runs") { f.write_file("include/lib/shared.h", "#define VERSION 2\n"); - // Multi-arg scoped invocation mirroring the real-world reproducer. auto result = f.pup({ "src/alpha", "src/beta", "app" }); THEN("pup picks up the change and the link reflects the new VERSION") @@ -11855,10 +11420,6 @@ SCENARIO("Scoped build with multiple scopes detects out-of-scope header changes" REQUIRE(result.success()); REQUIRE_FALSE(result.is_noop()); - // Strict check: the linked binary must differ from the pre-edit - // version. If pup recompiled only some .o files and re-linked - // with stale others, the binary would be byte-identical or only - // partially updated — either way still buggy. auto const binary_after = f.read_file("app/app"); REQUIRE(binary_before != binary_after); } @@ -11866,9 +11427,6 @@ SCENARIO("Scoped build with multiple scopes detects out-of-scope header changes" } } -// Same shape as above but with the build done out-of-tree under build/, -// invoked with -B. Variants share the source tree but have independent indices -// — a fix in one variant's index must not let the other re-link with stale .o. SCENARIO("Out-of-tree variant build picks up out-of-scope header changes", "[e2e][incremental][scope][multi-scope][variant]") { @@ -11904,9 +11462,6 @@ SCENARIO("Out-of-tree variant build picks up out-of-scope header changes", } } -// Probes the scoped-initial-build axis: when the FIRST build is already -// scoped (rather than full), is the index populated correctly enough that -// a subsequent header edit propagates through the linked binary? SCENARIO("Scoped rebuild after a SCOPED initial multi-scope build", "[e2e][incremental][scope][multi-scope]") { @@ -11944,10 +11499,6 @@ SCENARIO("Scoped rebuild after a SCOPED initial multi-scope build", } } -// Probes the repeated-scoped-invocation axis: full build, then a no-op -// scoped pass, then a real edit + scoped rebuild. Mirrors the production -// pattern where developers run the same scoped command many times across -// a session before the edit that exposes the bug. SCENARIO("Header edit after a noop scoped pass still propagates to the linked binary", "[e2e][incremental][scope][multi-scope]") { @@ -11989,10 +11540,6 @@ SCENARIO("Header edit after a noop scoped pass still propagates to the linked bi } } -// ============================================================================= -// Target Build No-Op Tests -// ============================================================================= - SCENARIO("Target build stabilizes to no-op", "[e2e][incremental][target]") { GIVEN("a project with two independent targets") @@ -12000,7 +11547,6 @@ SCENARIO("Target build stabilizes to no-op", "[e2e][incremental][target]") auto f = E2EFixture { "target_noop" }; REQUIRE(f.init().success()); - // Full build — both targets auto first = f.build(); INFO("first build: " << first.stdout_output); REQUIRE(first.success()); @@ -12009,12 +11555,10 @@ SCENARIO("Target build stabilizes to no-op", "[e2e][incremental][target]") WHEN("target build runs for prog_a, then runs again") { - // First target build — may rebuild some commands auto target1 = f.pup({ "prog_a" }); INFO("target build 1: " << target1.stdout_output); REQUIRE(target1.success()); - // Second target build — must be no-op auto target2 = f.pup({ "prog_a" }); THEN("the second target build is a no-op") @@ -12030,15 +11574,6 @@ SCENARIO("Target build stabilizes to no-op", "[e2e][incremental][target]") SCENARIO("3-tree: group pattern %o must include build root prefix", "[e2e][out-of-tree-config]") { - // Reproduces GCC BSP pattern where: - // 1. Library archive uses order-only group: % in command - // 2. Consumer links the library via $(B)/$(LIB_DIR)/libmath.a - // - // The group pattern forces has_group_pattern=true, so final_instruction - // becomes cmd_text (parse-time %o expansion). Output PathIds must be - // BuildRoot-grounded so materialize_path() prepends the build root - // prefix. Without grounding, %o becomes "libmath.a" (bare filename) - // instead of "../../build/zzz_lib/libmath.a". GIVEN("a 3-tree project with order-only groups in archive command") { auto f = E2EFixture { "3tree_cross_subdir_output" }; @@ -12071,15 +11606,12 @@ SCENARIO("3-tree: group pattern %o must include build root prefix", "[e2e][out-o { INFO("stdout: " << result.stdout_output); REQUIRE(result.success()); - // The fixture's display is "AR %o", so the word after it is the rendered %o: a bare "libmath.a" is the bug, meaning the archive was written to the source tree. auto ar_pos = result.stdout_output.find("AR "); REQUIRE(ar_pos != std::string::npos); - auto output_start = ar_pos + 3; // strlen("AR ") - // Bounded to the line: %o is the last word on it, and the next line begins "[build]", which would satisfy the check below on its own. + auto output_start = ar_pos + 3; auto output_end = result.stdout_output.find_first_of(" \n", output_start); auto output_arg = result.stdout_output.substr(output_start, output_end - output_start); INFO("archive output arg: " << output_arg); - // %o must point to the build directory, not be a bare filename REQUIRE(output_arg.find("build") != std::string::npos); } } @@ -12112,10 +11644,6 @@ SCENARIO("3-tree: group pattern %o must include build root prefix", "[e2e][out-o } } -// ============================================================================= -// Build Statistics Report -// ============================================================================= - namespace { struct PhaseReport { @@ -12187,7 +11715,7 @@ auto parse_phase_report(std::string const& output) -> PhaseReport return report; } -} // namespace +} SCENARIO("Build statistics account for the whole build", "[e2e][stat]") { @@ -12343,14 +11871,8 @@ endif } } -// ============================================================================= -// Scoped build must not delete out-of-scope outputs (issue #122) -// ============================================================================= - namespace { -// A two-directory project (alpha, beta) plus a root-level rule, fully built in -// build/. Each rule just copies its source, so outputs are trivially checkable. auto build_scoped_stale_project(E2EFixture& f) -> void { f.write_file("r.c", "int r(void) { return 0; }\n"); @@ -12367,7 +11889,7 @@ auto build_scoped_stale_project(E2EFixture& f) -> void REQUIRE(f.exists("build/beta/b.out")); } -} // namespace +} SCENARIO("Scoped build preserves outputs of directories outside the scope", "[e2e][incremental][scope]") { @@ -12900,7 +12422,7 @@ auto count_occurrences(std::string const& haystack, std::string const& needle) - return n; } -} // namespace +} SCENARIO("A directory refused on a later parse round explains why it was refused", "[e2e][incremental][keep-going]") { @@ -13758,8 +13280,6 @@ SCENARIO("A glob over generated files is path-ordered and stable across builds", REQUIRE(f.read_file("matches.txt") == "alpha.gen mike.gen zeta.gen\n"); } - // The generated files exist on disk from here on, so a filesystem glob can - // now see what only the graph could see during the first build. WHEN("the unchanged project is rebuilt") { auto second = f.build(); @@ -13860,9 +13380,6 @@ SCENARIO("Imported values survive in the cache whatever order the imports are de SCENARIO("A continuation without a space before it builds and is scanned", "[e2e][incremental]") { - // Upstream tup rewrites `\`+newline to spaces, so the command runs as `gcc -c foo.c`; - // keeping the two bytes made the shell splice them into `-cfoo.c` and hid the compile - // from the dep scanner. auto env = EnvGuard { "PUP_IMPLICIT_DEPS", "1" }; GIVEN("a rule whose continuation carries no space before the backslash") @@ -14329,7 +13846,6 @@ SCENARIO("A group directory prefix after an extra outputs section names the grou REQUIRE(f.exists("side.log")); REQUIRE_FALSE(f.exists("sub")); REQUIRE(f.exists("two.txt")); - // An unresolved group only warns, so the silence is what witnesses the prefix moved it. auto const combined = result.stdout_output + result.stderr_output; REQUIRE(combined.find("has no members") == std::string::npos); } diff --git a/test/unit/test_eval.cpp b/test/unit/test_eval.cpp index 44d37a2f..a3805c5d 100644 --- a/test/unit/test_eval.cpp +++ b/test/unit/test_eval.cpp @@ -16,7 +16,7 @@ using pup::test::EnvGuard; namespace { auto intern(std::string_view s) -> StringId { return global_pool().intern(s); } auto sv(StringId id) -> std::string_view { return global_pool().get(id); } -} // namespace +} TEST_CASE("VarDb basic operations", "[eval]") { @@ -233,7 +233,7 @@ TEST_CASE("@(TUP_PLATFORM) respects CONFIG_TUP_PLATFORM in tup.config", "[eval][ auto ctx = EvalContext { .vars = &vars, .config_vars = &config_vars, - .tup_platform = intern(pup::PLATFORM), // compile-time default + .tup_platform = intern(pup::PLATFORM), }; @@ -280,7 +280,7 @@ TEST_CASE("@(TUP_ARCH) respects CONFIG_TUP_ARCH in tup.config", "[eval][arch]") auto ctx = EvalContext { .vars = &vars, .config_vars = &config_vars, - .tup_arch = intern(pup::ARCH), // compile-time default + .tup_arch = intern(pup::ARCH), }; @@ -326,7 +326,6 @@ TEST_CASE("Evaluator pattern expansion", "[eval]") auto ctx = EvalContext { .vars = &vars }; - // For foreach rules, all_inputs has just one element (the current input) auto flags = PatternFlags { .input_ext = "c", .input_dir = "src", @@ -519,7 +518,6 @@ TEST_CASE("Evaluator pattern expansion - multiple inputs", "[eval]") auto ctx = EvalContext { .vars = &vars }; - // For non-foreach rules, all_inputs has all input files auto flags = PatternFlags { .input_dir = "", .all_inputs = { "a.c", "b.c", "c.c" }, @@ -737,7 +735,6 @@ TEST_CASE("Evaluator pattern expansion - glob match", "[eval]") SECTION("%g - suffix pattern match") { - // Pattern: *_test.c, Input: foo_test.c, Match: foo auto flags = PatternFlags { .glob_match = "foo", .all_inputs = { "foo_test.c" }, @@ -966,16 +963,8 @@ TEST_CASE("Evaluator % pattern expansion", "[eval]") } } -// ============================================================================= -// TUP_VARIANT_OUTPUTDIR tests -// Based on tup test t8108-variant-outputdir.sh -// ============================================================================= - TEST_CASE("TUP_VARIANT_OUTPUTDIR expansion - no variant", "[eval][variant]") { - // Without variant, TUP_VARIANT_OUTPUTDIR should be "." - // This matches tup behavior where outputs go to current directory - auto vars = VarDb {}; auto ctx = EvalContext { .vars = &vars }; ctx.tup_variant_outputdir = intern("."); @@ -989,10 +978,6 @@ TEST_CASE("TUP_VARIANT_OUTPUTDIR expansion - no variant", "[eval][variant]") TEST_CASE("TUP_VARIANT_OUTPUTDIR expansion - in-tree variant", "[eval][variant]") { - // For in-tree variant build (e.g., build/ directory): - // From sub/dir Tupfile, TUP_VARIANT_OUTPUTDIR = ../../build/sub/dir - // This is the relative path from source sub/dir to output build/sub/dir - auto vars = VarDb {}; auto ctx = EvalContext { .vars = &vars }; ctx.tup_variant_outputdir = intern("../../build/sub/dir"); @@ -1006,9 +991,6 @@ TEST_CASE("TUP_VARIANT_OUTPUTDIR expansion - in-tree variant", "[eval][variant]" TEST_CASE("TUP_VARIANT_OUTPUTDIR in command expansion", "[eval][variant]") { - // Test that TUP_VARIANT_OUTPUTDIR expands correctly in commands - // This simulates: echo -o $(TUP_VARIANT_OUTPUTDIR)/out.txt - auto vars = VarDb {}; auto ctx = EvalContext { .vars = &vars }; ctx.tup_variant_outputdir = intern("../../build/sub/dir"); @@ -1022,14 +1004,6 @@ TEST_CASE("TUP_VARIANT_OUTPUTDIR in command expansion", "[eval][variant]") TEST_CASE("TUP_VARIANTDIR vs TUP_VARIANT_OUTPUTDIR", "[eval][variant]") { - // TUP_VARIANTDIR: relative path to variant's version of *included file's* directory - // TUP_VARIANT_OUTPUTDIR: relative path to variant's version of *current Tupfile's* directory - // - // From tup test t8108: - // - TUP_CWD = ../../rules (relative to included file) - // - TUP_VARIANTDIR = ../../build/rules (variant's rules directory) - // - TUP_VARIANT_OUTPUTDIR = ../../build/sub/dir (variant's output for this Tupfile) - auto vars = VarDb {}; auto ctx = EvalContext { .vars = &vars }; ctx.tup_cwd = intern("../../rules"); diff --git a/test/unit/test_index.cpp b/test/unit/test_index.cpp index ec301ef8..eb36e66a 100644 --- a/test/unit/test_index.cpp +++ b/test/unit/test_index.cpp @@ -30,15 +30,14 @@ namespace { auto sv(pup::StringId id) -> std::string_view { return pup::global_pool().get(id); } -} // namespace +} using namespace pup; using namespace pup::index; namespace pup::graph { -// Graph-module-internal (defined in dag.cpp, not in public header) auto add_condition_node(Graph& graph, ConditionNode node) -> Result; -} // namespace pup::graph +} namespace { @@ -72,7 +71,7 @@ auto find_file_by_path(Index const& index, std::string_view path) -> FileEntry c return nullptr; } -} // namespace +} TEST_CASE("Index format struct sizes", "[index]") { @@ -154,7 +153,6 @@ TEST_CASE("FileEntry conversion", "[index]") REQUIRE(raw.content_hash[0] == std::byte { 0xAB }); REQUIRE(raw.content_hash[31] == std::byte { 0xCD }); - // ID is computed from array index (41 + 1 = 42) auto restored = FileEntry::from_raw(raw, "main.cpp", 41); REQUIRE(restored.has_value()); @@ -169,7 +167,6 @@ TEST_CASE("FileEntry conversion", "[index]") TEST_CASE("CommandEntry conversion", "[index]") { - // Distinct values: a roundtrip that swapped the two fields must fail. auto key = pup::Hash256 {}; key[0] = std::byte { 0xAB }; key[31] = std::byte { 0xCD }; @@ -201,7 +198,6 @@ TEST_CASE("CommandEntry conversion", "[index]") REQUIRE(raw.signature == signature); REQUIRE(raw.flags == pup::index::to_underlying(pup::index::CommandFlag::MustRerun)); - // ID is computed from array index (4 + 1 = 5, then node_id::make_command) auto& pool = global_pool(); auto restored = CommandEntry::from_raw( raw, pool.get(cmd.instruction_pattern), pool.get(cmd.display), pool.get(cmd.env), @@ -389,8 +385,6 @@ TEST_CASE("Serialized edge section is independent of edge insertion order", "[in return std::vector { begin, begin + hdr.edge_count * sizeof(RawEdge) }; }; - // Ties on from (edges 4->cmd1 / 4->cmd2) and on (from, to) (Implicit vs - // Sticky 4->cmd1) exercise every leg of the canonical-order comparator. auto edges = std::vector { EdgeEntry { .from = 2, .to = cmd1, .type = LinkType::Normal }, EdgeEntry { .from = cmd1, .to = 3, .type = LinkType::Normal }, @@ -412,15 +406,10 @@ TEST_CASE("Serialized edge section is independent of edge insertion order", "[in TEST_CASE("Index serialization roundtrip", "[e2e][index]") { - // IDs must be consecutive and match array position (id = array_index + 1) - // Files: 1, 2, 3, 4, 5 in insertion order - // Commands: node_id::make_command(1) auto const cmd_id = node_id::make_command(1); auto index = Index {}; - // Add directories first (for parent chain) - // File 1: src directory index.add_file(FileEntry { .id = 1, .parent_id = 0, @@ -428,7 +417,6 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") .name = intern("src"), }); - // File 2: build directory index.add_file(FileEntry { .id = 2, .parent_id = 0, @@ -436,7 +424,6 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") .name = intern("build"), }); - // File 3: main.cpp (parent is src, id=1) index.add_file(FileEntry { .id = 3, .parent_id = 1, @@ -445,7 +432,6 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") .size = 1024, }); - // File 4: main.o (parent is build, id=2) index.add_file(FileEntry { .id = 4, .parent_id = 2, @@ -454,7 +440,6 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") .size = 4096, }); - // File 5: header file (implicit dependency) index.add_file(FileEntry { .id = 5, .parent_id = 0, @@ -463,7 +448,6 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") .size = 8192, }); - // Command 1 (v8: template + operands; v19: + key and signature hashes) auto cmd_key = pup::Hash256 {}; cmd_key[0] = std::byte { 0x11 }; cmd_key[31] = std::byte { 0x99 }; @@ -474,37 +458,31 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") .display = intern("CXX main.cpp"), .env = {}, .key = cmd_key, - .inputs = { 3 }, // main.cpp - .outputs = { 4 }, // main.o + .inputs = { 3 }, + .outputs = { 4 }, }); - // Add edges (file 3 -> cmd, cmd -> file 4, file 5 -> cmd implicit) index.add_edge(EdgeEntry { .from = 3, .to = cmd_id, .type = LinkType::Normal }); index.add_edge(EdgeEntry { .from = cmd_id, .to = 4, .type = LinkType::Normal }); index.add_edge(EdgeEntry { .from = 5, .to = cmd_id, .type = LinkType::Implicit }); - // Serialize auto data = serialize_index(index); REQUIRE(data.has_value()); REQUIRE(data->size() > sizeof(RawHeader) + sizeof(RawFooter)); - // Write to temp file and read back auto temp_path = pup::test::temp_path("pup_test_index").string(); auto write_result = write_index(temp_path, index); REQUIRE(write_result.has_value()); - // Verify file exists REQUIRE(std::filesystem::exists(temp_path)); - // Read back auto reader_result = open_index(temp_path); REQUIRE(reader_result.has_value()); auto& idx_file = *reader_result; REQUIRE(index_is_open(idx_file)); - // Check header auto const* hdr = index_header(idx_file); REQUIRE(hdr != nullptr); REQUIRE(std::memcmp(hdr->magic.data(), INDEX_MAGIC.data(), 4) == 0); @@ -513,10 +491,8 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") REQUIRE(hdr->command_count == 1); REQUIRE(hdr->edge_count == 3); - // Verify checksum REQUIRE(index_verify_checksum(idx_file)); - // Read full index auto read_result = read_index(idx_file); REQUIRE(read_result.has_value()); @@ -525,7 +501,6 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") REQUIRE(restored.command_count() == 1); REQUIRE(restored.edge_count() == 3); - // Verify file content (paths are computed from parent chain) auto* file1 = find_file_by_path(restored, "src/main.cpp"); REQUIRE(file1 != nullptr); REQUIRE(file1->id == 3); @@ -536,7 +511,6 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") REQUIRE(file2->id == 4); REQUIRE(file2->type == NodeType::Generated); - // Verify command (ID computed from position: node_id::make_command(0 + 1)) auto* cmd = restored.find_command_by_id(cmd_id); REQUIRE(cmd != nullptr); REQUIRE(cmd->instruction_pattern == intern("g++ -c %f -o %o")); @@ -545,17 +519,14 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") REQUIRE(cmd->inputs == pup::Vec { 3 }); REQUIRE(cmd->outputs == pup::Vec { 4 }); - // v8: Verify command reconstruction from template + operands auto reconstructed = get_command_string(restored, *cmd); REQUIRE(sv(reconstructed) =="g++ -c src/main.cpp -o build/main.o"); - // Verify header file (implicit dependency) auto* header = find_file_by_path(restored, "/usr/include/stdio.h"); REQUIRE(header != nullptr); REQUIRE(header->id == 5); REQUIRE(header->size == 8192); - // Verify implicit edge auto edges_to_cmd = restored.edges_to(cmd_id); REQUIRE(edges_to_cmd.size() == 2); auto found_implicit = false; @@ -573,18 +544,8 @@ TEST_CASE("Index serialization roundtrip", "[e2e][index]") TEST_CASE("Index ID contiguity requirement", "[e2e][index]") { - // This test documents a design constraint: IDs must be contiguous when - // stored in the index. The index format assigns IDs from array position - // on load (id = array_index + 1), so if there are gaps in stored IDs, - // parent_id references will be broken after round-trip. - // - // The build system ensures ID contiguity by storing ALL node types - // (including Ghost, Variable, Group) rather than skipping them. - // This test verifies the consequence of violating this constraint. - auto index = Index {}; - // Create entries with consecutive IDs (no gaps) index.add_file(FileEntry { .id = 1, .parent_id = 0, @@ -599,7 +560,6 @@ TEST_CASE("Index ID contiguity requirement", "[e2e][index]") .name = intern("build"), }); - // ID 3: placeholder (like a Ghost node would be stored) index.add_file(FileEntry { .id = 3, .parent_id = 0, @@ -607,7 +567,6 @@ TEST_CASE("Index ID contiguity requirement", "[e2e][index]") .name = intern("placeholder"), }); - // ID 4: subdirectory under src (parent=1) index.add_file(FileEntry { .id = 4, .parent_id = 1, @@ -615,7 +574,6 @@ TEST_CASE("Index ID contiguity requirement", "[e2e][index]") .name = intern("lib"), }); - // ID 5: file under lib (parent=4) index.add_file(FileEntry { .id = 5, .parent_id = 4, @@ -624,7 +582,6 @@ TEST_CASE("Index ID contiguity requirement", "[e2e][index]") .size = 100, }); - // Serialize and read back auto temp_path = pup::test::temp_path("pup_test_contiguous").string(); auto write_result = write_index(temp_path, index); REQUIRE(write_result.has_value()); @@ -637,7 +594,6 @@ TEST_CASE("Index ID contiguity requirement", "[e2e][index]") auto& restored = *read_result; - // With consecutive IDs, path reconstruction works correctly auto* foo = find_file_by_path(restored, "src/lib/foo.c"); REQUIRE(foo != nullptr); REQUIRE(foo->size == 100); @@ -662,10 +618,6 @@ TEST_CASE("Index reader validation", "[e2e][index]") TEST_CASE("A record whose declared layout does not fit the file is refused", "[e2e][index]") { - // Reading it as empty was the old behaviour: an out-of-bounds section became a zero-length - // span, so a damaged record loaded as the record of a project that had produced nothing -- - // and the source-file guard, which reads exactly that, went quiet and let a rule overwrite - // a checked-in file (#293). auto const cmd_id = node_id::make_command(1); auto index = Index {}; index.add_file(FileEntry { .id = 1, .name = intern("test.c") }); @@ -728,7 +680,6 @@ TEST_CASE("StringTable overflow handling", "[index]") { auto index = Index {}; - // Create a string larger than 64KB (0xFFFF = 65535 bytes max) auto huge_name = Buf {}; huge_name.resize(65536); std::memset(huge_name.data(), 'x', 65536); @@ -745,7 +696,6 @@ TEST_CASE("StringTable overflow handling", "[index]") { auto index = Index {}; - // Create a string exactly at the 64KB limit (65535 bytes) auto max_name = Buf {}; max_name.resize(65535); std::memset(max_name.data(), 'y', 65535); @@ -959,7 +909,6 @@ TEST_CASE("StringTable deduplication", "[index]") auto data = serialize_index(index); REQUIRE(data.has_value()); - // v6 length-prefixed: 2 (empty) + 2 (length) + 8 (main.cpp) = 12 auto const* hdr = reinterpret_cast(data->data()); REQUIRE(hdr->string_table_size == 12); } @@ -974,7 +923,6 @@ TEST_CASE("StringTable deduplication", "[index]") auto data = serialize_index(index); REQUIRE(data.has_value()); - // v6 length-prefixed: 2 (empty) + 2 (length) + 20 (path) = 24 auto const* hdr = reinterpret_cast(data->data()); REQUIRE(hdr->string_table_size == 24); } @@ -989,7 +937,6 @@ TEST_CASE("StringTable deduplication", "[index]") auto data = serialize_index(index); REQUIRE(data.has_value()); - // v8 length-prefixed: 2 (empty) + 2 (length) + 3 (gcc) = 7 auto const* hdr = reinterpret_cast(data->data()); REQUIRE(hdr->string_table_size == 7); } @@ -999,13 +946,11 @@ TEST_CASE("v8 template reconstruction", "[index][v8]") { auto index = Index {}; - // Create directory structure for path reconstruction index.add_file(FileEntry { .id = 1, .parent_id = 0, .type = NodeType::Directory, .name = intern("src") }); index.add_file(FileEntry { .id = 2, .parent_id = 0, .type = NodeType::Directory, .name = intern("build") }); index.add_file(FileEntry { .id = 3, .parent_id = 1, .type = NodeType::File, .name = intern("main.cpp") }); index.add_file(FileEntry { .id = 4, .parent_id = 2, .type = NodeType::Generated, .name = intern("main.o") }); - // Compute paths from parent chain index.compute_paths(); SECTION("get_command_string with %f and %o") @@ -1013,8 +958,8 @@ TEST_CASE("v8 template reconstruction", "[index][v8]") auto cmd = CommandEntry { .id = node_id::make_command(1), .instruction_pattern = intern("g++ -c %f -o %o"), - .inputs = { 3 }, // main.cpp - .outputs = { 4 }, // main.o + .inputs = { 3 }, + .outputs = { 4 }, }; index.add_command(cmd); @@ -1179,10 +1124,6 @@ TEST_CASE("v8 cross-directory path relativization", "[index][v8]") { auto index = Index {}; - // Create directory structure: - // lib/foo.c, lib/foo.o - // app/main.c, app/app - // Command runs from "app" directory, referencing "../lib/foo.o" index.add_file(FileEntry { .id = 1, .parent_id = 0, .type = NodeType::Directory, .name = intern("lib"), .path = intern("lib") }); index.add_file(FileEntry { .id = 2, .parent_id = 0, .type = NodeType::Directory, .name = intern("app"), .path = intern("app") }); index.add_file(FileEntry { .id = 3, .parent_id = 1, .type = NodeType::File, .name = intern("foo.c"), .path = intern("lib/foo.c") }); @@ -1192,27 +1133,24 @@ TEST_CASE("v8 cross-directory path relativization", "[index][v8]") SECTION("command in subdirectory referencing sibling directory") { - // Command runs from "app" directory, links with ../lib/foo.o auto cmd = CommandEntry { .id = node_id::make_command(1), - .dir_id = 2, // app directory + .dir_id = 2, .instruction_pattern = intern("gcc %f -o %o"), - .inputs = { 5, 4 }, // main.c, foo.o (from lib) - .outputs = { 6 }, // app + .inputs = { 5, 4 }, + .outputs = { 6 }, }; index.add_command(cmd); auto result = get_command_string(index, cmd); - // Paths should be relative to "app" directory REQUIRE(sv(result) =="gcc main.c ../lib/foo.o -o app"); } SECTION("command in root referencing subdirectory files") { - // Command runs from root, uses full paths auto cmd = CommandEntry { .id = node_id::make_command(1), - .dir_id = 0, // root directory (no relativization) + .dir_id = 0, .instruction_pattern = intern("gcc %f -o %o"), .inputs = { 5, 4 }, .outputs = { 6 }, @@ -1220,24 +1158,21 @@ TEST_CASE("v8 cross-directory path relativization", "[index][v8]") index.add_command(cmd); auto result = get_command_string(index, cmd); - // Paths should be full since dir_id is 0 REQUIRE(sv(result) =="gcc app/main.c lib/foo.o -o app/app"); } SECTION("command in subdirectory with same-directory files") { - // Command runs from "lib" directory, all files are local auto cmd = CommandEntry { .id = node_id::make_command(1), - .dir_id = 1, // lib directory + .dir_id = 1, .instruction_pattern = intern("gcc -c %f -o %o"), - .inputs = { 3 }, // foo.c - .outputs = { 4 }, // foo.o + .inputs = { 3 }, + .outputs = { 4 }, }; index.add_command(cmd); auto result = get_command_string(index, cmd); - // Paths should be relative to "lib" directory REQUIRE(sv(result) =="gcc -c foo.c -o foo.o"); } } @@ -1246,14 +1181,12 @@ TEST_CASE("v8 build_command_lookup", "[index][v8]") { auto index = Index {}; - // Create file entries index.add_file(FileEntry { .id = 1, .parent_id = 0, .type = NodeType::File, .name = intern("foo.c") }); index.add_file(FileEntry { .id = 2, .parent_id = 0, .type = NodeType::Generated, .name = intern("foo.o") }); index.add_file(FileEntry { .id = 3, .parent_id = 0, .type = NodeType::File, .name = intern("bar.c") }); index.add_file(FileEntry { .id = 4, .parent_id = 0, .type = NodeType::Generated, .name = intern("bar.o") }); index.compute_paths(); - // Add commands with template + operands index.add_command(CommandEntry { .id = node_id::make_command(1), .instruction_pattern = intern("gcc -c %f -o %o"), @@ -1288,8 +1221,6 @@ TEST_CASE("v8 build_command_lookup", "[index][v8]") { auto lookup = build_command_lookup(index); - // Both commands have same template but different operands - // So they should be distinct in the lookup REQUIRE(lookup.size() == 2); auto const* cmd1 = lookup.at("gcc -c foo.c -o foo.o"); @@ -1304,7 +1235,6 @@ TEST_CASE("v8 roundtrip with operand sections", "[e2e][index][v8]") { auto index = Index {}; - // Create file structure index.add_file(FileEntry { .id = 1, .parent_id = 0, .type = NodeType::Directory, .name = intern("src") }); index.add_file(FileEntry { .id = 2, .parent_id = 0, .type = NodeType::Directory, .name = intern("build") }); index.add_file(FileEntry { .id = 3, .parent_id = 1, .type = NodeType::File, .name = intern("main.cpp") }); @@ -1317,7 +1247,6 @@ TEST_CASE("v8 roundtrip with operand sections", "[e2e][index][v8]") auto cmd2_id = node_id::make_command(2); auto cmd3_id = node_id::make_command(3); - // Commands with templates + operands index.add_command(CommandEntry { .id = cmd1_id, .instruction_pattern = intern("g++ -c %f -o %o"), @@ -1340,11 +1269,9 @@ TEST_CASE("v8 roundtrip with operand sections", "[e2e][index][v8]") .outputs = { 7 }, }); - // Serialize auto data = serialize_index(index); REQUIRE(data.has_value()); - // Write and read back auto temp_path = pup::test::temp_path("pup_v8_roundtrip_test").string(); auto write_result = write_index(temp_path, index); REQUIRE(write_result.has_value()); @@ -1357,11 +1284,9 @@ TEST_CASE("v8 roundtrip with operand sections", "[e2e][index][v8]") auto& restored = *read_result; - // Verify structure REQUIRE(restored.file_count() == 7); REQUIRE(restored.command_count() == 3); - // Verify commands retain template + operands auto* cmd1 = restored.find_command_by_id(cmd1_id); REQUIRE(cmd1 != nullptr); REQUIRE(cmd1->instruction_pattern == intern("g++ -c %f -o %o")); @@ -1380,7 +1305,6 @@ TEST_CASE("v8 roundtrip with operand sections", "[e2e][index][v8]") REQUIRE(cmd3->inputs == pup::Vec { 5, 6 }); REQUIRE(cmd3->outputs == pup::Vec { 7 }); - // Verify command reconstruction auto cmd1_str = get_command_string(restored, *cmd1); REQUIRE(sv(cmd1_str) =="g++ -c src/main.cpp -o build/main.o"); @@ -1482,7 +1406,7 @@ auto require_graph_index_roundtrip(pup::graph::BuildGraph const& bs, std::string REQUIRE(sorted_edge_labels_of_index(*loaded) == sorted_edge_labels_of_graph(bs.graph)); } -} // namespace +} TEST_CASE("serialize_index rejects a non-dense file id sequence", "[index]") { @@ -1695,7 +1619,7 @@ auto sorted_paths_of_type(Index const& index, NodeType type) -> Vec return out; } -} // namespace +} TEST_CASE("A record too short to hold a header is damage rather than a format this reader skips", "[index]") { @@ -1727,7 +1651,6 @@ TEST_CASE("A file that does not carry the index magic is damage", "[index]") REQUIRE(data.has_value()); auto bytes = *data; - // No re-signing: the magic check runs before the checksum, so this is the arm under test. for (auto i = std::size_t { 0 }; i < INDEX_MAGIC.size(); ++i) { bytes[i] = std::byte { 'X' }; } @@ -1830,8 +1753,6 @@ TEST_CASE("An operand boundary that does not span its operands makes the record std::filesystem::remove(path); } -// Both wrap tests discriminate only because the position they wrap onto holds something: a wrap -// landing on zeroes would be rejected for the wrong reason, not for the wrap the test is about. static_assert(offsetof(RawHeader, file_count) == 8, "the operand wrap test lands on this field"); static_assert(offsetof(RawFileEntry, name_offset) == 8, "the name wrap test patches this field"); static_assert(offsetof(RawFileEntry, size) == 16, "the name wrap test plants its string here"); @@ -1883,7 +1804,6 @@ TEST_CASE("A name offset that wraps makes the record unreadable", "[index]") auto const* hdr = reinterpret_cast(bytes.data()); REQUIRE(hdr->file_count > 0); - // Planted in this entry's `size`, which nothing reads back, so the wrapped read finds a name. auto const planted = hdr->file_offset + offsetof(RawFileEntry, size); REQUIRE(planted < hdr->string_offset); auto constexpr PLANTED_NAME = std::string_view { "ghost!" }; @@ -1906,7 +1826,6 @@ TEST_CASE("A name offset that wraps makes the record unreadable", "[index]") REQUIRE_FALSE(restored.has_value()); REQUIRE(restored.error().code == ErrorCode::IndexDamaged); - // The record that names it is unreadable, so the recovery read must not answer for it either. opened->file.close(); REQUIRE(read_prior_paths(path).kind == PriorPaths::Kind::Lost); @@ -1938,7 +1857,6 @@ TEST_CASE("A record whose entry carries a type this putup cannot name is unreada REQUIRE_FALSE(restored.has_value()); REQUIRE(restored.error().code == ErrorCode::IndexDamaged); - // The recovery read builds the same entries, so it must not answer for this record either. opened->file.close(); REQUIRE(read_prior_paths(path).kind == PriorPaths::Kind::Lost); @@ -1972,7 +1890,6 @@ TEST_CASE("A record whose edge carries a link type this putup cannot name is unr REQUIRE_FALSE(restored.has_value()); REQUIRE(restored.error().code == ErrorCode::IndexDamaged); - // Damage in a section the recovery read never looks at leaves the recorded paths readable. opened->file.close(); REQUIRE(read_prior_paths(path).kind == PriorPaths::Kind::Known); @@ -1993,7 +1910,6 @@ TEST_CASE("A record whose edge carries link type zero is unreadable", "[index]") // NOLINTNEXTLINE(cppcoreguidelines-pro-type-reinterpret-cast) auto const* hdr = reinterpret_cast(bytes.data()); REQUIRE(hdr->edge_count > 0); - // LinkType starts at 1, so zero is as unnameable as anything past the last enumerator. bytes[hdr->edge_offset + offsetof(RawEdge, type)] = std::byte { 0 }; stamp_version(bytes, INDEX_VERSION); @@ -2136,7 +2052,6 @@ TEST_CASE("An empty recorded string is a value rather than a failure", "[index]" auto index = Index {}; index.add_file(FileEntry { .id = 1, .parent_id = 0, .name = intern("main.c") }); auto cmd_id = node_id::make_command(1); - // env is semantics-bearing and empty here, which is exactly the value a failure must not mimic. index.add_command(CommandEntry { .id = cmd_id, .instruction_pattern = intern("cc %f"), .display = StringId::Empty, .env = StringId::Empty, .inputs = { 1 } }); auto data = serialize_index(index); @@ -2169,7 +2084,6 @@ TEST_CASE("A record names the paths it recorded even at a version too old to tru REQUIRE(data.has_value()); auto const path = temp_index_path("pup_prior_window"); - // The window is a handful of integers, so it is enumerated rather than sampled. for (auto version = INDEX_LAYOUT_FLOOR - 1; version <= INDEX_VERSION + 1; ++version) { INFO("version=" << version); auto bytes = *data; @@ -2195,8 +2109,6 @@ TEST_CASE("A record names the paths it recorded even at a version too old to tru TEST_CASE("A record that classifies one path two ways is unreadable", "[index]") { - // The three lists partition the file table, so one path in two of them proves the record - // holds two entries for it -- a state its own writer forbids (#382). auto const pairs = std::array { std::pair { NodeType::File, NodeType::Generated }, std::pair { NodeType::File, NodeType::Ghost }, @@ -2226,8 +2138,6 @@ TEST_CASE("A record that classifies one path two ways is unreadable", "[index]") TEST_CASE("A record that names one path twice with one type is unreadable", "[index]") { - // The same rule from inside a single list: two entries for one path, whichever list they - // land in. Crafted bytes are not bound by the writer's funnel (#382). for (auto const type : { NodeType::File, NodeType::Generated, NodeType::Ghost }) { INFO("type=" << static_cast(type)); auto index = Index {}; @@ -2251,8 +2161,6 @@ TEST_CASE("A record that names one path twice with one type is unreadable", "[in TEST_CASE("A record that names one path once in each of two directories is readable", "[index]") { - // The positive control for the rejection above: same basename, different paths, which is - // what an out-of-tree build records for a source and the output that shadows it. auto index = Index {}; index.add_file(FileEntry { .id = 1, .type = NodeType::Directory, .name = intern("a") }); index.add_file(FileEntry { .id = 2, .type = NodeType::Directory, .name = intern("build") }); @@ -2330,8 +2238,6 @@ TEST_CASE("One flipped bit anywhere in a record makes it unrecoverable, never pa std::filesystem::remove(path); } -// The mirror of the recovery-path property above, for the path every build takes: a record that -// is not the one putup wrote is not evidence, whichever door it is read through (#294). TEST_CASE("One flipped bit anywhere in a record makes it unreadable", "[index]") { auto const index = index_with_every_node_type(); @@ -2375,8 +2281,6 @@ TEST_CASE("Keying a record's file table by path keeps every addressable entry an keyed.push_back(entry); } - // Same entries, each once: an index may record two nodes at one path, and dropping - // either would make the map disagree with the table it is derived from. std::sort(expected.begin(), expected.end()); std::sort(keyed.begin(), keyed.end()); REQUIRE(keyed == expected); @@ -2411,8 +2315,6 @@ TEST_CASE("Keying the index under construction by path answers the same however map.insert(paths[i], NodeId { static_cast(i + 1) }); } - // The walks that build this map also query it, so what it answers cannot depend on how - // far along the filling was — the insertion order is the graph's, not the caller's choice. for (auto i = std::size_t { 0 }; i < paths.size(); ++i) { auto found = map.find(paths[i]); REQUIRE(found.has_value()); diff --git a/test/unit/test_instruction.cpp b/test/unit/test_instruction.cpp index 0722fa5b..41cafb07 100644 --- a/test/unit/test_instruction.cpp +++ b/test/unit/test_instruction.cpp @@ -100,7 +100,7 @@ auto describe(Instruction const& atoms) -> std::string return out; } -} // namespace +} SCENARIO("Text appended as a literal never expands", "[instruction]") {