diff --git a/README.md b/README.md index 09bea8b..4b8b6de 100644 --- a/README.md +++ b/README.md @@ -125,6 +125,12 @@ Per-import precise tracking for both within-project and cross-project module dep Diamond deps (`A → {B, C}; B → X; C → X`) build `X` exactly once via a `std::shared_future` cache. +Tracking is derived from each translation unit's `import` statements, which are scanned when the sources are declared. `cfg.dependencies` is often assigned *afterwards* — `AddTest` works that way — so `Build()` re-resolves any import that matched nothing at scan time before it compares mtimes. Without that, a consumer of a dependency's module carried no edge to it at all: adding a data member to that dependency's interface rebuilt the library, relinked the consumer, and left the consumer's object compiled against the old class layout. Nothing fails to link when a member is added, so the result was a working build and a crash later. + +Everything that changes what gets built belongs in the variant id, since it names the `bin/` and `build/` directory. That includes project args crafter-build itself doesn't interpret: `crafter-build` and `crafter-build -- --no-webgpu` get separate directories rather than interleaving their outputs in one. The cached host PCMs under `/crafter.build/-/` are shared by every crafter-build on the machine, so they are invalidated by a hash of the module sources rather than by mtime — an mtime can't tell a newer PCM from one built by a different install. + +`crafter-build clean` removes the project's `bin/` and `build/` trees. It doesn't load `project.cpp`, so it still works when the project no longer compiles. + ## Tests Tests live under `tests//`. The simplest case is a single C++ file: diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp index 95a691d..6dc681c 100644 --- a/implementations/Crafter.Build-Clang.cpp +++ b/implementations/Crafter.Build-Clang.cpp @@ -25,9 +25,14 @@ namespace fs = std::filesystem; using namespace Crafter; -void Configuration::GetInterfacesAndImplementations(std::span interfaces, std::span implementations) { - auto resolveImport = [this](const std::string& importName, std::vector& localDeps, std::vector>& externalDeps) -> bool { - for(const std::unique_ptr& interface : this->interfaces) { +namespace { + // Map one `import X;` name onto either a module this Configuration owns or + // one exported by a Configuration reachable through its dependency DAG, + // appending the matching staleness edge. False means nothing in reach + // provides X — either it's supplied from outside the graph (`std`) or the + // dependency isn't wired up *yet*, which is why callers remember the name. + bool ResolveImportName(Configuration& cfg, const std::string& importName, std::vector& localDeps, std::vector>& externalDeps) { + for(const std::unique_ptr& interface : cfg.interfaces) { if(interface->name == importName) { localDeps.push_back(interface.get()); return true; @@ -50,10 +55,38 @@ void Configuration::GetInterfacesAndImplementations(std::span interfac return false; }; - for(Configuration* depCfg : this->dependencies) { + for(Configuration* depCfg : cfg.dependencies) { if (walk(depCfg)) return true; } return false; + } +} + +void Configuration::ResolvePendingImports() { + // Same resolution the scan used, and against both dependency kinds — a + // second GetInterfacesAndImplementations call can add interfaces that an + // earlier batch's import was looking for, so a pending name may land on a + // local module and not just an external one. + auto sweep = [this](std::vector& pending, std::vector& localDeps, std::vector>& externalDeps) { + std::erase_if(pending, [&](const std::string& name) { + return ResolveImportName(*this, name, localDeps, externalDeps); + }); + }; + for(const std::unique_ptr& interface : interfaces) { + for(const std::unique_ptr& partition : interface->partitions) { + sweep(partition->pendingImports, partition->moduleDependencies, partition->externalModuleDependencies); + } + } + for(Implementation& implementation : implementations) { + sweep(implementation.pendingImports, implementation.moduleDependencies, implementation.externalModuleDependencies); + } +} + +void Configuration::GetInterfacesAndImplementations(std::span interfaces, std::span implementations) { + auto resolveImport = [this](const std::string& importName, std::vector& localDeps, std::vector>& externalDeps, std::vector& pending) { + if (!ResolveImportName(*this, importName, localDeps, externalDeps)) { + pending.push_back(importName); + } }; std::vector> tempModulePaths = std::vector>(interfaces.size()); @@ -128,7 +161,7 @@ void Configuration::GetInterfacesAndImplementations(std::span interfac std::sregex_iterator modCurrent(fileContent.begin(), fileContent.end(), modulePattern); while (modCurrent != lastMatch) { std::smatch match = *modCurrent; - resolveImport(match[1].str(), partition->moduleDependencies, partition->externalModuleDependencies); + resolveImport(match[1].str(), partition->moduleDependencies, partition->externalModuleDependencies, partition->pendingImports); ++modCurrent; } } @@ -173,7 +206,7 @@ void Configuration::GetInterfacesAndImplementations(std::span interfac while (modCurrent != lastMatch) { std::smatch match2 = *modCurrent; if (match2[1] != match[1]) { - resolveImport(match2[1].str(), implementation.moduleDependencies, implementation.externalModuleDependencies); + resolveImport(match2[1].str(), implementation.moduleDependencies, implementation.externalModuleDependencies, implementation.pendingImports); } ++modCurrent; } @@ -188,7 +221,7 @@ void Configuration::GetInterfacesAndImplementations(std::span interfac std::sregex_iterator lastMatch; while (currentMatch != lastMatch) { std::smatch match2 = *currentMatch; - resolveImport(match2[1].str(), implementation.moduleDependencies, implementation.externalModuleDependencies); + resolveImport(match2[1].str(), implementation.moduleDependencies, implementation.externalModuleDependencies, implementation.pendingImports); ++currentMatch; } } @@ -224,6 +257,18 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map/main.cpp and + // only then hands back a builder whose .Dependencies() supplies the library. + // Any `import ;` in such a TU resolved to nothing and so carried + // no staleness edge, which meant a member added to a dependency's interface + // rebuilt the library, relinked the consumer, and silently kept the + // consumer's object compiled against the *old* class layout (issue #27). + // Re-resolving here — the last point before mtimes are compared, with the + // DAG fully wired — closes that window for every caller rather than for the + // ones that remember to declare in the right order. + config.ResolvePendingImports(); + // Auto-detect the WASI sysroot before any compile step runs so BuildStdPcm // and the main compile command see the same value. Linux-only — Windows // users supply cfg.sysroot pointing at their wasi-sdk install. Covers all @@ -1401,7 +1446,15 @@ ArgQuery Crafter::ApplyStandardArgs(Configuration& cfg, std::span(-1) || ec) { + throw std::runtime_error(std::format("could not remove {}: {}", dir.string(), ec.message())); + } + removed.push_back(std::move(dir)); + } + return removed; +} + static void PrintHelp(std::string_view argv0) { std::println( R"(Usage: @@ -1423,6 +1491,7 @@ R"(Usage: {0} test [test-options] [globs...] Build and run the project's tests {0} lint [lint-options] [globs...] Run the project's lint rules over its sources {0} format [format-options] [globs...] Apply the project's transform rules (rewrites files) + {0} clean Delete the project's bin/ and build/ trees {0} help | -h | --help Show this help Loads ./project.cpp (override with --project=), compiles it to a shared @@ -1465,10 +1534,17 @@ Format options (after the `format` subcommand): transform. `format` rewrites changed files in place; `lint` reports the same transforms as would-reformat findings without writing. +Clean (after the `clean` subcommand): + Removes bin/ and build/ next to the project file. Does not load project.cpp, + so it works when the project itself no longer compiles. Every target, variant + and dependency artifact under those trees goes with it. + Project args: Any flag not consumed above is forwarded verbatim to CrafterBuildProject as part of its `args` span. Project-specific flags (e.g. --target=, custom - feature toggles) live there. + feature toggles) live there. Flags ApplyStandardArgs does not itself + interpret are folded into the variant hash, so switching one lands in its own + bin/ and build/ directory instead of overwriting the other setting's. Environment: CRAFTER_BUILD_MARCH Override -march (default: native). @@ -1496,6 +1572,7 @@ int Crafter::Run(int argc, char** argv) { bool runTests = false; bool runLint = false; bool runFormat = false; + bool runClean = false; bool runAfterBuild = false; RunTestsOptions testOpts; RunLintOptions lintOpts; @@ -1506,11 +1583,13 @@ int Crafter::Run(int argc, char** argv) { if (arg == "-h" || arg == "--help" || (!runTests && !runLint && !runFormat && arg == "help")) { PrintHelp(argv0); return 0; - } else if (!runLint && !runFormat && arg == "test") { + } else if (!runLint && !runFormat && !runClean && arg == "test") { runTests = true; - } else if (!runTests && !runFormat && arg == "lint") { + } else if (!runTests && !runFormat && !runClean && arg == "lint") { runLint = true; - } else if (!runTests && !runLint && arg == "format") { + } else if (!runTests && !runLint && !runClean && arg == "clean") { + runClean = true; + } else if (!runTests && !runLint && !runClean && arg == "format") { runFormat = true; lintOpts.mode = LintMode::Apply; } else if (runFormat && arg == "--check") { @@ -1558,6 +1637,17 @@ int Crafter::Run(int argc, char** argv) { return 1; } + // Ahead of LoadProject on purpose — see CleanProject. + if (runClean) { + std::vector removed = CleanProject(projectFile); + if (removed.empty()) { + std::println("Nothing to clean"); + } else { + for (const fs::path& dir : removed) std::println("Removed {}", dir.string()); + } + return 0; + } + Configuration config = LoadProject(projectFile, projectArgs); SetParentProject(&config); diff --git a/implementations/Crafter.Build-Platform.cpp b/implementations/Crafter.Build-Platform.cpp index c0b35a8..55e035b 100644 --- a/implementations/Crafter.Build-Platform.cpp +++ b/implementations/Crafter.Build-Platform.cpp @@ -74,6 +74,54 @@ namespace { CacheLock(const CacheLock&) = delete; CacheLock& operator=(const CacheLock&) = delete; }; + + // The cached Crafter.Build PCMs are keyed by `-` alone, so + // every crafter-build on the machine writes to the same files regardless of + // which share/crafter-build its module sources came from. Freshness used to + // be a per-file mtime comparison, which cannot distinguish "this PCM is + // newer than my source" from "this PCM was built from *different* sources + // that happen to be newer" — so a second install or checkout silently + // compiled its project against the other's declarations. Same failure shape + // as issue #27: no error, a binary built against a layout nobody linked. + // + // A stamp over the bytes of every module source answers what the mtime + // can't. It covers the whole set rather than one file at a time because the + // PCMs import each other: a change to :Interface invalidates :Clang's PCM + // even though Crafter.Build-Clang.cppm is untouched. + std::string CrafterBuildSourceStamp(const fs::path& sourceDir, std::span moduleNames) { + std::string all; + auto append = [&all, &sourceDir](const fs::path& relative) { + std::ifstream in(sourceDir / relative, std::ios::binary); + std::ostringstream buffer; + buffer << in.rdbuf(); + all += relative.string(); + all += '\0'; + all += buffer.str(); + all += '\0'; + }; + for (std::string_view name : moduleNames) { + append(fs::path(std::format("{}.cppm", name))); + } + // Exported through `module;` preambles, so its contents land in the PCMs + // too. + append("Crafter.Build-Api.h"); + return std::format("{:016x}", std::hash{}(all)); + } + + fs::path CacheStampPath(const fs::path& cacheDir) { + return cacheDir / "crafter-build-sources.stamp"; + } + + std::string ReadCacheStamp(const fs::path& cacheDir) { + std::ifstream in(CacheStampPath(cacheDir), std::ios::binary); + std::string stamp; + std::getline(in, stamp); + return stamp; + } + + void WriteCacheStamp(const fs::path& cacheDir, const std::string& stamp) { + std::ofstream(CacheStampPath(cacheDir), std::ios::binary | std::ios::trunc) << stamp << '\n'; + } } fs::path Crafter::GetCrafterBuildHome() { @@ -328,13 +376,17 @@ std::string Crafter::GetBaseCommand(const Configuration& config) { namespace { void EnsureCrafterBuildPcms(const fs::path& sourceDir, const fs::path& cacheDir) { CacheLock lock(cacheDir); + // Re-derived under the lock: another builder may have refreshed the + // cache from its own sources while we waited. + std::string stamp = CrafterBuildSourceStamp(sourceDir, CrafterBuildModules); + bool upToDate = ReadCacheStamp(cacheDir) == stamp; for (std::string_view name : CrafterBuildModules) { fs::path cppmPath = sourceDir / std::format("{}.cppm", name); fs::path pcmPath = cacheDir / std::format("{}.pcm", name); if (!fs::exists(cppmPath)) { throw std::runtime_error(std::format("module source {} not found in {} (set CRAFTER_BUILD_HOME)", name, sourceDir.string())); } - if (fs::exists(pcmPath) && fs::last_write_time(cppmPath) < fs::last_write_time(pcmPath)) { + if (upToDate && fs::exists(pcmPath)) { continue; } std::string cmd = std::format( @@ -350,6 +402,10 @@ namespace { throw std::runtime_error(std::format("Failed to precompile {} (exit {}): {}", name, r.exitCode, r.output)); } } + // Last, so an interrupted rebuild leaves the stamp disagreeing with the + // sources and the next run starts over rather than trusting a half-built + // set of PCMs. + if (!upToDate) WriteCacheStamp(cacheDir, stamp); } } @@ -523,6 +579,10 @@ std::string Crafter::GetBaseCommand(const Configuration& config) { namespace { void EnsureCrafterBuildPcms(const fs::path& sourceDir, const fs::path& cacheDir) { CacheLock lock(cacheDir); + // Re-derived under the lock: another builder may have refreshed the + // cache from its own sources while we waited. + std::string stamp = CrafterBuildSourceStamp(sourceDir, CrafterBuildModules); + bool upToDate = ReadCacheStamp(cacheDir) == stamp; fs::path prefix = MingwPrefix(); for (std::string_view name : CrafterBuildModules) { fs::path cppmPath = sourceDir / std::format("{}.cppm", name); @@ -530,7 +590,7 @@ namespace { if (!fs::exists(cppmPath)) { throw std::runtime_error(std::format("module source {} not found in {} (set CRAFTER_BUILD_HOME)", name, sourceDir.string())); } - if (fs::exists(pcmPath) && fs::last_write_time(cppmPath) < fs::last_write_time(pcmPath)) { + if (upToDate && fs::exists(pcmPath)) { continue; } std::string cmd = std::format( @@ -546,6 +606,10 @@ namespace { throw std::runtime_error(std::format("Failed to precompile {} (exit {}): {}", name, r.exitCode, r.output)); } } + // Last, so an interrupted rebuild leaves the stamp disagreeing with the + // sources and the next run starts over rather than trusting a half-built + // set of PCMs. + if (!upToDate) WriteCacheStamp(cacheDir, stamp); } } @@ -839,13 +903,17 @@ namespace { void EnsureCrafterBuildPcms(const fs::path& sourceDir, const fs::path& cacheDir) { CacheLock lock(cacheDir); + // Re-derived under the lock: another builder may have refreshed the + // cache from its own sources while we waited. + std::string stamp = CrafterBuildSourceStamp(sourceDir, CrafterBuildModules); + bool upToDate = ReadCacheStamp(cacheDir) == stamp; for (std::string_view name : CrafterBuildModules) { fs::path cppmPath = sourceDir / std::format("{}.cppm", name); fs::path pcmPath = cacheDir / std::format("{}.pcm", name); if (!fs::exists(cppmPath)) { throw std::runtime_error(std::format("module source {} not found in {} (set CRAFTER_BUILD_HOME)", name, sourceDir.string())); } - if (fs::exists(pcmPath) && fs::last_write_time(cppmPath) < fs::last_write_time(pcmPath)) { + if (upToDate && fs::exists(pcmPath)) { continue; } std::string cmd = std::format( @@ -860,6 +928,10 @@ namespace { throw std::runtime_error(std::format("Failed to precompile {} (exit {}): {}", name, r.exitCode, r.output)); } } + // Last, so an interrupted rebuild leaves the stamp disagreeing with the + // sources and the next run starts over rather than trusting a half-built + // set of PCMs. + if (!upToDate) WriteCacheStamp(cacheDir, stamp); } } diff --git a/implementations/Crafter.Build-Test.cpp b/implementations/Crafter.Build-Test.cpp index f686d26..a6e3ec8 100644 --- a/implementations/Crafter.Build-Test.cpp +++ b/implementations/Crafter.Build-Test.cpp @@ -429,6 +429,10 @@ TestBuilder Configuration::AddTest(std::string_view name, std::span in t.config.mtune = this->mtune; t.config.sysroot = this->sysroot; t.config.debug = this->debug; + // Inherited so a project flag that changes what the library contains also + // moves the test's own outputs, rather than letting two flag settings share + // one test bin dir. + t.config.projectArgs = this->projectArgs; t.config.type = ConfigurationType::Executable; // Default source layout: tests//main.cpp resolved against the @@ -453,6 +457,7 @@ void Configuration::AddMarchVariants(std::string_view name, std::span t.config.mtune = tier.mtune; t.config.sysroot = this->sysroot; t.config.debug = this->debug; + t.config.projectArgs = this->projectArgs; t.config.type = ConfigurationType::Executable; fs::path mainSource = fs::path("tests") / std::string(name) / "main"; @@ -483,6 +488,14 @@ TestBuilder& TestBuilder::Args(std::vector a) { Ref().args = std:: TestBuilder& TestBuilder::Requires(std::string r) { Ref().requires_.push_back(std::move(r)); return *this; } TestBuilder& TestBuilder::Dependencies(std::vector d) { Ref().config.dependencies = std::move(d); + // AddTest already scanned tests//main.cpp, at which point this test + // had no dependencies, so every `import ;` in it came back + // unresolved. Place them now that the libraries are known — without this + // the test's object carries no staleness edge to the interfaces it consumes + // and survives a layout change to them (issue #27). Build() re-runs the + // same sweep as a backstop; doing it here keeps the Configuration coherent + // for anyone inspecting it before the build. + Ref().config.ResolvePendingImports(); return *this; } TestBuilder& TestBuilder::LinkFlag(std::string f) { Ref().config.linkFlags.push_back(std::move(f)); return *this; } diff --git a/interfaces/Crafter.Build-Clang.cppm b/interfaces/Crafter.Build-Clang.cppm index 64c22f7..637e174 100644 --- a/interfaces/Crafter.Build-Clang.cppm +++ b/interfaces/Crafter.Build-Clang.cppm @@ -250,10 +250,28 @@ export namespace Crafter { // clobber the baseline's. Not for direct project use — declare // wasmVariants instead. std::vector wasmVariantFlags; + // The project args ApplyStandardArgs did not interpret — i.e. the + // project's own flags, whose effect on the build the framework cannot + // see. Hashed into VariantId because such a flag typically decides what + // gets compiled or bundled (`--no-webgpu` dropping entries from + // cfg.files, say), and without it both flag settings share one bin dir + // and interleave their outputs there. Populated by ApplyStandardArgs; + // the args it does recognise are excluded, since their effect already + // shows up in target/march/mtune/debug/sysroot/type. + std::vector projectArgs; std::vector tests; // Lint rules for `crafter-build lint`. Populate via AddLintRule. std::vector lintRules; CRAFTER_API void GetInterfacesAndImplementations(std::span interfaces, std::span implementations); + // Retry the `import X;` names GetInterfacesAndImplementations could not + // place, against the dependency DAG as it stands now. Sources are + // scanned when they're declared, but `dependencies` is often assigned + // afterwards (AddTest does exactly this), and an import that resolved to + // nothing leaves no staleness edge — so a dependency's interface could + // change and this Configuration's objects would be silently reused + // against the new layout. Build() calls this before checking mtimes; + // calling it again is harmless. + CRAFTER_API void ResolvePendingImports(); // Declare a test. Sources default to `tests//main.cpp` resolved // against this Configuration's path; target/march/mtune/sysroot/debug // are inherited from this Configuration so cross-arch projects don't @@ -306,6 +324,12 @@ export namespace Crafter { compileKey += "|W:"; compileKey += f; } + // Sorted by ApplyStandardArgs, so flag order on the command line + // doesn't split the cache. + for (const std::string& a : projectArgs) { + compileKey += "|A:"; + compileKey += a; + } std::size_t configHash = std::hash{}(compileKey); return std::format("{}-{}-{}-{}-{:08x}", name, target, march, mtune, configHash); } @@ -336,6 +360,16 @@ export namespace Crafter { CRAFTER_API int Run(int argc, char** argv); + // Delete the bin/ and build/ trees beside `projectFile`, returning the paths + // that existed and were removed. Backs `crafter-build clean`. + // + // Deliberately does not load the project: cleaning is most often reached + // when something is already wrong, and a clean that first needs project.cpp + // to compile (and its git dependencies to be present) is useless exactly + // then. That means variant directories are not enumerated — the whole tree + // goes, which is what the by-hand `rm -rf bin build` did anyway. + CRAFTER_API std::vector CleanProject(const fs::path& projectFile); + // Add a small index.html + runtime.js pair next to the .wasm output so the // build can be loaded directly in a browser (just `serve` the bin dir and // open it). Opt-in: WASI builds destined for wasmtime/wasmer don't need diff --git a/interfaces/Crafter.Build-Implementation.cppm b/interfaces/Crafter.Build-Implementation.cppm index fb9842a..95f816b 100644 --- a/interfaces/Crafter.Build-Implementation.cppm +++ b/interfaces/Crafter.Build-Implementation.cppm @@ -15,6 +15,9 @@ namespace Crafter { std::vector moduleDependencies; std::vector partitionDependencies; std::vector> externalModuleDependencies; + // See ModulePartition::pendingImports — imports this TU declared that + // no reachable Configuration provided when the source was scanned. + std::vector pendingImports; fs::path path; CRAFTER_API Implementation(fs::path&& path); CRAFTER_API bool Check(const fs::path& buildDir, const fs::path& pcmDir, fs::file_time_type sourceFloor = fs::file_time_type::min()) const; diff --git a/interfaces/Crafter.Build-Interface.cppm b/interfaces/Crafter.Build-Interface.cppm index e7274cb..b1c2d0c 100644 --- a/interfaces/Crafter.Build-Interface.cppm +++ b/interfaces/Crafter.Build-Interface.cppm @@ -14,6 +14,13 @@ namespace Crafter { std::vector moduleDependencies; std::vector partitionDependencies; std::vector> externalModuleDependencies; + // Names from `import X;` that matched neither a module in this + // Configuration nor one reachable through its dependencies at the time + // the source was scanned. Retried by ResolvePendingImports (see + // Crafter.Build:Clang) so a dependency wired up after + // GetInterfacesAndImplementations still produces a staleness edge. + // Names that never resolve (`std`, module-mapped externals) stay here. + std::vector pendingImports; std::atomic compiled; bool needsRecompiling; bool checked = false; diff --git a/project.cpp b/project.cpp index 251a8af..0def7cd 100644 --- a/project.cpp +++ b/project.cpp @@ -107,6 +107,8 @@ extern "C" Configuration CrafterBuildProject(std::span a cfg.AddTest("StaticLib").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("ModuleInterface").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("DependencyLink").Dependencies({ CrafterBuildLib.get() }); + cfg.AddTest("IncrementalInterfaceChange").Dependencies({ CrafterBuildLib.get() }); + cfg.AddTest("CleanProject").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("ShaderCompile").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("StandardArgs").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("TestRunnerSpec").Dependencies({ CrafterBuildLib.get() }); @@ -120,6 +122,9 @@ extern "C" Configuration CrafterBuildProject(std::span a // for project.so. cfg.AddTest("ConcurrentCacheRace").Dependencies({ CrafterBuildLib.get() }) .LinkFlag("-Wl,--export-dynamic").LinkFlag("-ldl"); + // Same LoadProject wiring as ConcurrentCacheRace above. + cfg.AddTest("HostCacheSourceStamp").Dependencies({ CrafterBuildLib.get() }) + .LinkFlag("-Wl,--export-dynamic").LinkFlag("-ldl"); cfg.AddTest("ConcurrentDependencyReset").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("Lint").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("HouseRules").Dependencies({ CrafterBuildLib.get() }); diff --git a/tests/CleanProject/main.cpp b/tests/CleanProject/main.cpp new file mode 100644 index 0000000..4a9c8f0 --- /dev/null +++ b/tests/CleanProject/main.cpp @@ -0,0 +1,79 @@ +// 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; + +// `crafter-build clean` exists so the fix for a stale build is discoverable +// rather than folklore. It must not need the project to load: the state you most +// want to clear is one where the build is already broken. +namespace { + std::int32_t Failures = 0; + + void Check(bool cond, std::string_view msg) { + if (!cond) { + std::println(std::cerr, "FAIL: {}", msg); + ++Failures; + } + } + + fs::path StageProject(std::string_view leaf) { + fs::path root = fs::temp_directory_path() / std::format("crafter-build-clean-{}", leaf); + fs::remove_all(root); + fs::create_directories(root / "bin" / "app-x86_64-pc-linux-gnu-native-native-0badf00d"); + fs::create_directories(root / "build" / "app-x86_64-pc-linux-gnu-native-native-0badf00d"); + std::ofstream(root / "bin" / "app-x86_64-pc-linux-gnu-native-native-0badf00d" / "app") << "binary"; + std::ofstream(root / "build" / "app-x86_64-pc-linux-gnu-native-native-0badf00d" / "main_impl.o") << "object"; + std::ofstream(root / "keep-me.txt") << "untouched"; + return root; + } +} + +int main() { + { + // A project file that could never compile: clean has to work anyway. + fs::path root = StageProject("broken"); + fs::path projectFile = root / "project.cpp"; + std::ofstream(projectFile) << "this is not valid C++ at all\n"; + + std::vector removed = CleanProject(projectFile); + Check(removed.size() == 2, std::format("both trees reported removed, got {}", removed.size())); + Check(!fs::exists(root / "bin"), "bin/ is gone"); + Check(!fs::exists(root / "build"), "build/ is gone"); + Check(fs::exists(root / "keep-me.txt"), "unrelated files in the project root are left alone"); + Check(fs::exists(projectFile), "the project file itself is left alone"); + } + + { + // Idempotent, and quiet about it — nothing to remove is not an error. + fs::path root = StageProject("twice"); + fs::path projectFile = root / "project.cpp"; + std::ofstream(projectFile) << "\n"; + + CleanProject(projectFile); + std::vector second = CleanProject(projectFile); + Check(second.empty(), "a second clean reports nothing removed"); + } + + { + // Resolved relative to the project file, not the cwd, so --project= + // cleans the project it names. + fs::path root = StageProject("elsewhere"); + fs::path projectFile = root / "project.cpp"; + std::ofstream(projectFile) << "\n"; + fs::path cwdBin = fs::current_path() / "bin"; + bool cwdBinExisted = fs::exists(cwdBin); + + CleanProject(projectFile); + Check(!fs::exists(root / "bin"), "the named project's bin/ is gone"); + Check(fs::exists(cwdBin) == cwdBinExisted, "the cwd's bin/ is untouched"); + } + + if (Failures > 0) { + std::println(std::cerr, "{} assertions failed", Failures); + return 1; + } + return 0; +} diff --git a/tests/HostCacheSourceStamp/main.cpp b/tests/HostCacheSourceStamp/main.cpp new file mode 100644 index 0000000..b6904e8 --- /dev/null +++ b/tests/HostCacheSourceStamp/main.cpp @@ -0,0 +1,150 @@ +// SPDX-License-Identifier: LGPL-3.0-only +// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +#include +import std; +import Crafter.Build; +namespace fs = std::filesystem; +using namespace Crafter; + +// The host PCM cache is keyed by `-` alone, so every +// crafter-build on a machine — a package install and a working checkout, two +// checkouts of different versions — writes the same `Crafter.Build-*.pcm` +// files. Freshness was a per-file mtime comparison, which says "the PCM is +// newer than my source, reuse it" whether or not it was built from *my* +// source. The loser then compiled its project.cpp against the other install's +// declarations: no error, and a project.so built against a layout that doesn't +// match the library it's loaded into — the same silent shape as issue #27. +// +// So the invalidation has to key on the source bytes, and this test asserts it +// notices a change the mtime rule provably cannot see: different content with a +// deliberately *older* timestamp. +namespace { + std::int32_t Failures = 0; + + void Check(bool cond, std::string_view msg) { + if (!cond) { + std::println(std::cerr, "FAIL: {}", msg); + ++Failures; + } + } + + // The cache subdirectory is named for the host target and march; glob for + // it rather than reconstructing the naming rule here. + fs::path FindModuleCacheDir(const fs::path& cacheRoot) { + for (const fs::directory_entry& entry : fs::directory_iterator(cacheRoot / "crafter.build")) { + if (entry.is_directory()) return entry.path(); + } + return {}; + } + + std::string ReadStamp(const fs::path& moduleCacheDir) { + std::ifstream in(moduleCacheDir / "crafter-build-sources.stamp"); + std::string stamp; + std::getline(in, stamp); + return stamp; + } +} + +int main() { + fs::path installedHome = GetCrafterBuildHome(); + if (!fs::exists(installedHome / "Crafter.Build.cppm")) { + std::println(std::cerr, "SKIP: no module sources at {}", installedHome.string()); + return 77; + } + + fs::path scratch = fs::temp_directory_path() / "crafter-build-host-cache-source-stamp"; + std::error_code ec; + fs::remove_all(scratch, ec); + fs::create_directories(scratch); + + // A private copy of the module sources, so the mutation below can't touch + // the real install, and a cold private cache so the first LoadProject has + // to populate it. + fs::path home = scratch / "share" / "crafter-build"; + fs::create_directories(home.parent_path()); + fs::copy(installedHome, home, fs::copy_options::recursive); + setenv("CRAFTER_BUILD_HOME", home.string().c_str(), 1); + + fs::path cacheRoot = scratch / "cache"; + fs::create_directories(cacheRoot); + setenv("XDG_CACHE_HOME", cacheRoot.string().c_str(), 1); + + fs::path projectDir = scratch / "project"; + fs::create_directories(projectDir); + fs::path projectFile = projectDir / "project.cpp"; + std::ofstream(projectFile) << + "import std;\n" + "import Crafter.Build;\n" + "using namespace Crafter;\n" + "extern \"C\" Configuration CrafterBuildProject(std::span) {\n" + " Configuration cfg;\n" + " cfg.path = \"./\";\n" + " cfg.name = \"stamped\";\n" + " cfg.outputName = \"stamped\";\n" + " cfg.target = HostTarget();\n" + " cfg.type = ConfigurationType::Executable;\n" + " return cfg;\n" + "}\n"; + + std::array noArgs = {}; + + try { + (void)LoadProject(projectFile, noArgs); + } catch (const std::exception& e) { + std::println(std::cerr, "FAIL: first LoadProject threw: {}", e.what()); + return 1; + } + + fs::path moduleCacheDir = FindModuleCacheDir(cacheRoot); + Check(!moduleCacheDir.empty(), "a module cache directory was created"); + if (moduleCacheDir.empty()) return 1; + + fs::path probePcm = moduleCacheDir / "Crafter.Build-Clang.pcm"; + Check(fs::exists(probePcm), "the cold run populated the cache"); + if (!fs::exists(probePcm)) return 1; + + std::string firstStamp = ReadStamp(moduleCacheDir); + Check(!firstStamp.empty(), "the cold run recorded a source stamp"); + fs::file_time_type afterCold = fs::last_write_time(probePcm); + + // Unchanged sources must be a no-op — the stamp is an invalidation key, not + // an excuse to precompile eleven modules on every invocation. + try { + (void)LoadProject(projectFile, noArgs); + } catch (const std::exception& e) { + std::println(std::cerr, "FAIL: warm LoadProject threw: {}", e.what()); + return 1; + } + Check(fs::last_write_time(probePcm) == afterCold, "unchanged sources do not rebuild the cache"); + Check(ReadStamp(moduleCacheDir) == firstStamp, "unchanged sources keep the same stamp"); + + // Now the case the mtime rule cannot see. The edit lands in a module the + // probe does not name, because the cached PCMs import each other: a change + // anywhere in the set invalidates all of them. + fs::path edited = home / "Crafter.Build-Progress.cppm"; + { + std::ofstream append(edited, std::ios::app); + append << "\n// Content change with a backdated timestamp.\n"; + } + fs::last_write_time(edited, afterCold - std::chrono::hours(24)); + Check(fs::last_write_time(edited) < fs::last_write_time(probePcm), "the edited source really is older than the cached PCM"); + + try { + (void)LoadProject(projectFile, noArgs); + } catch (const std::exception& e) { + std::println(std::cerr, "FAIL: LoadProject after the edit threw: {}", e.what()); + return 1; + } + + Check(fs::last_write_time(probePcm) > afterCold, "a content change rebuilds the cache even when the source's mtime is older"); + Check(ReadStamp(moduleCacheDir) != firstStamp, "the recorded stamp follows the sources"); + + fs::remove_all(scratch, ec); + + if (Failures > 0) { + std::println(std::cerr, "{} assertions failed", Failures); + return 1; + } + return 0; +} diff --git a/tests/IncrementalInterfaceChange/fixture/lib/Widget-grown.cppm.in b/tests/IncrementalInterfaceChange/fixture/lib/Widget-grown.cppm.in new file mode 100644 index 0000000..e1616a2 --- /dev/null +++ b/tests/IncrementalInterfaceChange/fixture/lib/Widget-grown.cppm.in @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: LGPL-3.0-only +// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +// Copied over lib/Widget.cppm mid-test to stand in for the interface edit from +// issue #27: one extra data member, every signature and mangled name unchanged. +// Kept as a .cppm.in so the module scanner never treats it as a source of its +// own — and so the text does not have to live in a string literal inside the +// test, where `export module Widget;` would make the test itself look like an +// implementation unit of Widget. + +export module Widget; +import std; + +export struct Widget { + std::string a; + std::string b; +}; + +export std::size_t WidgetSizeInLibrary(); diff --git a/tests/IncrementalInterfaceChange/fixture/lib/Widget.cpp b/tests/IncrementalInterfaceChange/fixture/lib/Widget.cpp new file mode 100644 index 0000000..7228586 --- /dev/null +++ b/tests/IncrementalInterfaceChange/fixture/lib/Widget.cpp @@ -0,0 +1,7 @@ +// SPDX-License-Identifier: LGPL-3.0-only +// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +module Widget; +import std; + +std::size_t WidgetSizeInLibrary() { return sizeof(Widget); } diff --git a/tests/IncrementalInterfaceChange/fixture/lib/Widget.cppm b/tests/IncrementalInterfaceChange/fixture/lib/Widget.cppm new file mode 100644 index 0000000..aefc61e --- /dev/null +++ b/tests/IncrementalInterfaceChange/fixture/lib/Widget.cppm @@ -0,0 +1,19 @@ +// SPDX-License-Identifier: LGPL-3.0-only +// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +export module Widget; +import std; + +// The test rewrites this struct to add a member. Adding one changes the class +// layout while leaving every signature and mangled name untouched, so nothing +// downstream fails to link — a consumer object left over from before the change +// keeps the old sizeof and quietly disagrees with the library. +export struct Widget { + std::string a; +}; + +// Deliberately out-of-line, in the library's own implementation unit, so the +// value reflects the layout the *library* was compiled against rather than the +// caller's. An inline body would be instantiated from the (rebuilt) BMI in the +// consumer and would agree with it by construction. +export std::size_t WidgetSizeInLibrary(); diff --git a/tests/IncrementalInterfaceChange/fixture/main.cpp b/tests/IncrementalInterfaceChange/fixture/main.cpp new file mode 100644 index 0000000..667809b --- /dev/null +++ b/tests/IncrementalInterfaceChange/fixture/main.cpp @@ -0,0 +1,17 @@ +// SPDX-License-Identifier: LGPL-3.0-only +// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +import std; +import Widget; + +// Prints " " and exits 1 when they disagree. A mismatch is the +// observable form of the mixed-layout binary a stale consumer object produces — +// this reports it instead of waiting for the SIGSEGV that the real-world case +// (issue #27) produced in a destructor. +int main() { + std::size_t here = sizeof(Widget); + std::size_t inLibrary = WidgetSizeInLibrary(); + std::print("{} {}", here, inLibrary); + return here == inLibrary ? 0 : 1; +} diff --git a/tests/IncrementalInterfaceChange/main.cpp b/tests/IncrementalInterfaceChange/main.cpp new file mode 100644 index 0000000..a570060 --- /dev/null +++ b/tests/IncrementalInterfaceChange/main.cpp @@ -0,0 +1,199 @@ +// 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; + +// Adding a data member to a class in a module interface must rebuild every +// object compiled against the old layout. The dangerous shape (issue #27) is a +// consumer whose sources were scanned *before* its `dependencies` were assigned: +// its `import ;` matched nothing, so its object carried no staleness +// edge to the interface, and a layout change rebuilt the library, relinked the +// consumer, and produced a binary mixing both layouts with no error or warning. +// +// `AddTest` is exactly that shape — it scans tests//main.cpp and only then +// returns a builder whose .Dependencies() supplies the library — which is why +// the original report saw the corruption in test executables specifically. + +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" / "IncrementalInterfaceChange" / "fixture"; + fs::path staged = fs::temp_directory_path() / "crafter-build-incremental-interface-change"; + fs::remove_all(staged); + fs::copy(source, staged, fs::copy_options::recursive); + return staged; + } + + // Swap in the variant carrying the extra member. Comes from a file rather + // than a string literal here: the module scanner reads raw source, so an + // `export module Widget;` spelled inside this test would make the test look + // like an implementation unit of Widget. + void GrowWidget(const fs::path& staged) { + fs::copy_file(staged / "lib" / "Widget-grown.cppm.in", staged / "lib" / "Widget.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 / "lib" / "Widget.cppm", fs::file_time_type::clock::now()); + } + + std::unique_ptr MakeLib(const fs::path& staged) { + auto lib = std::make_unique(); + lib->path = staged / "lib"; + lib->name = "widget"; + lib->outputName = "widget"; + lib->target = HostTarget(); + lib->type = ConfigurationType::LibraryStatic; + std::array ifaces = { "Widget" }; + std::array impls = { "Widget" }; + lib->GetInterfacesAndImplementations(ifaces, impls); + return lib; + } + + // Scan first, wire the dependency up afterwards — the ordering that used to + // silently drop the staleness edge. + Configuration MakeConsumerScannedBeforeDependencies(const fs::path& staged, Configuration* lib) { + Configuration app; + app.path = staged; + app.name = "widget-app"; + app.outputName = "widget-app"; + app.target = HostTarget(); + app.type = ConfigurationType::Executable; + std::array ifaces = {}; + std::array impls = { "main" }; + app.GetInterfacesAndImplementations(ifaces, impls); + app.dependencies = { lib }; + return app; + } + + bool BuildOk(Configuration& app, 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 + // library's second build entirely. + std::unordered_map> depResults; + std::mutex depMutex; + BuildResult r = Build(app, depResults, depMutex); + if (!r.result.empty()) { + std::println(std::cerr, "FAIL: {} build failed: {}", label, r.result); + ++Failures; + return false; + } + return true; + } +} + +int main() { + { + // The scan leaves the unmatched import recorded rather than forgotten, + // and ResolvePendingImports places it once the library is reachable. + fs::path staged = StageFixture(); + std::unique_ptr lib = MakeLib(staged); + Configuration app; + app.path = staged; + app.name = "widget-app"; + app.outputName = "widget-app"; + app.target = HostTarget(); + app.type = ConfigurationType::Executable; + std::array ifaces = {}; + std::array impls = { "main" }; + app.GetInterfacesAndImplementations(ifaces, impls); + + Check(app.implementations.size() == 1, "consumer has one implementation"); + if (app.implementations.size() == 1) { + const Implementation& impl = app.implementations[0]; + Check(impl.externalModuleDependencies.empty(), "no external dep resolvable before dependencies are assigned"); + Check(std::ranges::find(impl.pendingImports, "Widget") != impl.pendingImports.end(), "unresolved 'import Widget;' is recorded as pending"); + + app.dependencies = { lib.get() }; + app.ResolvePendingImports(); + + Check(impl.externalModuleDependencies.size() == 1, "ResolvePendingImports adds the external module dep"); + if (impl.externalModuleDependencies.size() == 1) { + Check(impl.externalModuleDependencies[0].first->name == "Widget", "external dep is the Widget module"); + Check(impl.externalModuleDependencies[0].second == lib->PcmDir() / "Widget.pcm", "external dep points at the library's BMI"); + } + Check(std::ranges::find(impl.pendingImports, "Widget") == impl.pendingImports.end(), "resolved import is no longer pending"); + + // Idempotent: a second sweep must not duplicate the edge (Build + // runs one unconditionally, on top of whatever callers already did). + app.ResolvePendingImports(); + Check(impl.externalModuleDependencies.size() == 1, "ResolvePendingImports is idempotent"); + } + } + + { + // AddTest is the reported path: it scans the test source, then hands + // back a builder whose .Dependencies() names the library. + fs::path staged = StageFixture(); + fs::create_directories(staged / "tests" / "Consumer"); + fs::copy_file(staged / "main.cpp", staged / "tests" / "Consumer" / "main.cpp"); + std::unique_ptr lib = MakeLib(staged); + + Configuration app; + app.path = staged; + app.name = "host"; + app.outputName = "host"; + app.target = HostTarget(); + app.type = ConfigurationType::Executable; + // AddTest resolves tests//main against the cwd, which for a real + // run is the directory holding project.cpp. Stand in the staged project + // for the declaration so the fixture's test source is the one scanned. + fs::path restore = fs::current_path(); + fs::current_path(staged); + app.AddTest("Consumer").Dependencies({ lib.get() }); + fs::current_path(restore); + + Check(app.tests.size() == 1, "one test declared"); + if (app.tests.size() == 1 && app.tests[0].config.implementations.size() == 1) { + const Implementation& impl = app.tests[0].config.implementations[0]; + Check(impl.externalModuleDependencies.size() == 1, "AddTest(...).Dependencies() resolves the test's import of the library module"); + Check(impl.pendingImports.empty() || std::ranges::find(impl.pendingImports, "Widget") == impl.pendingImports.end(), "test's 'import Widget;' is no longer pending"); + } + } + + { + // End to end, and deliberately without calling ResolvePendingImports: + // the guarantee under test is that Build() closes the window on its own, + // for consumers that never knew they had to ask. + fs::path staged = StageFixture(); + std::unique_ptr lib = MakeLib(staged); + Configuration app = MakeConsumerScannedBeforeDependencies(staged, lib.get()); + fs::path binary = app.BinDir() / "widget-app"; + fs::path consumerObject = app.BuildDir() / "main_impl.o"; + + if (BuildOk(app, "first pass")) { + auto first = RunCommandWithTimeout(binary.string(), std::chrono::seconds(30)); + Check(first.exitCode == 0 && !first.crashed && !first.timedOut, std::format("first pass agrees on the layout (exit={} output='{}')", first.exitCode, first.output)); + + fs::file_time_type objectBefore = fs::last_write_time(consumerObject); + + // Same edit as the original report: one more member on a class in a + // module interface. Every signature and mangled name is unchanged, + // so a missed rebuild produces no diagnostic of any kind. + GrowWidget(staged); + + if (BuildOk(app, "second pass")) { + Check(fs::last_write_time(consumerObject) > objectBefore, "consumer object is recompiled after the interface gains a member"); + + auto second = RunCommandWithTimeout(binary.string(), std::chrono::seconds(30)); + Check(second.exitCode == 0 && !second.crashed && !second.timedOut, std::format("second pass agrees on the layout (exit={} output='{}')", second.exitCode, second.output)); + } + } + } + + if (Failures > 0) { + std::println(std::cerr, "{} assertions failed", Failures); + return 1; + } + return 0; +} diff --git a/tests/StandardArgs/main.cpp b/tests/StandardArgs/main.cpp index b30ca46..5024908 100644 --- a/tests/StandardArgs/main.cpp +++ b/tests/StandardArgs/main.cpp @@ -83,6 +83,31 @@ int main() { Check(!q.Get("--other=").has_value(), "ArgQuery::Get returns nullopt for absent prefix"); } + // Flags ApplyStandardArgs doesn't itself interpret are collected into + // cfg.projectArgs, which feeds VariantId — a project flag that changes what + // gets compiled has to move the output directory with it. + { + Configuration cfg; + std::array raw = { + "--debug", "--no-webgpu", "--target=aarch64-linux-gnu", "--feature=fancy", "bare", + }; + ApplyStandardArgs(cfg, raw); + std::vector expected = { "--feature=fancy", "--no-webgpu", "bare" }; + Check(cfg.projectArgs == expected, std::format("only unrecognised args are collected, sorted; got [{}]", std::format("{}", std::views::join_with(cfg.projectArgs, std::string(", ")) | std::ranges::to()))); + } + + // Order and repetition must not perturb the set — otherwise the same build + // spelled two ways would land in two directories. + { + Configuration a; + Configuration b; + std::array forward = { "--alpha", "--beta" }; + std::array reversed = { "--beta", "--alpha", "--beta" }; + ApplyStandardArgs(a, forward); + ApplyStandardArgs(b, reversed); + Check(a.projectArgs == b.projectArgs, "projectArgs is order- and duplicate-insensitive"); + } + if (Failures > 0) { std::println(std::cerr, "{} assertions failed", Failures); return 1; diff --git a/tests/VariantId/main.cpp b/tests/VariantId/main.cpp index 0f1b2e4..022b0d6 100644 --- a/tests/VariantId/main.cpp +++ b/tests/VariantId/main.cpp @@ -96,6 +96,25 @@ int main() { b.march = "x86-64-v3"; Check(a.VariantId() != b.VariantId(), "march change perturbs VariantId"); } + { + // A project flag the framework can't interpret still decides what gets + // compiled and bundled, so it has to key into VariantId. Otherwise both + // settings share one bin dir and interleave their outputs there — an + // index.html from one alongside a script from the other. + Configuration a = MakeBase(); + Configuration b = MakeBase(); + b.projectArgs.push_back("--no-webgpu"); + Check(a.VariantId() != b.VariantId(), "project arg perturbs VariantId"); + Check(a.BinDir() != b.BinDir(), "project arg yields a distinct BinDir"); + } + { + // Distinct project args must not collide with each other either. + Configuration a = MakeBase(); + a.projectArgs.push_back("--no-webgpu"); + Configuration b = MakeBase(); + b.projectArgs.push_back("--no-audio"); + Check(a.VariantId() != b.VariantId(), "different project args yield different VariantId"); + } { // PcmDir() differs between Executable (in BuildDir) and Library // (in BinDir) — Library PCMs land in the installable bin dir so