fix: refuse a build that holds two builds of one module #42

Closed
jorijnvdgraaf wants to merge 0 commits from claude/issue-41 into master

Fixes #41. Stacked on #40: merge that first. Until then, this PR also shows #40's commits; its own are the last three.

What was wrong

A project that two paths load with different arg lists is two configurations, each with a variant of its own, and Build() built both. The consumer's compile got both PCM dirs on -fprebuilt-module-path. Clang resolved the module by name to the first dir for every import, including the import inside the other build's consumers, and nothing checked that it was the build they had been compiled against. In #41's repro (P → M, P → A → M, where the two lists change a struct M exports), the build exited 0 and the program read garbage or crashed.

The fix

Before a configuration makes a directory or builds a dependency, Build() checks it and its transitive dependencies for two configurations that export a module of the same name. If it finds them, it stops with both paths and what tells the two builds apart:

module M is built twice for P:
  P -> M (<scratch>/m/, M-x86_64-pc-linux-gnu-native-native-9a004cf3f25748f5, project args ["--split"])
  P -> A -> M (<scratch>/m/, M-x86_64-pc-linux-gnu-native-native-4b9c9c270ea53b40, project args ["--split", "--wide"])
A program can hold one build of each module: load M with the same args, and from the same place, along every path.
  • By module name, not by project file. This also catches a checkout on one path meeting a git clone of the same project on the other, for example when only one path honours --local.
  • Per configuration, not per invocation. Two builds of one project in different programs are fine, such as a test and a #pragma crafter dependency tool.
  • Not done: picking one of the two builds. Arg lists have no merge rule, so the consumer that loses would silently get a different build from the one it asked for.

The README and the GitProjectSpec comment, which #40 has saying both builds end up in the program, now say the build stops.

Testing

  • New tests/DuplicateModule, with #41's diamond as the fixture:
    • Two lists: Build() fails, names the module and both paths, and leaves no bin/ anywhere.
    • One list: there is one M, and the build goes through.
  • With the check disabled, all four checks of the two-list case fail.
  • format and lint are clean. The full suite gives 37 passed and 2 skipped, the same environment skips as #40 (wasmtime missing, no CRAFTER_BUILD_HOME).
  • The self-host build passes, so no existing graph in this repo is refused.

No current Catcrafts graph hits this: the projects that change their list give that one list to all of their siblings.

🤖 Generated with Claude Code

Fixes #41. **Stacked on #40:** merge that first. Until then, this PR also shows #40's commits; its own are the last three. ## What was wrong A project that two paths load with different arg lists is two configurations, each with a variant of its own, and `Build()` built both. The consumer's compile got both PCM dirs on `-fprebuilt-module-path`. Clang resolved the module by name to the first dir for every import, including the import inside the other build's consumers, and nothing checked that it was the build they had been compiled against. In #41's repro (`P → M`, `P → A → M`, where the two lists change a struct `M` exports), the build exited 0 and the program read garbage or crashed. ## The fix Before a configuration makes a directory or builds a dependency, `Build()` checks it and its transitive dependencies for two configurations that export a module of the same name. If it finds them, it stops with both paths and what tells the two builds apart: ``` module M is built twice for P: P -> M (<scratch>/m/, M-x86_64-pc-linux-gnu-native-native-9a004cf3f25748f5, project args ["--split"]) P -> A -> M (<scratch>/m/, M-x86_64-pc-linux-gnu-native-native-4b9c9c270ea53b40, project args ["--split", "--wide"]) A program can hold one build of each module: load M with the same args, and from the same place, along every path. ``` - **By module name, not by project file.** This also catches a checkout on one path meeting a git clone of the same project on the other, for example when only one path honours `--local`. - **Per configuration, not per invocation.** Two builds of one project in different programs are fine, such as a test and a `#pragma crafter dependency` tool. - **Not done:** picking one of the two builds. Arg lists have no merge rule, so the consumer that loses would silently get a different build from the one it asked for. The README and the `GitProjectSpec` comment, which #40 has saying both builds end up in the program, now say the build stops. ## Testing - New `tests/DuplicateModule`, with #41's diamond as the fixture: - Two lists: `Build()` fails, names the module and both paths, and leaves no `bin/` anywhere. - One list: there is one `M`, and the build goes through. - With the check disabled, all four checks of the two-list case fail. - `format` and `lint` are clean. The full suite gives 37 passed and 2 skipped, the same environment skips as #40 (wasmtime missing, no `CRAFTER_BUILD_HOME`). - The self-host build passes, so no existing graph in this repo is refused. No current Catcrafts graph hits this: the projects that change their list give that one list to all of their siblings. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Every project.cpp with a sibling dependency opened with the same line,

    std::vector<std::string> depArgs(args.begin(), args.end());

and handed depArgs to each GitProject and LocalProject. CrafterBuildProject
receives a span of string_views and the specs wanted owned strings, and
without the copy the sibling got none of the project's args: it was built
for the host while the project cross-compiled, and in release while it
built --debug. GitProject then turned the copy straight back into
string_views for LoadProject.

