A sibling loaded with two arg lists is built twice, and both builds end up in one program #41

New issue

Closed

opened 2026-10-31 03:45:45 +00:00 by jorijnvdgraaf · 0 comments

jorijnvdgraaf commented

2026-10-31 03:45:45 +00:00

Owner

Summary

When a project reaches the same sibling along two paths with different arg lists, the sibling is loaded and built twice, and both builds go into one program without a word from crafter-build. If the lists change what the sibling declares, that program is put together from two incompatible versions of one module. In the repro below the build succeeds and the program reads garbage; an earlier variant of the same repro segfaulted.

The graph is P → M and P → A → M:

M reads a flag of its own: --wide adds -DWIDE, which gives the exported struct S more members.
A loads M with the default args and exports MakeS(), which returns an S.
P sends M its args without --wide, and loads A with the default.

crafter-build --wide in P, then the program. Both exit 0:

P->M [], P->A->M ["--wide"], one M: false
[9/10] Compiling interface M
[10/11] Compiling interface M
[13/13] Compiling interface A
[15/15] Compiling main.cpp
[16/16] Linking P
Built 16 steps in 4802ms

P: sizeof(S) = 8, A: sizeof(S) = 40
s.b = 32712

With --wide on both paths there is one M, and the program prints 40, 40 and s.b = 2.

The fixture relies on the default args from #40; on master, .args = depArgs in A builds the same graph. Line numbers refer to 6e2d835.

Fixture: three LocalProject projects, no network

m/interfaces/M.cppm

export module M;
#ifdef WIDE
export struct S { int a; int b; long long pad[4]; };
#else
export struct S { int a; int b; };
#endif
export int Size() { return sizeof(S); }

m/project.cpp

import std;
import Crafter.Build;
namespace fs = std::filesystem;
using namespace Crafter;
// --wide is M's own flag: it changes what M declares.
extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) {
    Configuration cfg;
    cfg.path = "./";
    cfg.name = "M";
    cfg.outputName = "M";
    cfg.type = ConfigurationType::LibraryStatic;
    ApplyStandardArgs(cfg, args);
    if (std::ranges::contains(args, std::string_view("--wide"))) cfg.compileFlags.push_back("-DWIDE");
    std::array<fs::path, 1> ifaces = { "interfaces/M" };
    std::array<fs::path, 0> impls = {};
    cfg.GetInterfacesAndImplementations(ifaces, impls);
    return cfg;
}

a/interfaces/A.cppm

export module A;
import M;
export int ASize() { return Size(); }
export S MakeS() { return S{ 1, 2 }; }

a/project.cpp

import std;
import Crafter.Build;
namespace fs = std::filesystem;
using namespace Crafter;
// A loads M with the default: whatever A was given.
extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) {
    Configuration* m = LocalProject({ .projectFile = "../m/project.cpp" });
    Configuration cfg;
    cfg.path = "./";
    cfg.name = "A";
    cfg.outputName = "A";
    cfg.type = ConfigurationType::LibraryStatic;
    ApplyStandardArgs(cfg, args);
    cfg.dependencies = { m };
    std::array<fs::path, 1> ifaces = { "interfaces/A" };
    std::array<fs::path, 0> impls = {};
    cfg.GetInterfacesAndImplementations(ifaces, impls);
    return cfg;
}

p/project.cpp

import std;
import Crafter.Build;
namespace fs = std::filesystem;
using namespace Crafter;
// P sends M its args without --wide; A gets the default and passes it all on.
extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) {
    std::vector<std::string> mArgs;
    for (std::string_view a : args) if (a != "--wide") mArgs.emplace_back(a);
    Configuration* m = LocalProject({ .projectFile = "../m/project.cpp", .args = mArgs });
    Configuration* a = LocalProject({ .projectFile = "../a/project.cpp" });
    std::println("P->M {}, P->A->M {}, one M: {}", m->projectArgs, a->dependencies[0]->projectArgs, m == a->dependencies[0]);
    Configuration cfg;
    cfg.path = "./";
    cfg.name = "P";
    cfg.outputName = "P";
    cfg.type = ConfigurationType::Executable;
    ApplyStandardArgs(cfg, args);
    cfg.dependencies = { m, a };
    std::array<fs::path, 0> ifaces = {};
    std::array<fs::path, 1> impls = { "main" };
    cfg.GetInterfacesAndImplementations(ifaces, impls);
    return cfg;
}

p/main.cpp

import std;
import M;
import A;
int main() {
    std::println(std::cerr, "P: sizeof(S) = {}, A: sizeof(S) = {}", sizeof(S), ASize());
    S s = MakeS();
    std::println(std::cerr, "s.b = {}", s.b);
}

Cause

