Crafter.Build/tests/TransitiveInterfaceChange/main.cpp
catbot f38521298b
Some checks failed
CI / build-test-release (pull_request) Failing after 6m5s
fix: track what a primary module interface imports
A primary module interface unit — `export module Widget;`, no partitions —
recorded nothing about what it imported. GetInterfacesAndImplementations
registered the Module and then erased the file from the scan list, so the
import pass only ever saw partitions, and Module had no vectors to hold an
edge anyway.

Two consequences, both reported as issue #26:

  Module::Check consulted only its own .cppm and its partitions. A data
  member added to an imported module left Widget.pcm, Widget.o and every
  consumer object untouched while the imported library rebuilt and both
  binaries relinked — one executable holding two class layouts, no
  diagnostic, and a crash somewhere unrelated. Wiping build/ was the only
  cure, so `crafter-build test` could not be trusted straight after an
  interface edit.

  Module::Compile waited on nothing. Two modules in one Configuration
  compile on concurrent threads, so a primary interface importing a
  sibling was a coin flip between working and "module 'Base' not found".

Partitions never had either problem — they carry the same three vectors and
Check/Compile honour them — which is why the gap only surfaced on a module
whose interface is one flat unit.

Module now carries moduleDependencies, externalModuleDependencies and
pendingImports with the same meanings as on ModulePartition; primary units
stay in the scan list so their imports land there; Check sees through them;
Compile orders itself behind a local sibling; and ResolvePendingImports
sweeps them so an edge survives dependencies being wired up afterwards.
Build() now Checks every interface before spawning any compile thread — the
`compiled` flag a waiter blocks on is raised either by a Compile that runs
or by the Check that decides none is needed, so a Check still pending while
another module's thread waits would have hung the build.

Resolves #26

Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
2026-07-30 18:19:04 +00:00

237 lines
11 KiB
C++

