diff --git a/project.cpp b/project.cpp index 33d04a7..f3e5af1 100644 --- a/project.cpp +++ b/project.cpp @@ -109,6 +109,7 @@ extern "C" Configuration CrafterBuildProject(std::span a cfg.AddTest("DependencyLink").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("IncrementalInterfaceChange").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("TransitiveInterfaceChange").Dependencies({ CrafterBuildLib.get() }); + cfg.AddTest("IncrementalHeaderChange").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("CleanProject").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("ShaderCompile").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("StandardArgs").Dependencies({ CrafterBuildLib.get() }); diff --git a/tests/IncrementalHeaderChange/fixture/lib/Widget.cpp b/tests/IncrementalHeaderChange/fixture/lib/Widget.cpp new file mode 100644 index 0000000..cf222f1 --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/Widget.cpp @@ -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; } diff --git a/tests/IncrementalHeaderChange/fixture/lib/Widget.cppm b/tests/IncrementalHeaderChange/fixture/lib/Widget.cppm new file mode 100644 index 0000000..3240a22 --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/Widget.cppm @@ -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 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(); diff --git a/tests/IncrementalHeaderChange/fixture/lib/counter-limit-grown.h.in b/tests/IncrementalHeaderChange/fixture/lib/counter-limit-grown.h.in new file mode 100644 index 0000000..97ae023 --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/counter-limit-grown.h.in @@ -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 diff --git a/tests/IncrementalHeaderChange/fixture/lib/counter-limit.h b/tests/IncrementalHeaderChange/fixture/lib/counter-limit.h new file mode 100644 index 0000000..f83ba18 --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/counter-limit.h @@ -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 diff --git a/tests/IncrementalHeaderChange/fixture/lib/counter.c b/tests/IncrementalHeaderChange/fixture/lib/counter.c new file mode 100644 index 0000000..e1b99ee --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/counter.c @@ -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; +} diff --git a/tests/IncrementalHeaderChange/fixture/lib/widget count-grown.h.in b/tests/IncrementalHeaderChange/fixture/lib/widget count-grown.h.in new file mode 100644 index 0000000..89801ab --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/widget count-grown.h.in @@ -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 diff --git a/tests/IncrementalHeaderChange/fixture/lib/widget count.h b/tests/IncrementalHeaderChange/fixture/lib/widget count.h new file mode 100644 index 0000000..f310f51 --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/widget count.h @@ -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 diff --git a/tests/IncrementalHeaderChange/fixture/lib/widget-layout-grown.h.in b/tests/IncrementalHeaderChange/fixture/lib/widget-layout-grown.h.in new file mode 100644 index 0000000..d4c0485 --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/widget-layout-grown.h.in @@ -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 diff --git a/tests/IncrementalHeaderChange/fixture/lib/widget-layout.h b/tests/IncrementalHeaderChange/fixture/lib/widget-layout.h new file mode 100644 index 0000000..081caa7 --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/lib/widget-layout.h @@ -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 diff --git a/tests/IncrementalHeaderChange/fixture/main.cpp b/tests/IncrementalHeaderChange/fixture/main.cpp new file mode 100644 index 0000000..b1d8639 --- /dev/null +++ b/tests/IncrementalHeaderChange/fixture/main.cpp @@ -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 " " 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; +} diff --git a/tests/IncrementalHeaderChange/main.cpp b/tests/IncrementalHeaderChange/main.cpp new file mode 100644 index 0000000..bda0682 --- /dev/null +++ b/tests/IncrementalHeaderChange/main.cpp @@ -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 -grown.h.in in for .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 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); + // 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 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; + } + + // " " — 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 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::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 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 before = stamps(); + Grow(staged / "lib" / "widget count.h"); + if (BuildOk(app, "implementation header pass")) { + std::array 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 before = stamps(); + Grow(staged / "lib" / "counter-limit.h"); + if (BuildOk(app, "C header pass")) { + std::array 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 before = stamps(); + Grow(staged / "lib" / "widget-layout.h"); + if (BuildOk(app, "interface header pass")) { + std::array 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 before = stamps(); + fs::remove(lib->BuildDir() / "Widget_impl.o.d"); + if (BuildOk(app, "missing depfile pass")) { + std::array 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; +}