GitProject and LocalProject key their cache on the arg list (implementations/Crafter.Build-External.cpp:368, :453), so M with [] and M with [--wide] are two Configurations with two variant directories, and both get built. That part is right: A gets the list it passes and is compiled against the M it asked for, which is why it reports 40.
GetCompileCommand walks a configuration's dependencies transitively, deduplicating by pointer (implementations/Crafter.Build-Clang.cpp:351), and puts each one's PcmDir on -fprebuilt-module-path (:358). P's compile gets both M directories, its own first. Clang finds module M by name and takes the first match, for P's import M and for the M that A's BMI imports alike, and nothing checks that it is the M that A was compiled against. P's code sees the 8-byte S, while A's object code was compiled with the 40-byte one, which is the S that MakeS() returns.

A program can contain one module named M. Each consumer getting its own M holds for each compile, but not inside P, which contains both. The two paths have to agree.

When the lists differ only in flags that don't change how M compiles (a flag only P reads, --debug), the two builds are interchangeable and the cost is building M twice. That is the case #40's README suggests a filtered list for: "a list built from the project's args, say one without a flag only this project reads (it would otherwise give the sibling a build directory of its own)". Filtering on one path is what gives the sibling its second build directory.

No current Catcrafts graph hits this: Network, Graphics-imsd-messages, HelloDom and SimpleClient each send one changed list to all of their siblings.

Suggested fix

