Merge pull request 'Rebuild when an #included header changes' (#31) from claude/issue-30 into master
Some checks failed
CI / build-test-release (push) Failing after 11m21s
Some checks failed
CI / build-test-release (push) Failing after 11m21s
This commit is contained in:
commit
8d921fd7bd
18 changed files with 443 additions and 8 deletions
|
|
@ -121,12 +121,15 @@ Per-import precise tracking for both within-project and cross-project module dep
|
||||||
|
|
||||||
- Touch `lib/Hello.cppm` → only consumers of `Hello` rebuild.
|
- Touch `lib/Hello.cppm` → only consumers of `Hello` rebuild.
|
||||||
- Touch `lib/Other.cppm` → only consumers of `Other` rebuild.
|
- Touch `lib/Other.cppm` → only consumers of `Other` rebuild.
|
||||||
|
- Touch a header → only the units that `#include` it (directly or transitively) rebuild.
|
||||||
- External CMake dep produces fresh `.a` files → whole project rebuilds (deliberately coarse — cmake-dep changes are rare).
|
- External CMake dep produces fresh `.a` files → whole project rebuilds (deliberately coarse — cmake-dep changes are rare).
|
||||||
|
|
||||||
Diamond deps (`A → {B, C}; B → X; C → X`) build `X` exactly once via a `std::shared_future<BuildResult>` cache.
|
Diamond deps (`A → {B, C}; B → X; C → X`) build `X` exactly once via a `std::shared_future<BuildResult>` cache.
|
||||||
|
|
||||||
Tracking is derived from each translation unit's `import` statements, which are scanned when the sources are declared. Every kind of unit is scanned — primary module interfaces, partitions and implementation units alike — and an interface that imports a sibling module in the same `Configuration` also gets its compile ordered behind it. `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 all of 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.
|
Tracking is derived from each translation unit's `import` statements, which are scanned when the sources are declared. Every kind of unit is scanned — primary module interfaces, partitions and implementation units alike — and an interface that imports a sibling module in the same `Configuration` also gets its compile ordered behind it. `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 all of 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.
|
||||||
|
|
||||||
|
`#include`s are tracked separately, because scanning can't see them: a header pulls in more headers, and which ones are open depends on the preprocessor state at the point of inclusion. So every compile is asked to report what it actually opened — `clang -MD -MF <artifact>.d`, written next to the object or BMI it belongs to — and the next staleness check reads that file back and compares every prerequisite's mtime against the artifact. C sources are covered the same way, and there they are the *only* dependency record. An artifact whose depfile is missing, or that names a prerequisite which no longer exists, is rebuilt once: neither state is evidence of freshness. Before this, a header edit changed no `.cppm` or `.cpp` mtime at all, so a build reported nothing to do and left objects compiled against the old contents — the same mixed-layout failure as above, reached through `#include` instead of `import`.
|
||||||
|
|
||||||
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 `<cache>/crafter.build/<target>-<march>/` 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.
|
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 `<cache>/crafter.build/<target>-<march>/` 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.
|
`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.
|
||||||
|
|
|
||||||
|
|
@ -780,12 +780,15 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
|
||||||
files += std::format(" {}_source.o ", (buildDir / cFile.filename()).string());
|
files += std::format(" {}_source.o ", (buildDir / cFile.filename()).string());
|
||||||
const std::string objPath = std::format("{}_source.o", (buildDir / cFile.filename()).string());
|
const std::string objPath = std::format("{}_source.o", (buildDir / cFile.filename()).string());
|
||||||
const std::string srcPath = std::format("{}.c", cFile.string());
|
const std::string srcPath = std::format("{}.c", cFile.string());
|
||||||
if (!fs::exists(objPath) || (fs::exists(srcPath) && fs::last_write_time(srcPath) > fs::last_write_time(objPath))) {
|
// C sources carry their dependencies entirely in #includes, so the
|
||||||
|
// depfile the compile below writes is the whole record of what this
|
||||||
|
// object was built from beyond the .c itself.
|
||||||
|
if (!fs::exists(objPath) || (fs::exists(srcPath) && fs::last_write_time(srcPath) > fs::last_write_time(objPath)) || NewestPrerequisite(std::format("{}.d", objPath)) > fs::last_write_time(objPath)) {
|
||||||
threads.emplace_back([&cFile, &buildDir, &buildError, &buildCancelled, &config, &includeFlags, &defineFlags, &userFlags, &cArchFlags, <oCompileFlags]() {
|
threads.emplace_back([&cFile, &buildDir, &buildError, &buildCancelled, &config, &includeFlags, &defineFlags, &userFlags, &cArchFlags, <oCompileFlags]() {
|
||||||
Progress::Task task(std::format("Compiling {}.c", cFile.filename().string()));
|
Progress::Task task(std::format("Compiling {}.c", cFile.filename().string()));
|
||||||
if (buildCancelled.load(std::memory_order_relaxed)) return;
|
if (buildCancelled.load(std::memory_order_relaxed)) return;
|
||||||
|
|
||||||
std::string result = RunCommand(std::format("clang {}.c --target={}{} -O3{} -c{}{}{} -o {}_source.o", cFile.string(), config.target, cArchFlags, ltoCompileFlags, includeFlags, defineFlags, userFlags, (buildDir / cFile.filename()).string()));
|
std::string result = RunCommand(std::format("clang {0}.c --target={1}{2} -O3{3} -c{4}{5}{6} -MD -MF {7}_source.o.d -o {7}_source.o", cFile.string(), config.target, cArchFlags, ltoCompileFlags, includeFlags, defineFlags, userFlags, (buildDir / cFile.filename()).string()));
|
||||||
if (result.empty()) return;
|
if (result.empty()) return;
|
||||||
|
|
||||||
bool expected = false;
|
bool expected = false;
|
||||||
|
|
@ -800,6 +803,10 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
|
||||||
files += std::format(" {}_source.o ", (buildDir / cFile.filename()).string());
|
files += std::format(" {}_source.o ", (buildDir / cFile.filename()).string());
|
||||||
const std::string objPath = std::format("{}_source.o", (buildDir / cFile.filename()).string());
|
const std::string objPath = std::format("{}_source.o", (buildDir / cFile.filename()).string());
|
||||||
const std::string srcPath = std::format("{}.cu", cFile.string());
|
const std::string srcPath = std::format("{}.cu", cFile.string());
|
||||||
|
// Still .cu-mtime only: the header tracking the C and C++ paths get
|
||||||
|
// comes from asking the compiler for a depfile, and nvcc's -MD/-MF are
|
||||||
|
// untested here (no CUDA toolkit in CI), so a .cu that only sees a
|
||||||
|
// changed header does not rebuild yet.
|
||||||
if (!fs::exists(objPath) || (fs::exists(srcPath) && fs::last_write_time(srcPath) > fs::last_write_time(objPath))) {
|
if (!fs::exists(objPath) || (fs::exists(srcPath) && fs::last_write_time(srcPath) > fs::last_write_time(objPath))) {
|
||||||
threads.emplace_back([&cFile, &buildDir, &buildError, &buildCancelled]() {
|
threads.emplace_back([&cFile, &buildDir, &buildError, &buildCancelled]() {
|
||||||
Progress::Task task(std::format("Compiling {}.cu", cFile.filename().string()));
|
Progress::Task task(std::format("Compiling {}.cu", cFile.filename().string()));
|
||||||
|
|
@ -1102,6 +1109,21 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map<fs::path, s
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
// C and CUDA objects are archive members too, and their compile
|
||||||
|
// loop above is the only thing that knows it rebuilt one — it
|
||||||
|
// sets no repack flag, so without this a .c edit recompiled the
|
||||||
|
// object and then linked nothing.
|
||||||
|
if (!buildResult.repack) {
|
||||||
|
auto sourceObjNewer = [&](const std::vector<fs::path>& sources) {
|
||||||
|
for (const fs::path& source : sources) {
|
||||||
|
if (objNewer(buildDir / std::format("{}_source.o", source.filename().string()))) return true;
|
||||||
|
}
|
||||||
|
return false;
|
||||||
|
};
|
||||||
|
if (sourceObjNewer(config.cFiles) || sourceObjNewer(config.cuda)) {
|
||||||
|
buildResult.repack = true;
|
||||||
|
}
|
||||||
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|
|
||||||
|
|
@ -15,7 +15,10 @@ namespace Crafter {
|
||||||
bool Implementation::Check(const fs::path& buildDir, const fs::path& pcmDir, fs::file_time_type sourceFloor) const {
|
bool Implementation::Check(const fs::path& buildDir, const fs::path& pcmDir, fs::file_time_type sourceFloor) const {
|
||||||
std::string objPath = std::format("{}_impl.o", (buildDir/path.filename()).string());
|
std::string objPath = std::format("{}_impl.o", (buildDir/path.filename()).string());
|
||||||
std::string cppPath = std::format("{}.cpp", path.string());
|
std::string cppPath = std::format("{}.cpp", path.string());
|
||||||
if(!fs::exists(objPath) || std::max(fs::last_write_time(cppPath), sourceFloor) >= fs::last_write_time(objPath)) {
|
// The depfile clang wrote beside the object lists the headers this TU
|
||||||
|
// included; nothing else in the configuration knows about them, since
|
||||||
|
// the source scanner only reads `import` lines.
|
||||||
|
if(!fs::exists(objPath) || std::max({fs::last_write_time(cppPath), sourceFloor, NewestPrerequisite(std::format("{}.d", objPath))}) >= fs::last_write_time(objPath)) {
|
||||||
return true;
|
return true;
|
||||||
}
|
}
|
||||||
fs::file_time_type objTime = fs::last_write_time(objPath);
|
fs::file_time_type objTime = fs::last_write_time(objPath);
|
||||||
|
|
@ -48,7 +51,8 @@ namespace Crafter {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
std::string result = RunCommand(std::format("{} {}.cpp -c -o {}_impl.o", clang, path.string(), (buildDir/path.filename()).string()));
|
// -MD leaves <name>_impl.o.d beside the object for Check to read.
|
||||||
|
std::string result = RunCommand(std::format("{0} {1}.cpp -c -MD -MF {2}_impl.o.d -o {2}_impl.o", clang, path.string(), (buildDir/path.filename()).string()));
|
||||||
|
|
||||||
bool expected = false;
|
bool expected = false;
|
||||||
if(!result.empty() && buildCancelled.compare_exchange_strong(expected, true)) {
|
if(!result.empty() && buildCancelled.compare_exchange_strong(expected, true)) {
|
||||||
|
|
|
||||||
|
|
@ -15,7 +15,13 @@ namespace Crafter {
|
||||||
checked = true;
|
checked = true;
|
||||||
std::string pcmPath = std::format("{}.pcm", (pcmDir/path.filename()).generic_string());
|
std::string pcmPath = std::format("{}.pcm", (pcmDir/path.filename()).generic_string());
|
||||||
std::string cppmPath = std::format("{}.cppm", path.generic_string());
|
std::string cppmPath = std::format("{}.cppm", path.generic_string());
|
||||||
if(fs::exists(pcmPath) && std::max(fs::last_write_time(cppmPath), sourceFloor) < fs::last_write_time(pcmPath)) {
|
// NewestPrerequisite covers the headers this partition #includes —
|
||||||
|
// they are inputs to the BMI just as much as the .cppm is, and the
|
||||||
|
// .cppm's own mtime says nothing about them. It only ever bounds
|
||||||
|
// *this* artifact, so it goes in the comparison below and not into
|
||||||
|
// the `sourceFloor` handed to the recursive Checks: each of those
|
||||||
|
// reads its own depfile.
|
||||||
|
if(fs::exists(pcmPath) && std::max({fs::last_write_time(cppmPath), sourceFloor, NewestPrerequisite(std::format("{}.d", pcmPath))}) < fs::last_write_time(pcmPath)) {
|
||||||
fs::file_time_type pcmTime = fs::last_write_time(pcmPath);
|
fs::file_time_type pcmTime = fs::last_write_time(pcmPath);
|
||||||
for(ModulePartition* dependency : partitionDependencies) {
|
for(ModulePartition* dependency : partitionDependencies) {
|
||||||
if(dependency->Check(pcmDir, sourceFloor)) {
|
if(dependency->Check(pcmDir, sourceFloor)) {
|
||||||
|
|
@ -71,7 +77,9 @@ namespace Crafter {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
std::string result = RunCommand(std::format("{} {}.cppm --precompile -o {}.pcm", clang, path.string(), (pcmDir/path.filename()).string()));
|
// -MD records every header the preamble pulled in, next to the BMI as
|
||||||
|
// <name>.pcm.d, so the next Check can see an edit to one of them.
|
||||||
|
std::string result = RunCommand(std::format("{0} {1}.cppm --precompile -MD -MF {2}.pcm.d -o {2}.pcm", clang, path.string(), (pcmDir/path.filename()).string()));
|
||||||
|
|
||||||
if (!result.empty()) {
|
if (!result.empty()) {
|
||||||
bool expected = false;
|
bool expected = false;
|
||||||
|
|
@ -103,7 +111,9 @@ namespace Crafter {
|
||||||
checked = true;
|
checked = true;
|
||||||
std::string pcmPath = std::format("{}.pcm", (pcmDir/path.filename()).generic_string());
|
std::string pcmPath = std::format("{}.pcm", (pcmDir/path.filename()).generic_string());
|
||||||
std::string cppmPath = std::format("{}.cppm", path.generic_string());
|
std::string cppmPath = std::format("{}.cppm", path.generic_string());
|
||||||
if(fs::exists(pcmPath) && std::max(fs::last_write_time(cppmPath), sourceFloor) < fs::last_write_time(pcmPath)) {
|
// See ModulePartition::Check — the depfile clang wrote beside the BMI
|
||||||
|
// is the only record of which headers this interface unit included.
|
||||||
|
if(fs::exists(pcmPath) && std::max({fs::last_write_time(cppmPath), sourceFloor, NewestPrerequisite(std::format("{}.d", pcmPath))}) < fs::last_write_time(pcmPath)) {
|
||||||
fs::file_time_type pcmTime = fs::last_write_time(pcmPath);
|
fs::file_time_type pcmTime = fs::last_write_time(pcmPath);
|
||||||
// Every partition gets Check()ed even once one is known stale:
|
// Every partition gets Check()ed even once one is known stale:
|
||||||
// Compile() drives partition rebuilds off their own
|
// Compile() drives partition rebuilds off their own
|
||||||
|
|
@ -191,7 +201,9 @@ namespace Crafter {
|
||||||
return;
|
return;
|
||||||
}
|
}
|
||||||
|
|
||||||
std::string result = RunCommand(std::format("{} {}.cppm --precompile -o {}.pcm", clang, path.string(), (pcmDir/path.filename()).string()));
|
// -MD records every header the preamble pulled in, next to the BMI as
|
||||||
|
// <name>.pcm.d, so the next Check can see an edit to one of them.
|
||||||
|
std::string result = RunCommand(std::format("{0} {1}.cppm --precompile -MD -MF {2}.pcm.d -o {2}.pcm", clang, path.string(), (pcmDir/path.filename()).string()));
|
||||||
|
|
||||||
if (!result.empty()) {
|
if (!result.empty()) {
|
||||||
bool expected = false;
|
bool expected = false;
|
||||||
|
|
|
||||||
|
|
@ -164,6 +164,71 @@ fs::path Crafter::GetCrafterBuildHome() {
|
||||||
throw std::runtime_error(msg);
|
throw std::runtime_error(msg);
|
||||||
}
|
}
|
||||||
|
|
||||||
|
fs::file_time_type Crafter::NewestPrerequisite(const fs::path& depFile) {
|
||||||
|
std::ifstream in(depFile, std::ios::binary);
|
||||||
|
if (!in) return fs::file_time_type::max();
|
||||||
|
std::ostringstream buffer;
|
||||||
|
buffer << in.rdbuf();
|
||||||
|
const std::string text = std::move(buffer).str();
|
||||||
|
|
||||||
|
// Make-rule tokenizing, as clang writes it: `target: prereq prereq`, with a
|
||||||
|
// trailing '\' continuing the rule onto the next line (clang wraps at ~80
|
||||||
|
// columns, so any real depfile has several), '\ ' and '\#' escaping a
|
||||||
|
// character into a filename, and '$$' standing for a literal '$'. A
|
||||||
|
// backslash that is none of those is a Windows path separator and stays.
|
||||||
|
std::vector<std::string> tokens;
|
||||||
|
std::string token;
|
||||||
|
auto flush = [&tokens, &token]() {
|
||||||
|
if (!token.empty()) {
|
||||||
|
tokens.push_back(token);
|
||||||
|
token.clear();
|
||||||
|
}
|
||||||
|
};
|
||||||
|
for (std::size_t i = 0; i < text.size(); ++i) {
|
||||||
|
const char c = text[i];
|
||||||
|
const char next = i + 1 < text.size() ? text[i + 1] : '\0';
|
||||||
|
if (c == '\\' && (next == '\n' || next == '\r')) {
|
||||||
|
flush();
|
||||||
|
++i;
|
||||||
|
} else if (c == '\\' && (next == ' ' || next == '\t' || next == '#')) {
|
||||||
|
token += next;
|
||||||
|
++i;
|
||||||
|
} else if (c == '$' && next == '$') {
|
||||||
|
token += '$';
|
||||||
|
++i;
|
||||||
|
} else if (c == ' ' || c == '\t' || c == '\n' || c == '\r') {
|
||||||
|
flush();
|
||||||
|
} else {
|
||||||
|
token += c;
|
||||||
|
}
|
||||||
|
}
|
||||||
|
flush();
|
||||||
|
|
||||||
|
// A token ending in ':' names a rule target, not an input. The first one
|
||||||
|
// closes the target list (and a Windows `C:\build\x.o:` still ends in ':',
|
||||||
|
// where splitting on the first colon would not); a later one opens another
|
||||||
|
// rule, as `-MP` phony targets do, whose name is not a file to stat.
|
||||||
|
bool sawTarget = false;
|
||||||
|
fs::file_time_type newest = fs::file_time_type::min();
|
||||||
|
for (const std::string& prerequisite : tokens) {
|
||||||
|
if (prerequisite.ends_with(':')) {
|
||||||
|
sawTarget = true;
|
||||||
|
continue;
|
||||||
|
}
|
||||||
|
if (!sawTarget) continue;
|
||||||
|
std::error_code ec;
|
||||||
|
const fs::file_time_type stamp = fs::last_write_time(prerequisite, ec);
|
||||||
|
// A prerequisite that has since been deleted makes the recorded set
|
||||||
|
// unusable as evidence — the source that included it must have changed
|
||||||
|
// too, and rebuilding is what writes a depfile that no longer names it.
|
||||||
|
if (ec) return fs::file_time_type::max();
|
||||||
|
if (stamp > newest) newest = stamp;
|
||||||
|
}
|
||||||
|
// No rule at all: a truncated or empty depfile proves nothing.
|
||||||
|
if (!sawTarget) return fs::file_time_type::max();
|
||||||
|
return newest;
|
||||||
|
}
|
||||||
|
|
||||||
bool Crafter::MatchGlob(std::string_view glob, std::string_view name) {
|
bool Crafter::MatchGlob(std::string_view glob, std::string_view name) {
|
||||||
std::size_t gi = 0;
|
std::size_t gi = 0;
|
||||||
std::size_t ni = 0;
|
std::size_t ni = 0;
|
||||||
|
|
|
||||||
|
|
@ -27,6 +27,18 @@ namespace Crafter {
|
||||||
// module sources, wasi-runtime/, etc). Honors CRAFTER_BUILD_HOME; otherwise
|
// module sources, wasi-runtime/, etc). Honors CRAFTER_BUILD_HOME; otherwise
|
||||||
// derives <prefix>/share/crafter-build from the running executable's path.
|
// derives <prefix>/share/crafter-build from the running executable's path.
|
||||||
export CRAFTER_API fs::path GetCrafterBuildHome();
|
export CRAFTER_API fs::path GetCrafterBuildHome();
|
||||||
|
// Newest mtime among the prerequisites a compiler recorded in `depFile`
|
||||||
|
// (clang's `-MD -MF <artifact>.d`). This is how a build step learns which
|
||||||
|
// headers its source pulled in: the module scanner only reads `import`
|
||||||
|
// lines, so an `#include`d file is invisible to it until the compiler
|
||||||
|
// reports what it actually opened.
|
||||||
|
//
|
||||||
|
// Returns file_time_type::max() when freshness cannot be proven — no
|
||||||
|
// depfile (nothing has compiled this artifact since depfiles were emitted,
|
||||||
|
// including every object left over from an older crafter-build) or a
|
||||||
|
// prerequisite that no longer exists. Callers compare the result against
|
||||||
|
// their artifact's mtime, so max() reads as "rebuild".
|
||||||
|
fs::file_time_type NewestPrerequisite(const fs::path& depFile);
|
||||||
// Wildcard name matching ('*', '?') shared by the test and lint verbs.
|
// Wildcard name matching ('*', '?') shared by the test and lint verbs.
|
||||||
bool MatchGlob(std::string_view glob, std::string_view name);
|
bool MatchGlob(std::string_view glob, std::string_view name);
|
||||||
// Empty `globs` matches everything.
|
// Empty `globs` matches everything.
|
||||||
|
|
|
||||||
|
|
@ -109,6 +109,7 @@ extern "C" Configuration CrafterBuildProject(std::span<const std::string_view> a
|
||||||
cfg.AddTest("DependencyLink").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("DependencyLink").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("IncrementalInterfaceChange").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("IncrementalInterfaceChange").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("TransitiveInterfaceChange").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("TransitiveInterfaceChange").Dependencies({ CrafterBuildLib.get() });
|
||||||
|
cfg.AddTest("IncrementalHeaderChange").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("CleanProject").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("CleanProject").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("ShaderCompile").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("ShaderCompile").Dependencies({ CrafterBuildLib.get() });
|
||||||
cfg.AddTest("StandardArgs").Dependencies({ CrafterBuildLib.get() });
|
cfg.AddTest("StandardArgs").Dependencies({ CrafterBuildLib.get() });
|
||||||
|
|
|
||||||
11
tests/IncrementalHeaderChange/fixture/lib/Widget.cpp
Normal file
11
tests/IncrementalHeaderChange/fixture/lib/Widget.cpp
Normal file
|
|
@ -0,0 +1,11 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
module;
|
||||||
|
#include "widget count.h"
|
||||||
|
module Widget;
|
||||||
|
import std;
|
||||||
|
|
||||||
|
std::size_t WidgetSizeInLibrary() { return sizeof(Widget); }
|
||||||
|
|
||||||
|
std::int64_t WidgetCountInLibrary() { return WIDGET_COUNT; }
|
||||||
21
tests/IncrementalHeaderChange/fixture/lib/Widget.cppm
Normal file
21
tests/IncrementalHeaderChange/fixture/lib/Widget.cppm
Normal file
|
|
@ -0,0 +1,21 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
module;
|
||||||
|
// The whole point of the fixture: an input to this interface unit that the
|
||||||
|
// module scanner cannot see. WIDGET_SLOTS comes from a header, so the class
|
||||||
|
// layout below changes without Widget.cppm being touched.
|
||||||
|
#include "widget-layout.h"
|
||||||
|
export module Widget;
|
||||||
|
import std;
|
||||||
|
|
||||||
|
export struct Widget {
|
||||||
|
std::array<std::int64_t, WIDGET_SLOTS> slots;
|
||||||
|
};
|
||||||
|
|
||||||
|
// Out-of-line in the library's implementation unit, so the value reflects the
|
||||||
|
// layout the *library* was compiled against rather than the caller's — same
|
||||||
|
// reasoning as the IncrementalInterfaceChange fixture.
|
||||||
|
export std::size_t WidgetSizeInLibrary();
|
||||||
|
// Reads a macro from a header only the implementation unit includes.
|
||||||
|
export std::int64_t WidgetCountInLibrary();
|
||||||
|
|
@ -0,0 +1,7 @@
|
||||||
|
/* SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® */
|
||||||
|
|
||||||
|
/* Copied over lib/counter-limit.h mid-test. */
|
||||||
|
#pragma once
|
||||||
|
|
||||||
|
#define COUNTER_LIMIT 9
|
||||||
|
|
@ -0,0 +1,8 @@
|
||||||
|
/* SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® */
|
||||||
|
|
||||||
|
/* Included from counter.c — the C compile path keeps all of its dependencies in
|
||||||
|
#includes, so it has nothing but the depfile to go on. */
|
||||||
|
#pragma once
|
||||||
|
|
||||||
|
#define COUNTER_LIMIT 5
|
||||||
8
tests/IncrementalHeaderChange/fixture/lib/counter.c
Normal file
8
tests/IncrementalHeaderChange/fixture/lib/counter.c
Normal file
|
|
@ -0,0 +1,8 @@
|
||||||
|
/* SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® */
|
||||||
|
|
||||||
|
#include "counter-limit.h"
|
||||||
|
|
||||||
|
long CounterLimitInLibrary(void) {
|
||||||
|
return COUNTER_LIMIT;
|
||||||
|
}
|
||||||
|
|
@ -0,0 +1,7 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
// Copied over lib/widget count.h mid-test.
|
||||||
|
#pragma once
|
||||||
|
|
||||||
|
#define WIDGET_COUNT 2
|
||||||
12
tests/IncrementalHeaderChange/fixture/lib/widget count.h
Normal file
12
tests/IncrementalHeaderChange/fixture/lib/widget count.h
Normal file
|
|
@ -0,0 +1,12 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
// Included from Widget.cpp, the module's implementation unit — nothing else
|
||||||
|
// sees it, so an edit here must rebuild that one object and nothing more.
|
||||||
|
//
|
||||||
|
// The space in the filename is deliberate: clang escapes it as `widget\ count.h`
|
||||||
|
// in the depfile, so a build that reads depfiles has to unescape make syntax
|
||||||
|
// rather than splitting the line on whitespace.
|
||||||
|
#pragma once
|
||||||
|
|
||||||
|
#define WIDGET_COUNT 1
|
||||||
|
|
@ -0,0 +1,9 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
// Copied over lib/widget-layout.h mid-test: one more slot in the exported
|
||||||
|
// class. Kept as a .h.in so the original header is the only one any source
|
||||||
|
// #includes.
|
||||||
|
#pragma once
|
||||||
|
|
||||||
|
#define WIDGET_SLOTS 2
|
||||||
11
tests/IncrementalHeaderChange/fixture/lib/widget-layout.h
Normal file
11
tests/IncrementalHeaderChange/fixture/lib/widget-layout.h
Normal file
|
|
@ -0,0 +1,11 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
// Included from Widget.cppm's global module fragment. WIDGET_SLOTS decides the
|
||||||
|
// layout of the exported class, so growing it (the test copies
|
||||||
|
// widget-layout-grown.h.in over this file) invalidates the BMI and every object
|
||||||
|
// compiled against it — while leaving Widget.cppm itself untouched, which is
|
||||||
|
// exactly what the mtime comparison used to miss.
|
||||||
|
#pragma once
|
||||||
|
|
||||||
|
#define WIDGET_SLOTS 1
|
||||||
19
tests/IncrementalHeaderChange/fixture/main.cpp
Normal file
19
tests/IncrementalHeaderChange/fixture/main.cpp
Normal file
|
|
@ -0,0 +1,19 @@
|
||||||
|
// SPDX-License-Identifier: LGPL-3.0-only
|
||||||
|
// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts®
|
||||||
|
|
||||||
|
import std;
|
||||||
|
import Widget;
|
||||||
|
|
||||||
|
extern "C" long CounterLimitInLibrary();
|
||||||
|
|
||||||
|
// Prints "<sizeof here> <sizeof in library> <count> <limit>" and exits 1 when
|
||||||
|
// the two sizes disagree. The sizes are the mixed-layout check; the count and
|
||||||
|
// the limit report which build of the library's own objects got linked in, so a
|
||||||
|
// stale implementation object or C object shows up as an old value rather than
|
||||||
|
// as silence.
|
||||||
|
int main() {
|
||||||
|
std::size_t here = sizeof(Widget);
|
||||||
|
std::size_t inLibrary = WidgetSizeInLibrary();
|
||||||
|
std::print("{} {} {} {}", here, inLibrary, WidgetCountInLibrary(), CounterLimitInLibrary());
|
||||||
|
return here == inLibrary ? 0 : 1;
|
||||||
|
}
|
||||||
203
tests/IncrementalHeaderChange/main.cpp
Normal file
203
tests/IncrementalHeaderChange/main.cpp
Normal file
|
|
@ -0,0 +1,203 @@
|
||||||
|
// 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;
|
||||||
|
|
||||||
|
// Editing a header must rebuild everything that #includes it. The staleness
|
||||||
|
// check used to compare an artifact against its own source and its module
|
||||||
|
// imports only — and the module scanner reads `import` lines, so a header was
|
||||||
|
// invisible to it. A build after a header-only edit reported nothing to do and
|
||||||
|
// left objects compiled against the previous contents: the same silent
|
||||||
|
// mixed-layout binary as issue #27, reached through #include instead.
|
||||||
|
//
|
||||||
|
// Each pass below edits exactly one header and asserts both directions — the
|
||||||
|
// objects that include it are recompiled, the ones that don't are left alone —
|
||||||
|
// then runs the binary, because "was recompiled" is only interesting if the
|
||||||
|
// resulting program agrees with itself.
|
||||||
|
|
||||||
|
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" / "IncrementalHeaderChange" / "fixture";
|
||||||
|
fs::path staged = fs::temp_directory_path() / "crafter-build-incremental-header-change";
|
||||||
|
fs::remove_all(staged);
|
||||||
|
fs::copy(source, staged, fs::copy_options::recursive);
|
||||||
|
return staged;
|
||||||
|
}
|
||||||
|
|
||||||
|
// Swap <name>-grown.h.in in for <name>.h. The replacement text lives in a
|
||||||
|
// file rather than a string literal so the header the fixture #includes is
|
||||||
|
// the only one under that name, whichever variant is in place.
|
||||||
|
void Grow(const fs::path& header) {
|
||||||
|
fs::path grown = header.parent_path() / std::format("{}-grown.h.in", header.stem().string());
|
||||||
|
fs::copy_file(grown, header, fs::copy_options::overwrite_existing);
|
||||||
|
// copy_file carries the source's mtime across, which would leave the
|
||||||
|
// rewritten header looking older than the objects built from it.
|
||||||
|
fs::last_write_time(header, fs::file_time_type::clock::now());
|
||||||
|
}
|
||||||
|
|
||||||
|
std::unique_ptr<Configuration> MakeLib(const fs::path& staged) {
|
||||||
|
auto lib = std::make_unique<Configuration>();
|
||||||
|
lib->path = staged / "lib";
|
||||||
|
lib->name = "widget";
|
||||||
|
lib->outputName = "widget";
|
||||||
|
lib->target = HostTarget();
|
||||||
|
lib->type = ConfigurationType::LibraryStatic;
|
||||||
|
std::array<fs::path, 1> ifaces = { "Widget" };
|
||||||
|
std::array<fs::path, 1> impls = { "Widget" };
|
||||||
|
lib->GetInterfacesAndImplementations(ifaces, impls);
|
||||||
|
// cFiles are resolved against the cwd at build time, so spell it out.
|
||||||
|
lib->cFiles = { staged / "lib" / "counter" };
|
||||||
|
return lib;
|
||||||
|
}
|
||||||
|
|
||||||
|
Configuration MakeApp(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<fs::path, 0> ifaces = {};
|
||||||
|
std::array<fs::path, 1> 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<fs::path, std::shared_future<BuildResult>> 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;
|
||||||
|
}
|
||||||
|
|
||||||
|
// "<sizeof in the consumer> <sizeof in the library> <count> <limit>" — see
|
||||||
|
// the fixture's main.cpp.
|
||||||
|
void CheckRun(const fs::path& binary, std::string_view expected, std::string_view label) {
|
||||||
|
auto r = RunCommandWithTimeout(binary.string(), std::chrono::seconds(30));
|
||||||
|
Check(r.exitCode == 0 && !r.crashed && !r.timedOut && r.output == expected, std::format("{}: expected '{}', got '{}' (exit={})", label, expected, r.output, r.exitCode));
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
int main() {
|
||||||
|
fs::path staged = StageFixture();
|
||||||
|
std::unique_ptr<Configuration> lib = MakeLib(staged);
|
||||||
|
Configuration app = MakeApp(staged, lib.get());
|
||||||
|
|
||||||
|
fs::path binary = app.BinDir() / "widget-app";
|
||||||
|
// One artifact per compile path a header can reach: the module interface
|
||||||
|
// (BMI plus the object made from it), the module's implementation unit, a C
|
||||||
|
// source, and the consumer that imports the module.
|
||||||
|
fs::path interfacePcm = lib->PcmDir() / "Widget.pcm";
|
||||||
|
fs::path interfaceObject = lib->BuildDir() / "Widget.o";
|
||||||
|
fs::path libraryObject = lib->BuildDir() / "Widget_impl.o";
|
||||||
|
fs::path counterObject = lib->BuildDir() / "counter_source.o";
|
||||||
|
fs::path consumerObject = app.BuildDir() / "main_impl.o";
|
||||||
|
|
||||||
|
if (!BuildOk(app, "first pass")) {
|
||||||
|
std::println(std::cerr, "{} assertions failed", Failures);
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
CheckRun(binary, "8 8 1 5", "first pass");
|
||||||
|
|
||||||
|
auto stamps = [&]() {
|
||||||
|
return std::array<fs::file_time_type, 5>{
|
||||||
|
fs::last_write_time(interfacePcm),
|
||||||
|
fs::last_write_time(interfaceObject),
|
||||||
|
fs::last_write_time(libraryObject),
|
||||||
|
fs::last_write_time(counterObject),
|
||||||
|
fs::last_write_time(consumerObject),
|
||||||
|
};
|
||||||
|
};
|
||||||
|
|
||||||
|
// Nothing changed: the depfiles must not read as staleness of their own,
|
||||||
|
// or every build would recompile the world.
|
||||||
|
{
|
||||||
|
std::array<fs::file_time_type, 5> before = stamps();
|
||||||
|
if (BuildOk(app, "idle pass")) {
|
||||||
|
Check(stamps() == before, "an idle rebuild recompiles nothing");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A header only the module's implementation unit includes. Its name carries
|
||||||
|
// a space, so the depfile spells it escaped.
|
||||||
|
{
|
||||||
|
std::array<fs::file_time_type, 5> before = stamps();
|
||||||
|
Grow(staged / "lib" / "widget count.h");
|
||||||
|
if (BuildOk(app, "implementation header pass")) {
|
||||||
|
std::array<fs::file_time_type, 5> after = stamps();
|
||||||
|
Check(after[2] > before[2], "implementation object is recompiled after a header it includes changes");
|
||||||
|
Check(after[0] == before[0], "interface BMI is left alone by a header it does not include");
|
||||||
|
Check(after[3] == before[3], "C object is left alone by a header it does not include");
|
||||||
|
Check(after[4] == before[4], "consumer object is left alone by a header it does not include");
|
||||||
|
CheckRun(binary, "8 8 2 5", "implementation header pass");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A header only the C source includes.
|
||||||
|
{
|
||||||
|
std::array<fs::file_time_type, 5> before = stamps();
|
||||||
|
Grow(staged / "lib" / "counter-limit.h");
|
||||||
|
if (BuildOk(app, "C header pass")) {
|
||||||
|
std::array<fs::file_time_type, 5> after = stamps();
|
||||||
|
Check(after[3] > before[3], "C object is recompiled after a header it includes changes");
|
||||||
|
Check(after[2] == before[2], "implementation object is left alone by a header it does not include");
|
||||||
|
CheckRun(binary, "8 8 2 9", "C header pass");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// A header the interface unit includes, changing the layout of an exported
|
||||||
|
// class: the BMI and everything compiled against it has to follow.
|
||||||
|
{
|
||||||
|
std::array<fs::file_time_type, 5> before = stamps();
|
||||||
|
Grow(staged / "lib" / "widget-layout.h");
|
||||||
|
if (BuildOk(app, "interface header pass")) {
|
||||||
|
std::array<fs::file_time_type, 5> after = stamps();
|
||||||
|
Check(after[0] > before[0], "interface BMI is rebuilt after a header it includes changes");
|
||||||
|
Check(after[1] > before[1], "interface object is rebuilt after a header it includes changes");
|
||||||
|
Check(after[2] > before[2], "implementation object follows the rebuilt BMI");
|
||||||
|
Check(after[4] > before[4], "consumer object follows the rebuilt BMI");
|
||||||
|
CheckRun(binary, "16 16 2 9", "interface header pass");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
// An object built before depfiles were emitted at all — an upgrade over an
|
||||||
|
// existing build directory — has no record of what it included, so the one
|
||||||
|
// safe reading is "rebuild it once".
|
||||||
|
{
|
||||||
|
std::array<fs::file_time_type, 5> before = stamps();
|
||||||
|
fs::remove(lib->BuildDir() / "Widget_impl.o.d");
|
||||||
|
if (BuildOk(app, "missing depfile pass")) {
|
||||||
|
std::array<fs::file_time_type, 5> after = stamps();
|
||||||
|
Check(after[2] > before[2], "an object whose dependency record is missing is rebuilt");
|
||||||
|
Check(after[3] == before[3], "the objects that still have one are left alone");
|
||||||
|
}
|
||||||
|
}
|
||||||
|
|
||||||
|
if (Failures > 0) {
|
||||||
|
std::println(std::cerr, "{} assertions failed", Failures);
|
||||||
|
return 1;
|
||||||
|
}
|
||||||
|
return 0;
|
||||||
|
}
|
||||||
Loading…
Reference in a new issue