disallowedRequisites: Show forbidden chain(s) to forbidden path

doc/manual/rl-next/report-forbidden-path-as-chain.md

@@ -0,0 +1,16 @@
+---
+synopsis: "`disallowedRequisites` now reports chains of disallowed requisites"
+# issues: ...
+prs: 10877
+---
+
+When a build fails because of [`disallowedRequisites`](@docroot@/language/advanced-attributes.md#adv-attr-disallowedRequisites), the error message now includes the chain of references that led to the failure. This makes it easier to see in which derivations the chain can be broken, to resolve the problem.
+
+Example:
+
+```
+$ nix build --no-link /nix/store/1fsjsd98awcf3sgny9zfq4wa7i5dci4n-hercules-ci-agent-0.10.3.drv^out
+error: output '/nix/store/xg01gga2qsi9v2qm45r2gqrkk46wvrkd-hercules-ci-agent-0.10.3' is not allowed to refer to the following paths.
+       Shown below are chains that lead to the forbidden path(s). More may exist.
+         { /nix/store/xg01gga2qsi9v2qm45r2gqrkk46wvrkd-hercules-ci-agent-0.10.3 -> /nix/store/mfzkjrwv8ckqqbsbj5yj24ri9dsgs30l-hercules-ci-cnix-expr-0.3.6.3 -> /nix/store/vxlw2igrncif77fh2qg1jg02bd455mbl-ghc-9.6.5 }
+```

src/libstore/unix/build/local-derivation-goal.cc

@@ -2807,35 +2807,61 @@ void LocalDerivationGoal::checkOutputs(const std::map<std::string, ValidPathInfo
             std::optional<Strings> allowedReferences, allowedRequisites, disallowedReferences, disallowedRequisites;
         };
 
+        /** A path, with a chain of paths back to how we found it. */
+        struct PathWithSource {
+
+            StorePath path;
+
+            /**
+             * A parent of sorts that leads back to the starting set of the transitive closure, if that's what we're working with.
+             * If `path` was part of the starting set, this will be `nullptr`.
+             */
+            std::shared_ptr<PathWithSource> source;
+
+            /** Just the `path`, or something like `source of source -> source -> path`. */
+            std::string showChain(Store & store) {
+                if (source)
+                    return fmt("%s -> %s", source->showChain(store), store.printStorePath(path));
+                else
+                    return store.printStorePath(path);
+            };
+        };
+
+        struct Closure {
+            /** Keys: paths in the closure, values: reverse path from an initial path to the parent of the key */
+            std::map<StorePath, std::shared_ptr<PathWithSource>> pathsWithSources;
+            uint64_t size;
+        };
+
         /* Compute the closure and closure size of some output. This
            is slightly tricky because some of its references (namely
            other outputs) may not be valid yet. */
         auto getClosure = [&](const StorePath & path)
         {
             uint64_t closureSize = 0;
-            StorePathSet pathsDone;
-            std::queue<StorePath> pathsLeft;
-            pathsLeft.push(path);
+            std::map<StorePath, std::shared_ptr<PathWithSource>> pathsDone;
+            std::queue<PathWithSource> pathsLeft;
+            pathsLeft.push({path, nullptr});
 
             while (!pathsLeft.empty()) {
                 auto path = pathsLeft.front();
                 pathsLeft.pop();
-                if (!pathsDone.insert(path).second) continue;
+                if (!pathsDone.insert({path.path, path.source}).second) continue;
 
-                auto i = outputsByPath.find(worker.store.printStorePath(path));
+                auto i = outputsByPath.find(worker.store.printStorePath(path.path));
                 if (i != outputsByPath.end()) {
                     closureSize += i->second.narSize;
                     for (auto & ref : i->second.references)
-                        pathsLeft.push(ref);
+                        pathsLeft.push({ .path = ref, .source = std::make_shared<PathWithSource>(path) });
                 } else {
-                    auto info = worker.store.queryPathInfo(path);
+                    auto info = worker.store.queryPathInfo(path.path);
                     closureSize += info->narSize;
                     for (auto & ref : info->references)
-                        pathsLeft.push(ref);
+                        pathsLeft.push({ .path = ref, .source = std::make_shared<PathWithSource>(path) });
                 }
             }
 
-            return std::make_pair(std::move(pathsDone), closureSize);
+            return Closure { std::move(pathsDone), closureSize };
         };
 
         auto applyChecks = [&](const Checks & checks)
@@ -2845,7 +2871,7 @@ void LocalDerivationGoal::checkOutputs(const std::map<std::string, ValidPathInfo
                     worker.store.printStorePath(info.path), info.narSize, *checks.maxSize);
 
             if (checks.maxClosureSize) {
-                uint64_t closureSize = getClosure(info.path).second;
+                uint64_t closureSize = getClosure(info.path).size;
                 if (closureSize > *checks.maxClosureSize)
                     throw BuildError("closure of path '%s' is too large at %d bytes; limit is %d bytes",
                         worker.store.printStorePath(info.path), closureSize, *checks.maxClosureSize);
@@ -2868,32 +2894,51 @@ void LocalDerivationGoal::checkOutputs(const std::map<std::string, ValidPathInfo
                         throw BuildError("derivation contains an illegal reference specifier '%s'", i);
                 }
 
-                auto used = recursive
-                    ? getClosure(info.path).first
-                    : info.references;
+                std::map<StorePath, std::shared_ptr<PathWithSource>> used;
+                if (recursive) {
+                    used = getClosure(info.path).pathsWithSources;
+                } else {
+                    for (auto & ref : info.references)
+                        used.insert({ref, std::make_shared<PathWithSource>(PathWithSource { .path = info.path, .source = nullptr })});
+                }
 
                 if (recursive && checks.ignoreSelfRefs)
                     used.erase(info.path);
 
-                StorePathSet badPaths;
+                std::map<StorePath, std::shared_ptr<PathWithSource>> badPaths;
 
-                for (auto & i : used)
+                for (auto & [i, source] : used)
                     if (allowed) {
                         if (!spec.count(i))
-                            badPaths.insert(i);
+                            badPaths.insert({i, source});
                     } else {
                         if (spec.count(i))
-                            badPaths.insert(i);
+                            badPaths.insert({i, source});
                     }
 
                 if (!badPaths.empty()) {
                     std::string badPathsStr;
-                    for (auto & i : badPaths) {
-                        badPathsStr += "\n  ";
-                        badPathsStr += worker.store.printStorePath(i);
+
+                    if (recursive) {
+                        for (auto & [i, source] : badPaths) {
+                            badPathsStr += "\n  { ";
+                            badPathsStr += source->showChain(worker.store);
+                            badPathsStr += " -> ";
+                            badPathsStr += worker.store.printStorePath(i);
+                            badPathsStr += " }";
+                        }
+                        // Consider working with paths in a more stable order (https://github.com/NixOS/nix/issues/10875)
+                        // and/or get all possible chains and present them in the `nix-store -q --tree` format.
+                        throw BuildError("output '%s' is not allowed to refer to the following paths.\nShown below are chains that lead to the forbidden path(s). More may exist, in which case this pick is arbitrary.%s",
+                            worker.store.printStorePath(info.path), badPathsStr);
+                    } else {
+                        for (auto & [i, source] : badPaths) {
+                            badPathsStr += "\n  ";
+                            badPathsStr += worker.store.printStorePath(i);
+                        }
+                        throw BuildError("output '%s' is not allowed to have direct references to the following paths:%s",
+                            worker.store.printStorePath(info.path), badPathsStr);
                     }
-                    throw BuildError("output '%s' is not allowed to refer to the following paths:%s",
-                        worker.store.printStorePath(info.path), badPathsStr);
                 }
             };
 

tests/functional/check-reqs.nix

@@ -45,7 +45,7 @@ rec {
     name = "check-reqs";
     inherit deps;
     builder = builtins.toFile "builder.sh" "mkdir $out; ln -s $deps $out/depdir1";
-    disallowedRequisites = [dep1];
+    disallowedRequisites = [ dep1 dep2 ];
   };
 
   test7 = mkDerivation {

tests/functional/check-reqs.sh

@@ -13,6 +13,13 @@ nix-build -o $RESULT check-reqs.nix -A test1
 (! nix-build -o $RESULT check-reqs.nix -A test4) 2>&1 | grepQuiet 'check-reqs-dep1'
 (! nix-build -o $RESULT check-reqs.nix -A test4) 2>&1 | grepQuiet 'check-reqs-dep2'
 (! nix-build -o $RESULT check-reqs.nix -A test5)
-(! nix-build -o $RESULT check-reqs.nix -A test6)
+if isDaemonNewer "2.23pre"
+then
+  (! nix-build -o $RESULT check-reqs.nix -A test6) 2>&1 | grepQuiet '{ /.*-check-reqs -> /.*-check-reqs-deps -> /.*-check-reqs-dep1 }'
+  (! nix-build -o $RESULT check-reqs.nix -A test6) 2>&1 | grepQuiet '{ /.*-check-reqs -> /.*-check-reqs-deps -> /.*-check-reqs-dep2 }'
+else
+  (! nix-build -o $RESULT check-reqs.nix -A test6) 2>&1 | grepQuiet 'check-reqs-dep1'
+  (! nix-build -o $RESULT check-reqs.nix -A test6) 2>&1 | grepQuiet 'check-reqs-dep2'
+fi
 
 nix-build -o $RESULT check-reqs.nix -A test7