From d5b80fcf7e90651474b206db571c5192c89582e9 Mon Sep 17 00:00:00 2001
From: Jorijn van der Graaf <jorijnvdgraaf@catcrafts.net>
Date: Sat, 31 Oct 2026 05:19:30 +0100
Subject: [PATCH] fix: refuse a build that holds two builds of one module

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>
---
 implementations/Crafter.Build-Clang.cpp | 41 +++++++++++++++++++++++++
 1 file changed, 41 insertions(+)

diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp
index f93451a..2d1b47c 100644
--- a/implementations/Crafter.Build-Clang.cpp
+++ b/implementations/Crafter.Build-Clang.cpp
@@ -443,6 +443,42 @@ namespace {
         }
         return outcome;
     }
+
+    // Two configurations among `config` and its transitive dependencies that
+    // export a module of the same name: the same project loaded along two
+    // paths with different args, say, or from a checkout on one path and a
+    // clone on the other. A program can hold one module of each name. Built
+    // anyway, both PCM dirs went onto the consumer's -fprebuilt-module-path,
+    // clang resolved the module by name to the first for every import, the
+    // ones inside the other build's consumers included, and nothing checked
+    // that it was the build they had been compiled against. The program then
+    // linked code compiled against two layouts of the same types, and ran
+    // (issue #41). Empty when the dependencies agree, otherwise the error,
+    // naming the path to each of the two builds.
+    std::string DuplicateModuleError(const Configuration& config) {
+        struct Owner {
+            const Configuration* cfg;
+            std::string chain;
+        };
+        const auto describe = [](const Owner& owner) {
+            return std::format("{} ({}, {}, project args {})", owner.chain, owner.cfg->path.string(), owner.cfg->VariantId(), owner.cfg->projectArgs);
+        };
+        std::unordered_map<std::string, Owner> owners;
+        std::unordered_set<const Configuration*> visited;
+        std::string error;
+        std::function<void(const Configuration&, const std::string&)> walk = [&](const Configuration& c, const std::string& chain) {
+            if (!error.empty() || !visited.insert(&c).second) return;
+            for (const std::unique_ptr<Module>& iface : c.interfaces) {
+                const auto [it, inserted] = owners.emplace(iface->name, Owner{ &c, chain });
+                if (inserted || it->second.cfg == &c) continue;
+                error = std::format("module {} is built twice for {}:\n  {}\n  {}\nA program can hold one build of each module: load {} with the same args, and from the same place, along every path.", iface->name, config.name, describe(it->second), describe(Owner{ &c, chain }), c.name);
+                return;
+            }
+            for (const Configuration* dep : c.dependencies) walk(*dep, std::format("{} -> {}", chain, dep->name));
+        };
+        walk(config, config.name);
+        return error;
+    }
 }
 
 BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, std::shared_future<BuildResult>>& depResults, std::mutex& depMutex) {
@@ -451,6 +487,11 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
     if (const Toolchain& tc = GetToolchain(); !tc.error.empty()) {
         return {tc.error, false, {}};
     }
+    // Before any directory is made or dependency built, so a refused graph
+    // leaves nothing behind.
+    if (std::string duplicate = DuplicateModuleError(config); !duplicate.empty()) {
+        return {std::move(duplicate), false, {}};
+    }
     // Reset per-build cached state on every Module/ModulePartition so that
     // successive Build() calls on the same Configuration re-evaluate mtimes
     // (incremental-rebuild test scenarios).