From e280c955f7c2da68154fa2cd722a4ef68c081349 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Tue, 6 Oct 2026 16:50:41 +0200 Subject: [PATCH 1/7] fix(host-cache): hash every part of the project.so stamp The stamp that decides whether a compiled project file can be reused was meant to cover the crafter-build version, the toolchain key, the module-source stamp and the compile command. Only the version went in. The four parts were joined by a format string with "\0" between the fields, and a format string is read as a C string: it ended at the first NUL, so std::format saw "{}" and dropped the other arguments without a diagnostic. A project.so is loaded into the crafter-build process and fills in Configuration objects by their layout. With the stamp blind to the declarations, one built against other module sources was loaded as current whenever the version string was unchanged, which is every working-tree build (`git describe --dirty` does not follow the tree's contents) and every local Flatpak build. Growing a class that project files construct then corrupts the heap: a project.so built while Shader was 56 bytes, loaded after it had become 104, aborted with "free(): invalid size" as soon as it declared a shader. CI builds were spared only because each carries its own version. The parts are now appended to a std::string with NUL separators, the way the other stamps in this file are built. Every existing project.so carries a stamp of the version alone, so each is recompiled once. Co-Authored-By: Claude Fable 5.1 --- implementations/Crafter.Build-Platform.cpp | 13 ++++++++++++- 1 file changed, 12 insertions(+), 1 deletion(-) diff --git a/implementations/Crafter.Build-Platform.cpp b/implementations/Crafter.Build-Platform.cpp index f8a7694..03cdba2 100644 --- a/implementations/Crafter.Build-Platform.cpp +++ b/implementations/Crafter.Build-Platform.cpp @@ -845,7 +845,18 @@ Configuration Crafter::LoadProject(const fs::path& projectFile, std::span Date: Tue, 6 Oct 2026 16:50:41 +0200 Subject: [PATCH 2/7] test(host-cache): changed declarations recompile project.so HostCacheSourceStamp already edits a module source with a backdated mtime and expects the PCM cache to be rebuilt. Its own header names the real danger, a project.so compiled against declarations that no longer match the process it is loaded into, but it never looked at the project.so. It now records the file's mtime before the edit and expects a newer one after it, with project.cpp untouched, so only the stamp can force the rebuild. Fails without the previous commit: the stamp was the version string alone and the project.so was reused. Co-Authored-By: Claude Fable 5.1 --- tests/HostCacheSourceStamp/main.cpp | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/tests/HostCacheSourceStamp/main.cpp b/tests/HostCacheSourceStamp/main.cpp index b6904e8..4110c82 100644 --- a/tests/HostCacheSourceStamp/main.cpp +++ b/tests/HostCacheSourceStamp/main.cpp @@ -119,6 +119,17 @@ int main() { Check(fs::last_write_time(probePcm) == afterCold, "unchanged sources do not rebuild the cache"); Check(ReadStamp(moduleCacheDir) == firstStamp, "unchanged sources keep the same stamp"); + // The compiled project file is the other half of the same hazard: it is + // loaded into this process and must have been compiled against the + // declarations this process was built from. + fs::path projectSo; + for (const fs::directory_entry& entry : fs::recursive_directory_iterator(projectDir)) { + if (entry.path().filename() == "project.so") projectSo = entry.path(); + } + Check(!projectSo.empty(), "the project file was compiled to a project.so"); + if (projectSo.empty()) return 1; + const fs::file_time_type soBeforeEdit = fs::last_write_time(projectSo); + // 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. @@ -139,6 +150,11 @@ int main() { 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"); + // project.cpp itself is untouched, so only the stamp can force this. It + // once covered the version string alone (the rest sat behind a NUL in a + // format string): a project.so built against a class of another size was + // reused and corrupted the heap when it filled one in. + Check(fs::last_write_time(projectSo) > soBeforeEdit, "changed declarations recompile project.so even though project.cpp is unchanged"); fs::remove_all(scratch, ec); From ddaab5a32a3bd3e16687d66221e1b9b4bbfd6134 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Tue, 6 Oct 2026 16:50:41 +0200 Subject: [PATCH 3/7] feat(shader): per-entry defines and a named output variant Crafter.Graphics has to ship each of its compute shaders in two builds of one source: one that indexes VK_EXT_descriptor_heap and one that reads the same slots from a descriptor set, for drivers without the extension. A Shader was a path, an entry point and a stage, and its output name was derived from the path alone, so one source could only be compiled once. Shader gains `defines`, spelled like the compiler's -D ("NAME" is defined to 1, "NAME=VALUE" to VALUE) and handed to glslang as a preamble, and `variant`, which names the output: "ui.comp.glsl" with variant "sets" becomes "ui.comp.sets.spv". OutputName() is the one place that name is formed; Check, Compile and the forwarding of a dependency's shaders into the consumer's bin dir all use it. Two fields make one mistake easy: defines set, variant forgotten. Both entries then compiled in parallel into the same file, the build reported success with whichever finished last, and Check() called both up to date from then on. The same has always been true of a.vert next to a.frag. Build() now refuses a configuration in which two entries share an output name and says which. A define without a name, or one spanning lines, and a variant with a path separator are rejected by name. Left to glslang the first two surface as "-1:" with no file, and the last as a failed open after the whole compile. Not addressed: Check() still compares only the source's mtime, so a change to the defines, or to an included file, does not recompile (#35). Co-Authored-By: Claude Fable 5.1 --- implementations/Crafter.Build-Clang.cpp | 22 +++++++++++-- implementations/Crafter.Build-Shader.cpp | 41 ++++++++++++++++++++++-- interfaces/Crafter.Build-Shader.cppm | 18 +++++++++++ 3 files changed, 77 insertions(+), 4 deletions(-) diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp index d2aee22..05fa4b8 100644 --- a/implementations/Crafter.Build-Clang.cpp +++ b/implementations/Crafter.Build-Clang.cpp @@ -502,10 +502,28 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_map buildCancelled{false}; + // Two entries with the same output name would compile in parallel into + // one file: whichever finished last won, the build reported success, and + // Check() then called both up to date. The usual way in is one source + // registered twice with different defines and no variant; a.vert next to + // a.frag is another. + { + std::unordered_map outputs; + for (const Shader& shader : config.shaders) { + const auto [existing, inserted] = outputs.emplace(shader.OutputName().string(), &shader); + if (inserted) continue; + buildError = std::format("shaders {} and {} both compile to {}: give one of them a distinct Shader::variant", existing->second->path.string(), shader.path.string(), existing->first); + buildCancelled.store(true); + break; + } + } for (const Shader& shader : config.shaders) { + if (buildCancelled.load(std::memory_order_relaxed)) break; if (shader.Check(outputDir)) continue; threads.emplace_back([&shader, &outputDir, &shaderIncludeDirs, &buildError, &buildCancelled]() { - Progress::Task task(std::format("Compiling shader {}", shader.path.filename().string())); + // A source registered more than once shows up once per variant; + // name the variant so the lines can be told apart. + Progress::Task task(shader.variant.empty() ? std::format("Compiling shader {}", shader.path.filename().string()) : std::format("Compiling shader {} ({})", shader.path.filename().string(), shader.variant)); if (buildCancelled.load(std::memory_order_relaxed)) return; std::string result = shader.Compile(outputDir, shaderIncludeDirs); @@ -878,7 +896,7 @@ BuildResult Crafter::Build(Configuration& config, std::unordered_mapBinDir(); for (const Shader& shader : dep->shaders) { - fs::path src = depBinDir / shader.path.filename().replace_extension("spv"); + fs::path src = depBinDir / shader.OutputName(); if (!fs::exists(src)) continue; copyTree(src, outputDir / src.filename()); } diff --git a/implementations/Crafter.Build-Shader.cpp b/implementations/Crafter.Build-Shader.cpp index e251c50..722e635 100644 --- a/implementations/Crafter.Build-Shader.cpp +++ b/implementations/Crafter.Build-Shader.cpp @@ -36,9 +36,17 @@ namespace { namespace Crafter { Shader::Shader(fs::path&& path, std::string&& entrypoint, ShaderType type) : path(std::move(path)), entrypoint(std::move(entrypoint)), type(type) { + } + fs::path Shader::OutputName() const { + // replace_extension swaps only the last extension, so a stage suffix + // survives: "ui.comp.glsl" becomes "ui.comp.spv", or "ui.comp.sets.spv" + // for the variant "sets". + fs::path name = path.filename(); + name.replace_extension(variant.empty() ? std::string("spv") : std::format("{}.spv", variant)); + return name; } bool Shader::Check(const fs::path& outputDir) const { - fs::path spv = outputDir / path.filename().replace_extension("spv"); + fs::path spv = outputDir / OutputName(); return fs::exists(spv) && fs::last_write_time(path) < fs::last_write_time(spv); } std::string Shader::Compile(const fs::path& outputDir, std::span includeDirs) const { @@ -52,6 +60,9 @@ namespace Crafter { auto fail = [&](std::string_view stage, std::string log) { glslang::FinalizeProcess(); std::string out = path.string(); + // Several variants of one source compile in parallel; without the + // variant the message would not say which of them failed. + if (!variant.empty()) out += std::format(" ({})", variant); out += ": "; out += stage; if (!log.empty()) { @@ -75,8 +86,34 @@ namespace Crafter { const char* shaderSource = src.data(); const std::int32_t shaderSourceLen = static_cast(src.size()); + // A variant is one component of a file name. Caught here, because a + // separator would otherwise surface as "failed to open SPIR-V output" + // after the whole compile. + if (variant.find_first_of("/\\") != std::string::npos) { + return fail(std::format("shader variant '{}' must not contain a path separator", variant), {}); + } + + // One #define line per entry, split like the compiler's -D: "NAME" + // defines it to 1, "NAME=VALUE" to VALUE. glslang places the preamble + // ahead of the whole source (it finds #version by scanning first), so + // line numbers in the user's file are unaffected. It keeps the + // pointer it is handed instead of copying, so the string is declared + // here to outlive parse() below. + std::string preamble; + for (const std::string& define : defines) { + const std::size_t eq = define.find('='); + const std::string name = define.substr(0, eq); + // An error inside the preamble is reported as "-1:" with no + // file and no macro name, so reject the malformed ones by name. + if (name.empty() || define.find_first_of("\r\n") != std::string::npos) { + return fail(std::format("invalid shader define '{}': expected NAME or NAME=VALUE on one line", define), {}); + } + preamble += std::format("#define {} {}\n", name, eq == std::string::npos ? std::string("1") : define.substr(eq + 1)); + } + glslang::TShader shader(glslangType); shader.setStringsWithLengthsAndNames(&shaderSource, &shaderSourceLen, fileNameList, 1); + if (!preamble.empty()) shader.setPreamble(preamble.c_str()); shader.setEntryPoint(entrypoint.c_str()); shader.setSourceEntryPoint(entrypoint.c_str()); shader.setEnvTarget(glslang::EShTargetSpv, glslang::EShTargetSpv_1_4); @@ -110,7 +147,7 @@ namespace Crafter { glslang::GlslangToSpv(*intermediate, spirv, &logger); std::string spvLog = logger.getAllMessages(); - fs::path filename = path.filename().replace_extension("spv"); + fs::path filename = OutputName(); std::ofstream file(outputDir/filename, std::ios::binary); if (!file) { return fail("failed to open SPIR-V output", (outputDir/filename).string()); diff --git a/interfaces/Crafter.Build-Shader.cppm b/interfaces/Crafter.Build-Shader.cppm index 4b4d841..b059cfb 100644 --- a/interfaces/Crafter.Build-Shader.cppm +++ b/interfaces/Crafter.Build-Shader.cppm @@ -28,7 +28,25 @@ namespace Crafter { fs::path path; std::string entrypoint; ShaderType type; + // Preprocessor macros for this compile, spelled like the compiler's -D: + // "NAME" (defined to 1) or "NAME=VALUE". They are placed ahead of the + // source, so one file can be registered more than once and take a + // different #ifdef branch each time. Note the spelling differs from + // Configuration::defines, which is a list of {name, value} pairs. + std::vector defines; + // Names the output when one source is compiled more than once: empty + // gives ".spv", "sets" gives ".sets.spv". One file-name + // component: no path separators. Every entry of a configuration must + // end up with its own OutputName(); Build() fails if two collide, so + // each extra compile of a source needs its own variant. Check() + // compares only the source's mtime: changing defines without touching + // the source leaves the old output in place. + std::string variant; Shader(fs::path&& path, std::string&& entrypoint, ShaderType type); + // File name of the compiled SPIR-V, relative to the output directory. + // The single source of truth for it: Check, Compile and the artifact + // forwarding in Build() all go through here. + fs::path OutputName() const; bool Check(const fs::path& outputDir) const; std::string Compile(const fs::path& outputDir, std::span includeDirs) const; }; From 51d87a990f24ef02f423cafb89edd4a66bacc495 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Tue, 6 Oct 2026 16:50:41 +0200 Subject: [PATCH 4/7] test(shader): variants through the Shader API and through Build() ShaderCompile registers one source twice and checks that the variant gets its own output name, takes the other #ifdef branch and receives the value of a NAME=VALUE define. Staleness is pinned per output: while only the plain output exists the variant must still be due, or Build() would never compile a variant added to a built project. Malformed defines and a variant with a path separator must be refused by name. ShaderVariantBuild drives the same through Build(), which is where the entries compile in parallel into one directory: a variant added to an already built configuration is compiled into its own file, and two entries with one output name fail the build without touching the existing output. Its fixture tests the macro with `#if`, which pins that a bare define is 1 and not empty. Co-Authored-By: Claude Fable 5.1 --- project.cpp | 1 + tests/ShaderCompile/fixture/variant.frag | 15 +++ tests/ShaderCompile/main.cpp | 75 ++++++++++++- tests/ShaderVariantBuild/fixture/main.cpp | 6 + tests/ShaderVariantBuild/fixture/tint.frag | 12 ++ tests/ShaderVariantBuild/main.cpp | 121 +++++++++++++++++++++ 6 files changed, 229 insertions(+), 1 deletion(-) create mode 100644 tests/ShaderCompile/fixture/variant.frag create mode 100644 tests/ShaderVariantBuild/fixture/main.cpp create mode 100644 tests/ShaderVariantBuild/fixture/tint.frag create mode 100644 tests/ShaderVariantBuild/main.cpp diff --git a/project.cpp b/project.cpp index a7c3272..0395bbf 100644 --- a/project.cpp +++ b/project.cpp @@ -125,6 +125,7 @@ extern "C" Configuration CrafterBuildProject(std::span a cfg.AddTest("CompilerWarning").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("CleanProject").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("ShaderCompile").Dependencies({ CrafterBuildLib.get() }); + cfg.AddTest("ShaderVariantBuild").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("StandardArgs").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("TestRunnerSpec").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("VariantId").Dependencies({ CrafterBuildLib.get() }); diff --git a/tests/ShaderCompile/fixture/variant.frag b/tests/ShaderCompile/fixture/variant.frag new file mode 100644 index 0000000..7d0178c --- /dev/null +++ b/tests/ShaderCompile/fixture/variant.frag @@ -0,0 +1,15 @@ +#version 450 + +// Compiled twice by the ShaderCompile test: once as is, once with VARIANT_RED +// and VARIANT_ALPHA=0.5 defined. The red branch does not compile unless the +// NAME=VALUE define arrives, and the two outputs only differ if the bare one +// does. +layout(location = 0) out vec4 outColor; + +void main() { +#ifdef VARIANT_RED + outColor = vec4(1.0, 0.0, 0.0, VARIANT_ALPHA); +#else + outColor = vec4(0.0, 1.0, 0.0, 1.0); +#endif +} diff --git a/tests/ShaderCompile/main.cpp b/tests/ShaderCompile/main.cpp index 20221d2..1ead9ca 100644 --- a/tests/ShaderCompile/main.cpp +++ b/tests/ShaderCompile/main.cpp @@ -8,7 +8,10 @@ using namespace Crafter; // Compile a tiny GLSL fragment shader directly via the Shader API. Verifies // the SPIR-V output lands in the target dir and starts with the SPIR-V magic -// number, without dragging in the full Build() pipeline. +// number, without dragging in the full Build() pipeline. The second half +// registers one source twice, with and without defines, and checks that the +// variant gets its own output, its own staleness and the other #ifdef branch +// (tests/ShaderVariantBuild drives the same thing through Build()). int main() { fs::path src = fs::current_path() / "tests" / "ShaderCompile" / "fixture" / "triangle.frag"; if (!fs::exists(src)) { @@ -52,5 +55,75 @@ int main() { std::println(std::cerr, "Shader::Check returned false on an up-to-date spv"); return 1; } + + // One source registered twice: the second entry carries defines and a + // variant name. It must land in its own file instead of overwriting the + // first, and it must take the other #ifdef branch. Crafter.Graphics ships + // each UI shader in a descriptor-heap and a descriptor-set flavour this + // way; if the variant were dropped from the output name, whichever compile + // finished last would silently win. + fs::path variantSrc = fs::current_path() / "tests" / "ShaderCompile" / "fixture" / "variant.frag"; + Shader plain(fs::path(variantSrc), "main", ShaderType::Fragment); + Shader red(fs::path(variantSrc), "main", ShaderType::Fragment); + red.defines = {"VARIANT_RED", "VARIANT_ALPHA=0.5"}; + red.variant = "red"; + + if (plain.OutputName() != "variant.spv" || red.OutputName() != "variant.red.spv") { + std::println(std::cerr, "unexpected output names: {} and {}", plain.OutputName().string(), red.OutputName().string()); + return 1; + } + for (const Shader* each : {&plain, &red}) { + // Staleness is per output, not per source: while only variant.spv + // exists, the variant is still due. A Check() that ignored the + // variant would call it up to date here, and Build() would then + // never compile a variant added to an already built project. + if (each == &red && red.Check(outDir)) { + std::println(std::cerr, "Shader::Check called the variant up to date before it was ever compiled"); + return 1; + } + std::string variantErr = each->Compile(outDir, includeDirs); + if (!variantErr.empty()) { + std::println(std::cerr, "variant shader compile failed: {}", variantErr); + return 1; + } + if (!each->Check(outDir)) { + std::println(std::cerr, "Shader::Check returned false for {}", each->OutputName().string()); + return 1; + } + } + + // Malformed defines and variants are refused by name. Left to glslang, an + // empty macro name surfaces as "-1:" with no file, and a variant + // with a separator as a failed open after the whole compile. + for (const char* bad : {"", "=1", "A\nB"}) { + Shader malformed(fs::path(variantSrc), "main", ShaderType::Fragment); + malformed.defines = {bad}; + malformed.variant = "malformed"; + if (!malformed.Compile(outDir, includeDirs).contains("invalid shader define")) { + std::println(std::cerr, "a malformed define was not rejected by name"); + return 1; + } + } + Shader nested(fs::path(variantSrc), "main", ShaderType::Fragment); + nested.variant = "sub/red"; + if (!nested.Compile(outDir, includeDirs).contains("path separator")) { + std::println(std::cerr, "a variant with a path separator was not rejected by name"); + return 1; + } + + auto readAll = [](const fs::path& file) { + std::ifstream stream(file, std::ios::binary); + return std::string(std::istreambuf_iterator(stream), std::istreambuf_iterator()); + }; + std::string plainSpv = readAll(outDir / "variant.spv"); + std::string redSpv = readAll(outDir / "variant.red.spv"); + if (plainSpv.empty() || redSpv.empty()) { + std::println(std::cerr, "a variant output is missing or empty"); + return 1; + } + if (plainSpv == redSpv) { + std::println(std::cerr, "both variants compiled to the same SPIR-V: the defines did not reach the preprocessor"); + return 1; + } return 0; } diff --git a/tests/ShaderVariantBuild/fixture/main.cpp b/tests/ShaderVariantBuild/fixture/main.cpp new file mode 100644 index 0000000..97eb9af --- /dev/null +++ b/tests/ShaderVariantBuild/fixture/main.cpp @@ -0,0 +1,6 @@ +// SPDX-License-Identifier: LGPL-3.0-only +// SPDX-FileCopyrightText: Copyright (C) 2026 Catcrafts® + +int main() { + return 0; +} diff --git a/tests/ShaderVariantBuild/fixture/tint.frag b/tests/ShaderVariantBuild/fixture/tint.frag new file mode 100644 index 0000000..c227813 --- /dev/null +++ b/tests/ShaderVariantBuild/fixture/tint.frag @@ -0,0 +1,12 @@ +#version 450 + +// Built twice by the ShaderVariantBuild test: as is, and with TINT_RED defined. +layout(location = 0) out vec4 outColor; + +void main() { +#if TINT_RED + outColor = vec4(1.0, 0.0, 0.0, 1.0); +#else + outColor = vec4(0.0, 1.0, 0.0, 1.0); +#endif +} diff --git a/tests/ShaderVariantBuild/main.cpp b/tests/ShaderVariantBuild/main.cpp new file mode 100644 index 0000000..517b99c --- /dev/null +++ b/tests/ShaderVariantBuild/main.cpp @@ -0,0 +1,121 @@ +// 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; + +// One shader source can be registered more than once, each entry with its own +// defines and a variant that names its output ("tint.spv", "tint.red.spv"). +// Build() compiles the entries in parallel into one directory, which is where +// the feature can go wrong in ways the Shader API alone does not show: +// +// - both entries must land in the bin dir, with different contents; +// - an entry added later must still be compiled when the other one's output +// is already there and up to date; +// - two entries that would write the same file (defines set, variant +// forgotten) used to race silently: the file held whichever compile +// finished last, the build reported success, and Check() called both up +// to date from then on. That has to be a build error naming the cause. +// +// The fixture's `#if TINT_RED` also pins that a bare define is defined to 1, +// like the compiler's -D: an empty definition would not compile. +namespace { + std::int32_t Failures = 0; + + void Check(bool cond, std::string_view msg) { + if (!cond) { + std::println(std::cerr, "FAIL: {}", msg); + ++Failures; + } + } + + // Shader outputs are compared by mtime against the source, and the test + // adds an entry between passes, so work on a fresh copy outside the repo. + fs::path StageFixture() { + fs::path source = fs::current_path() / "tests" / "ShaderVariantBuild" / "fixture"; + fs::path staged = fs::temp_directory_path() / "crafter-build-shader-variant-build"; + fs::remove_all(staged); + fs::copy(source, staged, fs::copy_options::recursive); + return staged; + } + + Configuration MakeApp(const fs::path& staged) { + Configuration app; + app.path = staged; + app.name = "tinted"; + app.outputName = "tinted"; + app.target = HostTarget(); + app.type = ConfigurationType::Executable; + std::array ifaces = {}; + std::array impls = { "main" }; + app.GetInterfacesAndImplementations(ifaces, impls); + return app; + } + + Shader& AddTint(Configuration& app, const fs::path& staged) { + return app.shaders.emplace_back(staged / "tint.frag", std::string("main"), ShaderType::Fragment); + } + + BuildResult BuildOnce(Configuration& app) { + // A fresh depResults per pass: the map memoizes a Configuration's + // build for the duration of one pass. + std::unordered_map> depResults; + std::mutex depMutex; + return Build(app, depResults, depMutex); + } + + std::string ReadAll(const fs::path& file) { + std::ifstream stream(file, std::ios::binary); + return std::string(std::istreambuf_iterator(stream), std::istreambuf_iterator()); + } +} + +int main() { + const fs::path staged = StageFixture(); + + // Pass 1: the plain entry alone. + Configuration app = MakeApp(staged); + AddTint(app, staged); + BuildResult first = BuildOnce(app); + Check(first.result.empty(), std::format("the plain entry builds: {}", first.result)); + const fs::path plainSpv = app.BinDir() / "tint.spv"; + const fs::path redSpv = app.BinDir() / "tint.red.spv"; + Check(fs::exists(plainSpv), "tint.spv is in the bin dir"); + Check(!fs::exists(redSpv), "no variant output before the variant is registered"); + + // Pass 2: the variant is added while tint.spv is already up to date. It + // has its own staleness check and must be compiled, into its own file. + Shader& red = AddTint(app, staged); + red.defines = {"TINT_RED"}; + red.variant = "red"; + BuildResult second = BuildOnce(app); + Check(second.result.empty(), std::format("plain + variant builds: {}", second.result)); + Check(fs::exists(redSpv), "tint.red.spv is in the bin dir"); + const std::string plainBytes = ReadAll(plainSpv); + const std::string redBytes = ReadAll(redSpv); + Check(!plainBytes.empty() && !redBytes.empty(), "both outputs have content"); + Check(plainBytes != redBytes, "the variant took the other #if branch"); + + // Pass 3: the same define without a variant. Both entries would write + // tint.spv, so the build has to stop and say so, and must leave the file + // from pass 2 alone. + Configuration clash = MakeApp(staged); + AddTint(clash, staged); + Shader& unnamed = AddTint(clash, staged); + unnamed.defines = {"TINT_RED"}; + BuildResult third = BuildOnce(clash); + Check(!third.result.empty(), "two entries with one output name fail the build"); + Check(third.result.contains("tint.spv") && third.result.contains("variant"), std::format("the error names the file and the fix: {}", third.result)); + Check(ReadAll(plainSpv) == plainBytes, "the existing output is not overwritten by the failed build"); + + std::error_code ec; + fs::remove_all(staged, ec); + + if (Failures > 0) { + std::println(std::cerr, "{} assertions failed", Failures); + return 1; + } + return 0; +} From 3c96d78a7d7a2efc2c72899d9f0115dc90b522d1 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Tue, 6 Oct 2026 16:50:41 +0200 Subject: [PATCH 5/7] fix(sysroot): compile against the sysroot's libc++ headers clang's driver ranks the host toolchain's own libc++ headers (/../include/c++/v1) above --sysroot. BuildStdPcm has forced the sysroot's copy for the std module for that reason, but ordinary compiles did not. A unit that only does `import std;` never noticed. A unit that includes a standard header textually got the host's libc++, configured for the host's libc: cross-building Crafter.Graphics for aarch64 musl, Crafter.Asset's vendored gdeflate died on with "unknown rune table for this platform". Against a glibc sysroot the same unit would compile against the wrong libc++ release and say nothing. Every non-wasm, non-mingw compile with a sysroot now gets `-nostdinc++ -isystem /usr/include/c++/v1`. wasm has its own sysroot handling and mingw is a libstdc++ target. -nostdinc++ leaves -stdlib=libc++ unused on each compile, which clang reports once per unit, so the command also carries -Wno-unused-command-line-argument. The cost is that the driver's unused-argument diagnostics are silent on sysroot builds, including for a user's own misplaced flag (a -l or -Wl, option in compileFlags). Not addressed: external cmake dependencies are still configured without the target or the sysroot, and objects already built in an existing sysroot build directory are not invalidated by the new flags. Co-Authored-By: Claude Fable 5.1 --- implementations/Crafter.Build-Clang.cpp | 15 +++++++++++++++ 1 file changed, 15 insertions(+) diff --git a/implementations/Crafter.Build-Clang.cpp b/implementations/Crafter.Build-Clang.cpp index 05fa4b8..d10b158 100644 --- a/implementations/Crafter.Build-Clang.cpp +++ b/implementations/Crafter.Build-Clang.cpp @@ -296,6 +296,21 @@ CompileCommand Crafter::GetCompileCommand(const Configuration& config) { if (!config.sysroot.empty()) { out.command += std::format(" --sysroot={}", config.sysroot); + // Same reason as in BuildStdPcm: the driver ranks the host + // toolchain's own libc++ headers (/../include/c++/v1) above + // --sysroot. A unit that only does `import std;` never notices, but + // one that includes a standard header textually (vendored C++ such as + // Crafter.Asset's gdeflate) would get host libc++ configured for the + // host libc, which fails on a musl sysroot with "unknown rune table + // for this platform". Force the sysroot's copy. -nostdinc++ leaves + // -stdlib=libc++ unused on every compile ("argument unused during + // compilation"), hence the silencer; the price is that it also hides + // that diagnostic for a user's own misplaced flag (a -l or -Wl, + // option in compileFlags) on sysroot builds. Not for mingw, which + // uses libstdc++ and has no libc++ in its sysroot. + if (!isWasm && config.target != "x86_64-w64-mingw32") { + out.command += std::format(" -nostdinc++ -isystem {}/usr/include/c++/v1 -Wno-unused-command-line-argument", config.sysroot); + } } if (isWasm) { // -mllvm is consumed by codegen but not the link driver, which is the From 4ca8f14c70ec727046cec963f208be6bd08b6297 Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Tue, 6 Oct 2026 16:50:41 +0200 Subject: [PATCH 6/7] test(sysroot): the compile command forces the sysroot's libc++ GetCompileCommand is checked as text, without a compiler: the forced include directory is present for linux-gnu and linux-musl targets with a sysroot, and absent without a sysroot, for wasm32 and for mingw. Co-Authored-By: Claude Fable 5.1 --- project.cpp | 1 + tests/SysrootCompileFlags/main.cpp | 65 ++++++++++++++++++++++++++++++ 2 files changed, 66 insertions(+) create mode 100644 tests/SysrootCompileFlags/main.cpp diff --git a/project.cpp b/project.cpp index 0395bbf..43584c8 100644 --- a/project.cpp +++ b/project.cpp @@ -131,6 +131,7 @@ extern "C" Configuration CrafterBuildProject(std::span a cfg.AddTest("VariantId").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("ToolchainResolve").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("SysrootStdModule").Dependencies({ CrafterBuildLib.get() }); + cfg.AddTest("SysrootCompileFlags").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("VersionFlag").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("WasiBrowserRuntime").Dependencies({ CrafterBuildLib.get() }); cfg.AddTest("WasmVariants").Dependencies({ CrafterBuildLib.get() }); diff --git a/tests/SysrootCompileFlags/main.cpp b/tests/SysrootCompileFlags/main.cpp new file mode 100644 index 0000000..bfaf95b --- /dev/null +++ b/tests/SysrootCompileFlags/main.cpp @@ -0,0 +1,65 @@ +// 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; + +// clang's driver ranks the host toolchain's own libc++ headers +// (/../include/c++/v1) above --sysroot. A unit that only does +// `import std;` never notices, because the std module is precompiled with the +// sysroot's headers forced. A unit that includes a standard header textually +// (vendored C++ in a dependency, say) got the host's libc++ configured for the +// host's libc: against a musl sysroot every such unit died with "unknown rune +// table for this platform", and against a glibc sysroot it compiled against +// the wrong release without a word. +// +// GetCompileCommand therefore has to point every sysroot compile at the +// sysroot's copy. It must not do so where there is no sysroot, for wasm (which +// has its own sysroot handling) or for mingw (libstdc++, no libc++ at all). +// The command is checked as text: no compiler runs. +namespace { + std::int32_t Failures = 0; + + void Check(bool cond, std::string_view msg) { + if (!cond) { + std::println(std::cerr, "FAIL: {}", msg); + ++Failures; + } + } + + std::string CommandFor(std::string_view target, std::string_view sysroot) { + Configuration cfg; + cfg.path = "/tmp/crafter-build-sysroot-compile-flags"; + cfg.name = "flags"; + cfg.outputName = "flags"; + cfg.target = std::string(target); + cfg.march = "generic"; + cfg.mtune = "generic"; + cfg.type = ConfigurationType::Executable; + cfg.sysroot = std::string(sysroot); + return GetCompileCommand(cfg).command; + } +} + +int main() { + const std::string sysroot = "/opt/crafter-build-test-sysroot"; + const std::string forced = std::format("-nostdinc++ -isystem {}/usr/include/c++/v1", sysroot); + + for (std::string_view target : {"aarch64-alpine-linux-musl", "aarch64-unknown-linux-gnu", "x86_64-pc-linux-gnu"}) { + const std::string command = CommandFor(target, sysroot); + Check(command.contains(std::format("--sysroot={}", sysroot)), std::format("{}: the sysroot is passed", target)); + Check(command.contains(forced), std::format("{}: the sysroot's libc++ headers are forced", target)); + } + + Check(!CommandFor("x86_64-pc-linux-gnu", "").contains("/usr/include/c++/v1 -Wno-unused-command-line-argument"), "no sysroot: nothing is forced"); + Check(!CommandFor("wasm32-wasip1", sysroot).contains(forced), "wasm: left to its own sysroot handling"); + Check(!CommandFor("x86_64-w64-mingw32", sysroot).contains(forced), "mingw: libstdc++ target, libc++ headers are not forced"); + + if (Failures > 0) { + std::println(std::cerr, "{} assertions failed", Failures); + return 1; + } + return 0; +} From fe38cc1f6618a3f85c82083cd41fe812b5ab518c Mon Sep 17 00:00:00 2001 From: Jorijn van der Graaf Date: Tue, 6 Oct 2026 16:50:41 +0200 Subject: [PATCH 7/7] docs: sysroot compiles use the sysroot's libc++ headers Co-Authored-By: Claude Fable 5.1 --- README.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/README.md b/README.md index 590251b..921a814 100644 --- a/README.md +++ b/README.md @@ -145,7 +145,7 @@ TestRunner::Cmd("wine") // local exec via prefix co For elaborate setups (rsync, ssh ControlMaster, env vars on the spawn, `QEMU_LD_PREFIX` for sysroots), construct `TestRunner` directly — `copy`/`exec`/`cleanup`/`probe` are public string fields with `{bundle}`, `{remote_bundle}`, `{bin}`, `{bin_name}`, `{args}` placeholders. -For cross-arch builds, set `cfg.sysroot` on the `Configuration` to point at a sysroot for the target. `BuildStdPcm` and the compile/link commands both consume it: `--sysroot=` is passed to clang, and the std module is precompiled against the sysroot's libc++ headers. The module source has to come from the same libc++ release as those headers, so it is taken from `/usr/share/libc++/v1/std.cppm` when the sysroot ships it, else from the toolchain when both are the same major release, else from the sources crafter-build carries for other releases (`share/crafter-build/libc++-modules//`, `crafter-build --version` lists them). Alpine is the common case for the last two: its `libc++-dev` has the headers and no module source. When nothing matches, the build stops with one line naming both releases, installing that release's `share/libc++/v1` directory into the sysroot resolves it. The cheapest aarch64 sysroot is an extracted [Arch Linux ARM rootfs](http://os.archlinuxarm.org/) plus `libc++` + `libc++abi` + `gcc` installed into it via `pacman --root --config --dbpath /var/lib/pacman -Sy …` (point pacman at ALARM's mirrors via explicit config to avoid pulling host-arch packages). +For cross-arch builds, set `cfg.sysroot` on the `Configuration` to point at a sysroot for the target. `BuildStdPcm` and the compile/link commands both consume it: `--sysroot=` is passed to clang, and the std module is precompiled against the sysroot's libc++ headers. Ordinary compiles are pointed at the same headers (`-nostdinc++ -isystem /usr/include/c++/v1`), so a unit that `#include`s a standard header gets the sysroot's libc++ and not the host toolchain's. The module source has to come from the same libc++ release as those headers, so it is taken from `/usr/share/libc++/v1/std.cppm` when the sysroot ships it, else from the toolchain when both are the same major release, else from the sources crafter-build carries for other releases (`share/crafter-build/libc++-modules//`, `crafter-build --version` lists them). Alpine is the common case for the last two: its `libc++-dev` has the headers and no module source. When nothing matches, the build stops with one line naming both releases, installing that release's `share/libc++/v1` directory into the sysroot resolves it. The cheapest aarch64 sysroot is an extracted [Arch Linux ARM rootfs](http://os.archlinuxarm.org/) plus `libc++` + `libc++abi` + `gcc` installed into it via `pacman --root --config --dbpath /var/lib/pacman -Sy …` (point pacman at ALARM's mirrors via explicit config to avoid pulling host-arch packages). For `--target=x86_64-w64-mingw32` (Windows from Linux), the pipeline cross-compiles as long as `mingw-w64` is installed. Pair it with `--runner=cmd:wine` to run the result. Only the MinGW ABI is supported, there is no MSVC target and no Windows host. `-fuse-ld=lld` is auto-injected on every link, and `-lstdc++exp` is auto-linked for the mingw target so user projects don't need to declare those flags.