From d178403ea03d7139c9785c125dfce738334a8893 Mon Sep 17 00:00:00 2001 From: sunrisepeak Date: Wed, 9 Sep 2026 00:32:41 +0800 Subject: [PATCH] 0.5.1: roots that overlap are refused, and a file root globs nothing beside it The design record this implements states that overlapping roots are refused, and 0.5.0 shipped without the check. A file reachable from two roots has two namespace paths, and which one it got depended on the order of the list; it was also read twice and merged with itself, so the entry looked like two implementations agreeing. A misconfiguration that produces a plausible result is worse than one that stops, and this is the shape of defect where a requirement folded into a larger change disappears when that change ships. The re-run glob is now registered only for a directory root. A single file is its own root and its parent directory is not part of it, so globbing that parent made an unrelated file beside it an input to the build program. A root that is the package root itself no longer produces a `./**/*` pattern. A CI step covers the refusal: the fixture is perturbed to name `src` beside `src/kernels`, and the build fails naming both roots. --- .github/workflows/ci.yml | 29 +++++++++++++++++++++++++ README.md | 12 ++++++----- mcpp.toml | 2 +- src/plugins.cppm | 2 +- tools/island.cppm | 46 ++++++++++++++++++++++++++++++++++------ 5 files changed, 78 insertions(+), 13 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 182b31e..983e6bb 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -501,6 +501,35 @@ jobs: rm -f collide.log echo "ok: refused, naming both entry points and the prefix" + # OVERLAPPING ROOTS ARE REFUSED. + # + # A file reachable from two roots has two namespace paths, and which one + # it got would depend on the order of the list. It would also be read + # twice and merged with itself, so the entry would look like two + # implementations agreeing -- a misconfiguration that produces a + # plausible result rather than stopping. + - name: overlapping roots are refused + working-directory: tests/island-interface + run: | + cp build.mcpp /tmp/build.mcpp.overlap.bak + sed -i 's|opt.roots = { root + "/src/kernels", root + "/src/cpu" };|opt.roots = { root + "/src/kernels", root + "/src/cpu", root + "/src" };|' \ + build.mcpp + grep -q 'root + "/src" };' build.mcpp || { + echo "FAIL: the fixture was not perturbed; this step would assert nothing" + cp /tmp/build.mcpp.overlap.bak build.mcpp; exit 1; } + rm -rf target + set +e + "$MCPP" build > overlap.log 2>&1 + rc=$? + set -e + cp /tmp/build.mcpp.overlap.bak build.mcpp + [ "$rc" != 0 ] || { echo "FAIL: overlapping roots were accepted" + tail -20 overlap.log; exit 1; } + grep -q 'overlap' overlap.log || { + echo "FAIL: refused, but not for this reason"; tail -20 overlap.log; exit 1; } + rm -f overlap.log + echo "ok: refused, naming both roots" + # A ROOT THAT YIELDS NOTHING IS AN ERROR, NOT AN EMPTY MODULE. # # A misspelled root or a marker that never arrived would otherwise produce diff --git a/README.md b/README.md index 474333b..0a2ba6c 100644 --- a/README.md +++ b/README.md @@ -6,7 +6,7 @@ imports each one from `build.mcpp` under the module name the member declares. ```toml [build-dependencies.mcpp] -plugins = { version = "0.5.0", features = ["rules-spirv"], host-module = true } +plugins = { version = "0.5.1", features = ["rules-spirv"], host-module = true } ``` `[build-dependencies]`, not `[dependencies]`. The two keys answer separate @@ -61,7 +61,7 @@ A project names the rule and nothing else: ```toml [build-dependencies.mcpp] -plugins = { version = "0.5.0", features = ["rules-cuda"], host-module = true } +plugins = { version = "0.5.1", features = ["rules-cuda"], host-module = true } ``` The payloads each rule drives are declared **here**, under the feature that @@ -142,8 +142,8 @@ nothing enforcing it, which is the fragility the engine fix removes. A file renamed for a reason nobody can see is a defect waiting for the rename that looks harmless. -0.5.0 does not move it. Naming an island's entry points is a change to what this -package generates, not to what it asks the engine for. +0.5.0 and 0.5.1 do not move it. Naming an island's entry points is a change to +what this package generates, not to what it asks the engine for. The previous shared floor was 2026.9.7.1, the release that reads `device_extensions` and `rule_module`, reports `[language] modules` and the @@ -379,7 +379,9 @@ src/backends/cuda/image/blur.cu myapp_blur -> myapp::kernels::image::myapp_ src/backends/cuda/saxpy.cu myapp_saxpy -> myapp::kernels::myapp_saxpy ``` -**A root is a tree, and one of them supplies the shape.** `options::roots` names +**A root is a tree, and one of them supplies the shape.** Overlapping roots are +refused: a file reachable from two of them has two namespace paths, and which +one it got would depend on the order of the list. `options::roots` names the directories implementations live under; `options::layout_root` names the one whose directory structure decides where entry points live, and defaults to the first. Every other root only has to define the names, so a fallback tree may be diff --git a/mcpp.toml b/mcpp.toml index 9005ef3..6e717a7 100644 --- a/mcpp.toml +++ b/mcpp.toml @@ -1,7 +1,7 @@ [package] name = "plugins" namespace = "mcpp" -version = "0.5.0" +version = "0.5.1" description = "Official mcpp build plugins: rule packages under mcpp.rules.*, build-time utilities under mcpp.tools.*, each member selected by a feature" license = "Apache-2.0" authors = ["mcpp-community"] diff --git a/src/plugins.cppm b/src/plugins.cppm index 5f96b9c..533eb6d 100644 --- a/src/plugins.cppm +++ b/src/plugins.cppm @@ -49,7 +49,7 @@ export namespace mcpp::plugins { // // One package, one version: the number lives in mcpp.toml, and the CI step // `the collection states its own version` compares the two. -inline constexpr std::string_view version = "0.5.0"; +inline constexpr std::string_view version = "0.5.1"; } // namespace mcpp::plugins diff --git a/tools/island.cppm b/tools/island.cppm index bf4f2a2..7549df4 100644 --- a/tools/island.cppm +++ b/tools/island.cppm @@ -399,6 +399,31 @@ inline std::optional> scan(const options& opt) { } const auto exts = opt.extensions.empty() ? default_extensions() : opt.extensions; + // OVERLAPPING ROOTS ARE REFUSED. A file reachable from two of them has two + // namespace paths, and which one it got would depend on the order of the + // list. It would also be read twice and merged with itself, so the entry + // would look like two implementations agreeing -- a misconfiguration that + // produces a plausible result is worse than one that stops. + for (std::size_t i = 0; i < opt.roots.size(); ++i) { + std::error_code ec; + const auto a = std::filesystem::weakly_canonical(opt.roots[i], ec); + for (std::size_t j = i + 1; j < opt.roots.size(); ++j) { + const auto b = std::filesystem::weakly_canonical(opt.roots[j], ec); + const auto& outer = a.native().size() <= b.native().size() ? a : b; + const auto& inner = a.native().size() <= b.native().size() ? b : a; + const auto rel = inner.lexically_relative(outer); + const auto reltext = rel.generic_string(); + if (reltext.empty() || reltext.starts_with("..")) continue; + std::cerr << std::format( + "mcpp.tools.island: the roots `{}` and `{}` overlap.\n" + " A file reachable from both has two namespace paths, and which one " + "it got\n would depend on the order of this list. Roots are separate " + "implementation trees.\n", + outer.string(), inner.string()); + return std::nullopt; + } + } + struct record { entry e; std::size_t root = 0; @@ -481,12 +506,21 @@ inline std::optional> scan(const options& opt) { // is the sorted set of matching paths, which is exactly the question // "which files are here". The pattern is relative to the manifest // directory, so a root outside it registers its files and nothing else. - const auto rel = std::filesystem::path(base).lexically_relative( - std::filesystem::path(mcpp::manifest_dir())); - const auto reltext = rel.generic_string(); - if (!reltext.empty() && !reltext.starts_with("..")) { - for (auto const& e : exts) - mcpp::rerun_if_changed_glob((reltext + "/**/*" + e).c_str()); + // + // ONLY FOR A DIRECTORY ROOT. A single file is its own root, and its + // parent directory is not part of it: globbing that parent would make + // an unrelated file beside it an input to this program. + std::error_code dirEc; + if (std::filesystem::is_directory(std::filesystem::path(root), dirEc)) { + const auto rel = std::filesystem::path(base).lexically_relative( + std::filesystem::path(mcpp::manifest_dir())); + const auto reltext = rel.generic_string(); + if (!reltext.empty() && !reltext.starts_with("..")) { + const std::string prefix = reltext == "." ? std::string() + : reltext + "/"; + for (auto const& e : exts) + mcpp::rerun_if_changed_glob((prefix + "**/*" + e).c_str()); + } } }