Both specs now default their args to ProjectArgs(), the args LoadProject
was given for the project file whose CrafterBuildProject is running. It
reads the per-thread stack LoadProject already keeps for IsTopLevelProject
and ProjectDependency, whose frames now record the args. A default member
initializer rather than a new type or an optional: the member stays a
std::vector<std::string>, so every existing `.args = depArgs` keeps its
exact meaning (each of the ~50 calls across the Catcrafts projects passes
one), and `.args = {}` still forwards nothing. It is the shape of
Configuration::target = HostTarget(). A project.cpp that drops its .args
needs a crafter-build with this change: an older one compiles it and
forwards nothing.

ProjectArgs() is declared in :External, next to the defaults that call
it. :Platform, where it is defined, cannot be imported there: its
unexported `struct Configuration;` would make :External's exported one an
ill-formed redeclaration, which clang rejects.

The #pragma crafter dependency path now passes `.args = {}` explicitly.
Host tooling is built for the host whatever the consumer targets and was
loaded with no args; the default would otherwise hand it the consumer's
--target= and project flags.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ProjectArgs loads a chain app -> lib -> leaf of LocalProjects with --debug
and a project flag. lib, loaded without .args, has to see both, and so
does its own sibling, which takes the default one level further down.
Copies loaded with `.args = {}` and with an explicit list get nothing and
exactly that list. The GitProjectSpec default is checked on the spec
itself, since a GitProject would need a repository to clone. Without the
defaults, five of its checks fail.

ProjectDependency now also loads its consumer with a project flag: the
pragma dependency must still come back with no args. Without the explicit
`.args = {}` in the pragma path it gets the consumer's flag.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Crafter.Math and Crafter.Asset now get this project's args through the
GitProjectSpec default. Loading this file therefore needs a crafter-build
that exports ProjectArgs: after pulling, run ./build.sh before the
bootstrap binary loads it.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Crafter.LintRules now has AddLiteRules: the house rules without the ones
that only decide layout, whitespace and naming. This tree switches to it.
The full set's naming rule now wants camelCase statics and constants,
which this tree spells PascalCase throughout, so `lint` with it reported
49 findings on code that follows the documented convention; with the lite
set `lint` and `format --check` are clean.

AGENTS.md described the 17 rules as the enforced style and told
contributors to let `format` restyle their code. It now says which 8 rules
`lint` enforces, that `format` only rewrites string concatenation and
builtin integer types, and lists layout and naming as conventions kept by
hand.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs: sibling projects forward the parent's args by default
All checks were successful
Flatpak / flatpak (pull_request) Successful in 7m57s
5cf295e1d4
The sibling-project example dropped its `.args = depArgs`, which no
longer has to be written, and says what a sibling receives instead: the
args the project file was loaded with, or whatever `.args` is set to,
with `{}` for nothing. It also says that releases up to 2.2.0 forward
nothing without `.args`, which matters for a project.cpp that has to
load on both.

The pragma section says that a project file's own dependency is built
for the host and gets none of the project's args, the one place the
default does not apply.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ProjectArgs() read args that LoadProject recorded when it pushed the
frame, so they were there for everything that ran on that frame, not
only for CrafterBuildProject. Two things went wrong with that.

The pragma pre-pass runs on the frame too, so both of its specs needed
a hand-written `.args = {}` to keep the consumer's args from reaching
host tooling, and anything else built there would have needed one as
well. And dlopen runs a project file's static initialisers only on the
first load of that file: a later load gets the same mapping back. A
spec made at namespace scope therefore kept the first load's args for
every later load of the file, a silently wrong variant.

The frame now gets the args just before CrafterBuildProject is called.
The pre-pass and the static initialisers see none, on every load, so
the two opt-outs go. The dependencies the pre-pass resolved move the
other way, to just before dlopen: they are the same on every load, and
ProjectDependency() called from a static initialiser used to find none
and terminate the process.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ProjectArgs gets a sibling that makes its own sibling's spec at
namespace scope. Before the fix that spec took the args the file was
first loaded with; now it takes none on every load. The checks go
through one CheckLoaded helper, which prints what each sibling was
given, so a failure says which args a wrong frame handed over.

ProjectDependency loads a fixture whose static initialiser asks for its
pragma dependency, in a child, because a throw there terminates the
process: before the fix it found none. The check that the dependency
gets none of the consumer's args moves into the first load, which now
passes the app an arg it ignores; the separate load could not fail its
own check, and its early return skipped every later one.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
ForceHostTooling rewrites a `#pragma crafter dependency` and everything
it loads in place: host target, march native, release, -fPIC. Those are
entries of the project cache like any other, keyed by project file and
args alone. A project that loaded the same sibling as its tool, with
the same args, got the rewritten entry back. That happens whenever the
project gets no args, since host tooling gets none either: a consumer
built for x86-64-v2 then imported a sibling's BMI compiled with
-march=native, and clang refused it. Any arg on the command line split
the keys and hid it.

