Merge branch 'master' of https://forgejo.catcrafts.net/Catcrafts/Crafter.Build

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; }

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<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();

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

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

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;
+}

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

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

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

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

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 "<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;
+}

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 <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;
+}