Merge branch 'claude/issue-41': refuse a build that holds two builds of one module
All checks were successful
Flatpak / flatpak (push) Successful in 12m49s
All checks were successful
Flatpak / flatpak (push) Successful in 12m49s
PR #42, fixes #41. Build() now refuses a configuration whose transitive dependencies hold two configurations exporting a module of the same name, such as one project loaded along two paths with different arg lists, before it builds anything, and names both paths. Until now both builds ended up in the one program, which compiled cleanly and then read garbage. Merged cleanly on top of claude/forward-project-args; the suite, with claude/dependency-notes' fixtures, passes under the new check. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
commit
eefd1df3eb
11 changed files with 233 additions and 4 deletions
6
tests/DuplicateModule/fixture/a/interfaces/A.cppm
Normal file
6
tests/DuplicateModule/fixture/a/interfaces/A.cppm
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
export module A;
|
||||
import std;
|
||||
import M;
|
||||
export std::int32_t ASize() {
|
||||
return Size();
|
||||
}
|
||||
24
tests/DuplicateModule/fixture/a/project.cpp
Normal file
24
tests/DuplicateModule/fixture/a/project.cpp
Normal file
|
|
@ -0,0 +1,24 @@
|
|||
// SPDX-License-Identifier: LGPL-3.0-only
|
||||
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||
|
||||
import std;
|
||||
import Crafter.Build;
|
||||
namespace fs = std::filesystem;
|
||||
using namespace Crafter;
|
||||
|
||||
// Loads M with the default, so M gets 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;
|
||||
}
|
||||
12
tests/DuplicateModule/fixture/m/interfaces/M.cppm
Normal file
12
tests/DuplicateModule/fixture/m/interfaces/M.cppm
Normal file
|
|
@ -0,0 +1,12 @@
|
|||
export module M;
|
||||
import std;
|
||||
// --wide gives S more members, so the two builds of M the test asks for
|
||||
// differ in what they declare.
|
||||
#ifdef WIDE
|
||||
export struct S { std::int32_t a; std::int32_t b; std::int64_t pad[4]; };
|
||||
#else
|
||||
export struct S { std::int32_t a; std::int32_t b; };
|
||||
#endif
|
||||
export std::int32_t Size() {
|
||||
return static_cast<std::int32_t>(sizeof(S));
|
||||
}
|
||||
22
tests/DuplicateModule/fixture/m/project.cpp
Normal file
22
tests/DuplicateModule/fixture/m/project.cpp
Normal file
|
|
@ -0,0 +1,22 @@
|
|||
// SPDX-License-Identifier: LGPL-3.0-only
|
||||
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||
|
||||
import std;
|
||||
import Crafter.Build;
|
||||
namespace fs = std::filesystem;
|
||||
using namespace Crafter;
|
||||
|
||||
// --wide is this project's own flag, and 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;
|
||||
}
|
||||
6
tests/DuplicateModule/fixture/p/main.cpp
Normal file
6
tests/DuplicateModule/fixture/p/main.cpp
Normal file
|
|
@ -0,0 +1,6 @@
|
|||
import std;
|
||||
import M;
|
||||
import A;
|
||||
int main() {
|
||||
return Size() == ASize() ? 0 : 1;
|
||||
}
|
||||
32
tests/DuplicateModule/fixture/p/project.cpp
Normal file
32
tests/DuplicateModule/fixture/p/project.cpp
Normal file
|
|
@ -0,0 +1,32 @@
|
|||
// SPDX-License-Identifier: LGPL-3.0-only
|
||||
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||
|
||||
import std;
|
||||
import Crafter.Build;
|
||||
namespace fs = std::filesystem;
|
||||
using namespace Crafter;
|
||||
|
||||
// Loads M itself and through A, which passes on all of its args. With
|
||||
// --split, the list P gives M leaves --wide out, as a filter meant for one
|
||||
// sibling would, and the two paths load M with two different lists.
|
||||
extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> args) {
|
||||
const bool split = std::ranges::contains(args, std::string_view("--split"));
|
||||
std::vector<std::string> mArgs;
|
||||
for (std::string_view arg : args) {
|
||||
if (!split || arg != "--wide") mArgs.emplace_back(arg);
|
||||
}
|
||||
Configuration* m = LocalProject({ .projectFile = "../m/project.cpp", .args = mArgs });
|
||||
Configuration* a = LocalProject({ .projectFile = "../a/project.cpp" });
|
||||
|
||||
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;
|
||||
}
|
||||
84
tests/DuplicateModule/main.cpp
Normal file
84
tests/DuplicateModule/main.cpp
Normal file
|
|
@ -0,0 +1,84 @@
|
|||
// SPDX-License-Identifier: LGPL-3.0-only
|
||||
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||
|
||||
import std;
|
||||
import Crafter.Build;
|
||||
namespace fs = std::filesystem;
|
||||
using namespace Crafter;
|
||||
|
||||
// A program can hold one module of each name, so Build() refuses a graph in
|
||||
// which two configurations export the same module: the same project loaded
|
||||
// along two paths with two arg lists, say. It used to build both and put
|
||||
// both on the consumer's module path. Clang took the first for every import
|
||||
// of the module, the ones inside the other build's consumers included, and
|
||||
// the program linked code compiled against two layouts of one struct: it
|
||||
// built cleanly, then read garbage or crashed (issue #41). The fixture is
|
||||
// P -> M and P -> A -> M. With --split, P gives M its args without --wide,
|
||||
// which changes what M declares, while A passes them on whole.
|
||||
namespace {
|
||||
std::int32_t Failures = 0;
|
||||
|
||||
void Check(bool cond, std::string_view msg) {
|
||||
if (!cond) {
|
||||
std::println(std::cerr, "FAIL: {}", msg);
|
||||
++Failures;
|
||||
}
|
||||
}
|
||||
|
||||
BuildResult BuildPass(Configuration& cfg) {
|
||||
std::unordered_map<fs::path, std::shared_future<BuildResult>> depResults;
|
||||
std::mutex depMutex;
|
||||
return Build(cfg, depResults, depMutex);
|
||||
}
|
||||
}
|
||||
|
||||
int main() {
|
||||
const fs::path installedHome = GetCrafterBuildHome();
|
||||
if (!fs::exists(installedHome / "Crafter.Build.cppm")) {
|
||||
std::println(std::cerr, "SKIP: no module sources at {}", installedHome.string());
|
||||
return 77;
|
||||
}
|
||||
|
||||
// Staged because LoadProject and Build drop build/ and bin/ trees next
|
||||
// to the project files.
|
||||
const fs::path fixture = fs::current_path() / "tests" / "DuplicateModule" / "fixture";
|
||||
const fs::path scratch = fs::temp_directory_path() / "crafter-build-duplicate-module";
|
||||
std::error_code ec;
|
||||
fs::remove_all(scratch, ec);
|
||||
fs::create_directories(scratch);
|
||||
fs::copy(fixture, scratch, fs::copy_options::recursive);
|
||||
|
||||
// From P's own directory, as crafter-build runs it: the fixture's
|
||||
// LocalProject paths are relative to the working directory.
|
||||
const fs::path cwd = fs::current_path();
|
||||
fs::current_path(scratch / "p");
|
||||
try {
|
||||
// Two lists for M: refused before anything is built.
|
||||
{
|
||||
std::array<std::string_view, 2> args = { "--wide", "--split" };
|
||||
Configuration cfg = LoadProject("project.cpp", args);
|
||||
const BuildResult built = BuildPass(cfg);
|
||||
Check(!built.result.empty(), "a graph with two builds of one module fails to build");
|
||||
Check(built.result.contains("module M is built twice for P"), std::format("the error names the module and the program (got: {})", built.result));
|
||||
Check(built.result.contains("P -> M (") && built.result.contains("P -> A -> M ("), std::format("the error names the path to each build (got: {})", built.result));
|
||||
Check(!fs::exists(scratch / "m" / "bin") && !fs::exists(scratch / "a" / "bin") && !fs::exists(scratch / "p" / "bin"), "nothing was built");
|
||||
}
|
||||
|
||||
// One list along both paths: one M, and the build goes through.
|
||||
{
|
||||
std::array<std::string_view, 1> args = { "--wide" };
|
||||
Configuration cfg = LoadProject("project.cpp", args);
|
||||
const bool oneM = cfg.dependencies.size() == 2 && cfg.dependencies[1]->dependencies.size() == 1 && cfg.dependencies[0] == cfg.dependencies[1]->dependencies[0];
|
||||
Check(oneM, "the same list along both paths loads one M");
|
||||
const BuildResult built = BuildPass(cfg);
|
||||
Check(built.result.empty(), std::format("the graph with one M builds (got: {})", built.result));
|
||||
}
|
||||
} catch (const std::exception& e) {
|
||||
fs::current_path(cwd);
|
||||
std::println(std::cerr, "FAIL: threw: {}", e.what());
|
||||
return 1;
|
||||
}
|
||||
fs::current_path(cwd);
|
||||
|
||||
return Failures > 0 ? 1 : 0;
|
||||
}
|
||||
Loading…
Reference in a new issue