A thread-local count of pragma loads now marks the tool's load and
every load inside it, its project file's siblings included, and
GitProject and LocalProject put that mark in the key. The clone stays
shared; only the Configuration is separate, as for two arg lists. A
module-internal function rather than a spec field, because the
siblings of a tool are loaded by the tool's own project.cpp, which
cannot know it is host tooling.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fixture app has `#pragma crafter dependency "../tool"` and loads
../shared for itself, which the tool loads too; the app is loaded with
no args, the case where the two cache keys used to match. With
CRAFTER_BUILD_MARCH=x86-64-v2, the app's copy of shared has to keep
that march and no -fPIC, and the tool's copy has to be host tooling.
Before the fix the app got the tool's copy back: one Configuration,
with march native and -fPIC.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The help has advertised `[options] [-- project-args...]`, but Run never
looked for `--`. It fell through to projectArgs as a word of its own,
and the words after it were still read as this tool's options and
verbs: `crafter-build -- -v` turned verbose output on. In projectArgs
`--` changed the project's variant hash, and now that siblings inherit
the args, the hash of every sibling down the chain, each building into
directories of its own that the project's clean never removes.

`--` now ends this tool's options and goes nowhere itself. Every later
word is an operand of the verb, as POSIX has it: a project arg for a
build, a glob for test, lint and format, the script's name and then its
args for run. A glob rather than a project arg after `test` because
`crafter-build test -- 'House*'` is how such a filter gets typed; a
project arg goes before the `--`.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
`run -- record -- a` has to run the record script with the args `--`
and `a`: the first `--` ends the options, the second comes after the
script's name and is the script's. The project args keep only what
stood before `run`. And `run -- -x` has to look for a script called
-x, since after `--` an option-like word is an operand. Before the fix
the first `--` showed up in the project args, and `-x` joined it there
while the bare `run` listed the scripts and succeeded.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs: correct what the sibling-args docs promise
All checks were successful
Flatpak / flatpak (pull_request) Successful in 7m27s
84d9d66649
The README suggested giving one sibling a list without a flag only the
project reads, so that the sibling would not get a build directory of
its own. When another path loads the same sibling with the full list,
that filter is what gives it the second directory: two builds of one
project, both in the program (#41). It now says to build such a list
once and give it to every sibling. It also says the default is read
when the spec is made, so a spec made at namespace scope, on another
thread or in a script forwards nothing, and that cfg.projectArgs, the
part ApplyStandardArgs did not interpret, is no substitute: it drops
the target and the build type. The comments on ProjectArgs() and on
Configuration::projectArgs say the same.

The spec comment named --local as a flag the default carries down a
chain of siblings. Through a GitProject hop it breaks: the clone has no
checkouts beside it. The GitProject comment said the clone is keyed by
the args; only the Configuration is, and every arg list builds in the
one clone. AGENTS.md now notes that a stale harness gives no symptom
for a changed default member initialiser: project.cpp silently gets the
old default.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
A project that two paths load with different arg lists is two
configurations, each with a variant of its own, and Build() built both.
The consumer's compile got both PCM dirs on its -fprebuilt-module-path,
and clang resolved the module by name to the first for every import,
also for the import inside the other build's BMI consumers, with no
check that it was the build they had been compiled against. With
P -> M and P -> A -> M, where the two lists change a struct M exports,
P compiled against one layout, A against the other, and the program
built cleanly, then read garbage or crashed (#41).

Each configuration now checks, before it makes a directory or builds a
dependency, that no two configurations among it and its transitive
dependencies export a module of the same name, and otherwise fails with
both paths and what tells the builds apart. Going by module name rather
than by project file also catches a checkout on one path and a clone of
the same project on the other. Per configuration, not per invocation:
two builds of one project in different programs, such as a test and a
#pragma crafter dependency tool, are fine.

Picking one of the two builds was not an option: arg lists have no merge
rule, so whichever consumer lost would silently get a different build
from the one it asked for.

Fixes #41

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
The fixture is P -> M and P -> A -> M, where M's own flag --wide
changes a struct it exports. Loaded with --wide --split, P gives M its
args without --wide while A passes them on whole: Build(P) has to fail,
name the module and both paths, and leave no bin/ anywhere. Loaded with
--wide alone, both paths give M the same list, there is one M, and the
build has to go through. Before the fix the first case built cleanly.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
docs: two lists for one sibling now stop the build
All checks were successful
Flatpak / flatpak (pull_request) Successful in 7m49s
34874c7143
The README and the spec comment said that a project two paths load with
different lists ends up in the program twice. They now say the build
stops, with an error that names both paths.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
Author
Owner

Merged into master as eefd1df (Merge branch 'claude/issue-41'), right after #40; it merged cleanly. #41 was closed by the fix commit. Closed by hand because the repository does not allow marking a PR as manually merged.

Merged into master as eefd1df (Merge branch 'claude/issue-41'), right after #40; it merged cleanly. #41 was closed by the fix commit. Closed by hand because the repository does not allow marking a PR as manually merged.
jorijnvdgraaf closed this pull request 2026-10-31 04:32:08 +00:00
All checks were successful
Flatpak / flatpak (pull_request) Successful in 7m49s

Pull request closed

Sign in to join this conversation.
No description provided.