Fail before compiling when a configuration's transitive dependencies contain the same project as two different Configurations, and name both paths with their lists:
```
M is loaded twice in P's build, with different args:
  P → M      []
  P → A → M  [--wide]
A program can contain one build of M: send it the same list on both paths.
```
"Same project" can be the project file plus the configuration name. Matching on the module names the configurations export would also catch a LocalProject checkout and a GitProject clone of one project meeting in one program, for example when only one path honours --local.
Check per program, not per run: two builds of one project are fine when they go into different programs (two executables, a test, a #pragma crafter dependency tool).
README: replace the filtered-list advice with "build a changed list once and send it to every sibling", and say what happens otherwise.
A test built on the fixture above: Build() fails with that message when the lists differ, and builds with one M when they agree.

Not suggested: picking one of the two Ms and rebuilding the other consumer against it. Arg lists have no merge rule, so the consumer that loses silently gets a different M from the one it asked for.

## Summary When a project reaches the same sibling along two paths with different arg lists, the sibling is loaded and built twice, and both builds go into one program without a word from crafter-build. If the lists change what the sibling declares, that program is put together from two incompatible versions of one module. In the repro below the build succeeds and the program reads garbage; an earlier variant of the same repro segfaulted. The graph is `P → M` and `P → A → M`: - `M` reads a flag of its own: `--wide` adds `-DWIDE`, which gives the exported `struct S` more members. - `A` loads `M` with the default args and exports `MakeS()`, which returns an `S`. - `P` sends `M` its args without `--wide`, and loads `A` with the default. `crafter-build --wide` in `P`, then the program. Both exit 0: ``` P->M [], P->A->M ["--wide"], one M: false [9/10] Compiling interface M [10/11] Compiling interface M [13/13] Compiling interface A [15/15] Compiling main.cpp [16/16] Linking P Built 16 steps in 4802ms P: sizeof(S) = 8, A: sizeof(S) = 40 s.b = 32712 ``` With `--wide` on both paths there is one `M`, and the program prints `40`, `40` and `s.b = 2`. The fixture relies on the default args from #40; on master, `.args = depArgs` in `A` builds the same graph. Line numbers refer to `6e2d835`. <details> <summary>Fixture: three LocalProject projects, no network</summary> `m/interfaces/M.cppm` ```cpp export module M; #ifdef WIDE export struct S { int a; int b; long long pad[4]; }; #else export struct S { int a; int b; }; #endif export int Size() { return sizeof(S); } ``` `m/project.cpp` ```cpp import std; import Crafter.Build; namespace fs = std::filesystem; using namespace Crafter; // --wide is M's own flag: it changes what M declares. extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) { Configuration cfg; cfg.path = "./"; cfg.name = "M"; cfg.outputName = "M"; cfg.type = ConfigurationType::LibraryStatic; ApplyStandardArgs(cfg, args); if (std::ranges::contains(args, std::string_view("--wide"))) cfg.compileFlags.push_back("-DWIDE"); std::array<fs::path, 1> ifaces = { "interfaces/M" }; std::array<fs::path, 0> impls = {}; cfg.GetInterfacesAndImplementations(ifaces, impls); return cfg; } ``` `a/interfaces/A.cppm` ```cpp export module A; import M; export int ASize() { return Size(); } export S MakeS() { return S{ 1, 2 }; } ``` `a/project.cpp` ```cpp import std; import Crafter.Build; namespace fs = std::filesystem; using namespace Crafter; // A loads M with the default: whatever A was given. extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) { Configuration* m = LocalProject({ .projectFile = "../m/project.cpp" }); Configuration cfg; cfg.path = "./"; cfg.name = "A"; cfg.outputName = "A"; cfg.type = ConfigurationType::LibraryStatic; ApplyStandardArgs(cfg, args); cfg.dependencies = { m }; std::array<fs::path, 1> ifaces = { "interfaces/A" }; std::array<fs::path, 0> impls = {}; cfg.GetInterfacesAndImplementations(ifaces, impls); return cfg; } ``` `p/project.cpp` ```cpp import std; import Crafter.Build; namespace fs = std::filesystem; using namespace Crafter; // P sends M its args without --wide; A gets the default and passes it all on. extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) { std::vector<std::string> mArgs; for (std::string_view a : args) if (a != "--wide") mArgs.emplace_back(a); Configuration* m = LocalProject({ .projectFile = "../m/project.cpp", .args = mArgs }); Configuration* a = LocalProject({ .projectFile = "../a/project.cpp" }); std::println("P->M {}, P->A->M {}, one M: {}", m->projectArgs, a->dependencies[0]->projectArgs, m == a->dependencies[0]); Configuration cfg; cfg.path = "./"; cfg.name = "P"; cfg.outputName = "P"; cfg.type = ConfigurationType::Executable; ApplyStandardArgs(cfg, args); cfg.dependencies = { m, a }; std::array<fs::path, 0> ifaces = {}; std::array<fs::path, 1> impls = { "main" }; cfg.GetInterfacesAndImplementations(ifaces, impls); return cfg; } ``` `p/main.cpp` ```cpp import std; import M; import A; int main() { std::println(std::cerr, "P: sizeof(S) = {}, A: sizeof(S) = {}", sizeof(S), ASize()); S s = MakeS(); std::println(std::cerr, "s.b = {}", s.b); } ``` </details> ## Cause - `GitProject` and `LocalProject` key their cache on the arg list (`implementations/Crafter.Build-External.cpp:368`, `:453`), so `M` with `[]` and `M` with `[--wide]` are two `Configuration`s with two variant directories, and both get built. That part is right: `A` gets the list it passes and is compiled against the `M` it asked for, which is why it reports 40. - `GetCompileCommand` walks a configuration's dependencies transitively, deduplicating by pointer (`implementations/Crafter.Build-Clang.cpp:351`), and puts each one's `PcmDir` on `-fprebuilt-module-path` (`:358`). `P`'s compile gets both `M` directories, its own first. Clang finds module `M` by name and takes the first match, for `P`'s `import M` and for the `M` that `A`'s BMI imports alike, and nothing checks that it is the `M` that `A` was compiled against. `P`'s code sees the 8-byte `S`, while `A`'s object code was compiled with the 40-byte one, which is the `S` that `MakeS()` returns. A program can contain one module named `M`. Each consumer getting its own `M` holds for each compile, but not inside `P`, which contains both. The two paths have to agree. When the lists differ only in flags that don't change how `M` compiles (a flag only `P` reads, `--debug`), the two builds are interchangeable and the cost is building `M` twice. That is the case #40's README suggests a filtered list for: "a list built from the project's args, say one without a flag only this project reads (it would otherwise give the sibling a build directory of its own)". Filtering on one path is what gives the sibling its second build directory. No current Catcrafts graph hits this: Network, Graphics-imsd-messages, HelloDom and SimpleClient each send one changed list to all of their siblings. ## Suggested fix - Fail before compiling when a configuration's transitive dependencies contain the same project as two different `Configuration`s, and name both paths with their lists: ``` M is loaded twice in P's build, with different args: P → M [] P → A → M [--wide] A program can contain one build of M: send it the same list on both paths. ``` "Same project" can be the project file plus the configuration name. Matching on the module names the configurations export would also catch a `LocalProject` checkout and a `GitProject` clone of one project meeting in one program, for example when only one path honours `--local`. - Check per program, not per run: two builds of one project are fine when they go into different programs (two executables, a test, a `#pragma crafter dependency` tool). - README: replace the filtered-list advice with "build a changed list once and send it to every sibling", and say what happens otherwise. - A test built on the fixture above: `Build()` fails with that message when the lists differ, and builds with one `M` when they agree. Not suggested: picking one of the two `M`s and rebuilding the other consumer against it. Arg lists have no merge rule, so the consumer that loses silently gets a different `M` from the one it asked for.

jorijnvdgraaf referenced this issue from a commit

2026-10-31 04:11:50 +00:00

docs: correct what the sibling-args docs promise

jorijnvdgraaf referenced this issue

2026-10-31 04:12:34 +00:00

feat(external): forward a project's args to its sibling projects by default; lint with the lite rules #40

jorijnvdgraaf referenced this issue from a commit

2026-10-31 04:19:54 +00:00

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

jorijnvdgraaf referenced this issue from a pull request that will close it,

2026-10-31 04:19:54 +00:00

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