fix: key the host PCM cache on source content, add clean, hash project args

Three follow-ons to the stale-build report, all cases of an identity not
capturing something that changes the output.

The host PCM cache under <cache>/crafter.build/<target>-<march>/ is shared by
every crafter-build on the machine, and freshness was a per-file mtime
comparison. That cannot tell "this PCM is newer than my source" from "this PCM
was built from different sources that happen to be newer", so a package install
and a working checkout — or two checkouts of different versions — silently
compiled their project.cpp against each other's declarations. Invalidation now
keys on a stamp over the bytes of every module source, which also covers the
case one file's mtime never could: the cached PCMs import each other, so a
change to :Interface invalidates :Clang's PCM with Crafter.Build-Clang.cppm
untouched.

Project args ApplyStandardArgs does not itself interpret are now folded into
VariantId. Such a flag typically decides what gets compiled or bundled — the
report's example is --no-webgpu dropping entries from cfg.files — and without it
both settings shared one bin dir and interleaved their outputs there, leaving a
bundle matching neither. Sorted and deduplicated so flag order doesn't split the
cache, and inherited by test Configurations.

`crafter-build clean` removes the project's bin/ and build/ trees. It
deliberately does not load project.cpp: cleaning is most often reached when
something is already wrong, and a clean that first needs the project to compile
is useless exactly then.
This commit is contained in:
catbot 2026-07-30 17:43:09 +00:00
commit e8fde57582
9 changed files with 439 additions and 15 deletions

View file

@ -63,22 +63,22 @@ namespace {
}
void Configuration::ResolvePendingImports() {
// Discard the local-module hits: a name recorded as pending already failed
// to match this Configuration's own interfaces, and nothing adds interfaces
// between the scan and here. Only the external edge can newly appear.
std::vector<Module*> ignored;
auto sweep = [this, &ignored](std::vector<std::string>& pending, std::vector<std::pair<Module*, fs::path>>& externalDeps) {
// 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<std::string>& pending, std::vector<Module*>& localDeps, std::vector<std::pair<Module*, fs::path>>& externalDeps) {
std::erase_if(pending, [&](const std::string& name) {
return ResolveImportName(*this, name, ignored, externalDeps);
return ResolveImportName(*this, name, localDeps, externalDeps);
});
};
for(const std::unique_ptr<Module>& interface : interfaces) {
for(const std::unique_ptr<ModulePartition>& partition : interface->partitions) {
sweep(partition->pendingImports, partition->externalModuleDependencies);
sweep(partition->pendingImports, partition->moduleDependencies, partition->externalModuleDependencies);
}
}
for(Implementation& implementation : implementations) {
sweep(implementation.pendingImports, implementation.externalModuleDependencies);
sweep(implementation.pendingImports, implementation.moduleDependencies, implementation.externalModuleDependencies);
}
}
@ -1446,7 +1446,15 @@ ArgQuery Crafter::ApplyStandardArgs(Configuration& cfg, std::span<const std::str
else if (a.starts_with("--march=")) cfg.march = std::string(a.substr(std::string_view("--march=").size()));
else if (a.starts_with("--mtune=")) cfg.mtune = std::string(a.substr(std::string_view("--mtune=").size()));
else if (a.starts_with("--sysroot=")) cfg.sysroot = std::string(a.substr(std::string_view("--sysroot=").size()));
// Anything else is the project's own flag. Its effect on the output is
// opaque to the framework, so it has to key into VariantId or two flag
// settings share a bin dir and leave a bundle matching neither.
else cfg.projectArgs.emplace_back(a);
}
// Sorted and deduplicated so the identity depends on the set of flags, not
// on how they were ordered or repeated.
std::ranges::sort(cfg.projectArgs);
cfg.projectArgs.erase(std::ranges::unique(cfg.projectArgs).begin(), cfg.projectArgs.end());
if (sawLib && cfg.type == ConfigurationType::Executable) cfg.type = ConfigurationType::LibraryStatic;
if (sawShared && cfg.type == ConfigurationType::LibraryStatic) cfg.type = ConfigurationType::LibraryDynamic;
// WASI sysroot autodetect, applied at config-load time so the VariantId
@ -1461,6 +1469,21 @@ ArgQuery Crafter::ApplyStandardArgs(Configuration& cfg, std::span<const std::str
return ArgQuery{args};
}
std::vector<fs::path> Crafter::CleanProject(const fs::path& projectFile) {
fs::path projectDir = fs::absolute(projectFile).lexically_normal().parent_path();
std::vector<fs::path> removed;
for (std::string_view name : { "bin", "build" }) {
fs::path dir = projectDir / name;
std::error_code ec;
if (!fs::is_directory(dir, ec)) continue;
if (fs::remove_all(dir, ec) == static_cast<std::uintmax_t>(-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:
@ -1468,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=<path>), compiles it to a shared
@ -1510,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).
@ -1541,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;
@ -1551,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") {
@ -1603,6 +1637,17 @@ int Crafter::Run(int argc, char** argv) {
return 1;
}
// Ahead of LoadProject on purpose — see CleanProject.
if (runClean) {
std::vector<fs::path> 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);

View file

@ -74,6 +74,54 @@ namespace {
CacheLock(const CacheLock&) = delete;
CacheLock& operator=(const CacheLock&) = delete;
};
// The cached Crafter.Build PCMs are keyed by `<target>-<march>` 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<const std::string_view> 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<std::string>{}(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);
}
}

View file

@ -429,6 +429,10 @@ TestBuilder Configuration::AddTest(std::string_view name, std::span<fs::path> 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/<name>/main.cpp resolved against the
@ -453,6 +457,7 @@ void Configuration::AddMarchVariants(std::string_view name, std::span<fs::path>
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";