0.5.1: roots that overlap are refused, and a file root globs nothing beside it (#14)

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.

Author: Sunrisepeak

.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

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

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"]

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
 

tools/island.cppm

@@ -399,6 +399,31 @@ inline std::optional<std::vector<entry>> 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<std::vector<entry>> 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());
+            }
         }
     }