// 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;
// Issue #26. A *primary* module interface unit — `export module Widget;`, no
// partitions — records nothing about what it imports. The scanner registered
// the Module and then dropped the file before the import pass ran, and Module
// itself had nowhere to put the edge, so:
//
// * `Module::Check` looked only at its own .cppm and its partitions. A layout
// change in an imported module left Widget.pcm and Widget.o untouched, the
// consumer's object untouched, and the executable relinked — mixing two
// layouts with no diagnostic. That is the reported failure: rebuild the
// dependency, relink the test, crash somewhere unrelated.
//
// * `Module::Compile` waited on nothing. Two modules in one Configuration
// compile on concurrent threads, so importing a sibling was a coin flip
// between working and "module 'Base' not found".
//
// Partitions never had either problem — they carry the same three vectors and
// Check/Compile honour them — which is why the gap only shows up on a module
// whose interface is one flat unit. tests/IncrementalInterfaceChange covers the
// neighbouring consumer-side edge (issue #27).
namespace {
std::int32_t Failures = 0;
void Check(bool cond, std::string_view msg) {
if (!cond) {
std::println(std::cerr, "FAIL: {}", msg);
++Failures;
}
}
// The fixture is mutated during the run, so work on a copy outside the repo.
fs::path StageFixture() {
fs::path source = fs::current_path() / "tests" / "TransitiveInterfaceChange" / "fixture";
fs::path staged = fs::temp_directory_path() / "crafter-build-transitive-interface-change";
fs::remove_all(staged);
fs::copy(source, staged, fs::copy_options::recursive);
return staged;
}
// Swap in the Base variant carrying the extra member. From a file rather
// than a string literal: the module scanner reads raw source, so a module
// declaration spelled inside this test would make the test itself look like
// an implementation unit of Base.
void GrowBase(const fs::path& staged) {
fs::copy_file(staged / "base" / "Base-grown.cppm.in", staged / "base" / "Base.cppm", fs::copy_options::overwrite_existing);
// copy_file carries the source's mtime across, which would leave the
// rewritten interface looking older than the BMI built from it.
fs::last_write_time(staged / "base" / "Base.cppm", fs::file_time_type::clock::now());
}
std::unique_ptr<Configuration> MakeBaseLib(const fs::path& staged) {
auto base = std::make_unique<Configuration>();
base->path = staged / "base";
base->name = "base";
base->outputName = "base";
base->target = HostTarget();
base->type = ConfigurationType::LibraryStatic;
std::array<fs::path, 1> ifaces = { "Base" };
std::array<fs::path, 1> impls = { "Base" };
base->GetInterfacesAndImplementations(ifaces, impls);
return base;
}
std::unique_ptr<Configuration> MakeWidgetLib(const fs::path& staged) {
auto widget = std::make_unique<Configuration>();
widget->path = staged / "widget";
widget->name = "widget";
widget->outputName = "widget";
widget->target = HostTarget();
widget->type = ConfigurationType::LibraryStatic;
std::array<fs::path, 1> ifaces = { "Widget" };
std::array<fs::path, 1> impls = { "Widget" };
widget->GetInterfacesAndImplementations(ifaces, impls);
return widget;
}
Module* FindModule(const Configuration& cfg, std::string_view name) {
for (const std::unique_ptr<Module>& mod : cfg.interfaces) {
if (mod->name == name) return mod.get();
}
return nullptr;
}
bool BuildOk(Configuration& cfg, std::string_view label) {
// A fresh depResults per pass: the map memoizes each Configuration's
// build for the duration of one pass, so reusing it would skip the
// dependencies' second build entirely.
std::unordered_map<fs::path, std::shared_future<BuildResult>> depResults;
std::mutex depMutex;
BuildResult r = Build(cfg, depResults, depMutex);
if (!r.result.empty()) {
std::println(std::cerr, "FAIL: {} build failed: {}", label, r.result);
++Failures;
return false;
}
return true;
}
// Non-zero exit here means the two ends of the link disagree about the
// layout — see the fixture's main.cpp.
void CheckAgrees(const fs::path& binary, std::string_view label) {
auto run = RunCommandWithTimeout(binary.string(), std::chrono::seconds(30));
Check(run.exitCode == 0 && !run.crashed && !run.timedOut, std::format("{}: consumer and library agree on the layout (exit={} output='{}')", label, run.exitCode, run.output));
}
}
int main() {
{
// The scan records a primary interface unit's imports, and the
// pending/resolve machinery reaches them the same way it reaches a
// partition's.
fs::path staged = StageFixture();
std::unique_ptr<Configuration> base = MakeBaseLib(staged);
std::unique_ptr<Configuration> widget = MakeWidgetLib(staged);
Module* mod = FindModule(*widget, "Widget");
Check(mod != nullptr, "the primary interface unit registered a module named Widget");
if (mod != nullptr) {
Check(mod->partitions.empty(), "Widget has no partitions, so nothing else could carry the edge");
Check(mod->externalModuleDependencies.empty(), "nothing resolvable before dependencies are assigned");
Check(std::ranges::find(mod->pendingImports, "Base") != mod->pendingImports.end(), "the unresolved import of Base is remembered as pending");
Check(std::ranges::find(mod->pendingImports, "std") != mod->pendingImports.end(), "std stays pending — it is supplied from outside the graph");
widget->dependencies = { base.get() };
widget->ResolvePendingImports();
Check(mod->externalModuleDependencies.size() == 1, "ResolvePendingImports places the external module edge");
if (mod->externalModuleDependencies.size() == 1) {
Check(mod->externalModuleDependencies[0].first->name == "Base", "the edge names the Base module");
Check(mod->externalModuleDependencies[0].second == base->PcmDir() / "Base.pcm", "the edge points at the dependency's BMI");
}
Check(std::ranges::find(mod->pendingImports, "Base") == mod->pendingImports.end(), "the resolved name is no longer pending");
Check(std::ranges::find(mod->pendingImports, "std") != mod->pendingImports.end(), "std is still pending after the sweep");
// Build() runs a sweep unconditionally on top of whatever callers
// already did, so a repeat must not duplicate the edge.
widget->ResolvePendingImports();
Check(mod->externalModuleDependencies.size() == 1, "ResolvePendingImports is idempotent");
}
}
{
// Two modules in one Configuration, declared in the order that used to
// hide the problem: Widget — which imports Base — is registered first.
// Every primary unit is registered before any import is resolved, so
// declaration order must not matter.
fs::path staged = StageFixture();
Configuration cfg;
cfg.path = staged;
cfg.name = "sibling";
cfg.outputName = "sibling";
cfg.target = HostTarget();
cfg.type = ConfigurationType::Executable;
std::array<fs::path, 2> ifaces = { "widget/Widget", "base/Base" };
std::array<fs::path, 3> impls = { "widget/Widget", "base/Base", "main" };
cfg.GetInterfacesAndImplementations(ifaces, impls);
Module* widgetMod = FindModule(cfg, "Widget");
Module* baseMod = FindModule(cfg, "Base");
Check(widgetMod != nullptr && baseMod != nullptr, "both sibling modules registered");
if (widgetMod != nullptr && baseMod != nullptr) {
Check(std::ranges::find(widgetMod->moduleDependencies, baseMod) != widgetMod->moduleDependencies.end(), "Widget's import of its sibling resolved locally despite being declared first");
Check(widgetMod->externalModuleDependencies.empty(), "a sibling in the same Configuration is not an external edge");
Check(baseMod->moduleDependencies.empty(), "Base imports no sibling");
}
// That edge is also the compile-ordering guarantee: Widget's precompile
// needs Base.pcm on disk. Repeated from scratch because the failure was
// a thread race, and one lucky pass proves nothing.
for (std::int32_t attempt = 0; attempt < 4; ++attempt) {
fs::remove_all(cfg.BuildDir());
fs::remove_all(cfg.BinDir());
if (!BuildOk(cfg, std::format("sibling pass {}", attempt))) break;
CheckAgrees(cfg.BinDir() / "sibling", std::format("sibling pass {}", attempt));
}
}
{
// End to end, the reported shape: a consumer executable, a library whose
// primary interface unit imports a second library's module, and a member
// added to a class in that second module.
fs::path staged = StageFixture();
std::unique_ptr<Configuration> base = MakeBaseLib(staged);
std::unique_ptr<Configuration> widget = MakeWidgetLib(staged);
widget->dependencies = { base.get() };
Configuration app;
app.path = staged;
app.name = "consumer";
app.outputName = "consumer";
app.target = HostTarget();
app.type = ConfigurationType::Executable;
std::array<fs::path, 0> appIfaces = {};
std::array<fs::path, 1> appImpls = { "main" };
app.GetInterfacesAndImplementations(appIfaces, appImpls);
app.dependencies = { widget.get() };
fs::path binary = app.BinDir() / "consumer";
fs::path widgetInterfaceObject = widget->BuildDir() / "Widget.o";
fs::path widgetImplObject = widget->BuildDir() / "Widget_impl.o";
fs::path consumerObject = app.BuildDir() / "main_impl.o";
if (BuildOk(app, "first pass")) {
CheckAgrees(binary, "first pass");
fs::file_time_type interfaceBefore = fs::last_write_time(widgetInterfaceObject);
fs::file_time_type implBefore = fs::last_write_time(widgetImplObject);
fs::file_time_type consumerBefore = fs::last_write_time(consumerObject);
GrowBase(staged);
if (BuildOk(app, "second pass")) {
// All three sat on the old layout before the fix, and all three
// were left alone while base rebuilt and both binaries relinked.
Check(fs::last_write_time(widgetInterfaceObject) > interfaceBefore, "the importing interface unit is recompiled");
Check(fs::last_write_time(widgetImplObject) > implBefore, "that module's implementation unit is recompiled");
Check(fs::last_write_time(consumerObject) > consumerBefore, "the consumer's translation unit is recompiled");
CheckAgrees(binary, "second pass");
}
}
}
if (Failures > 0) {
std::println(std::cerr, "{} assertions failed", Failures);
return 1;
}
return 0;
}