Merge "libstore: Show illegal path references in fixed-outputs derivations" into main

doc/manual/change-authors.yml

@@ -204,6 +204,11 @@ teofilc:
   forgejo: teofilc
   github: TeofilC
 
+thubrecht:
+  display_name: Tom Hubrecht
+  forgejo: tom-hubrecht
+  github: Tom-Hubrecht
+
 thufschmitt:
   display_name: Théophane Hufschmitt
   github: thufschmitt

doc/manual/rl-next/illegal-path-references.md

@@ -0,0 +1,43 @@
+---
+synopsis: Show illegal path references in fixed-outputs derivations
+issues: [fj#530]
+cls: [2726]
+category: Fixes
+credits: [thubrecht]
+---
+
+The error created when referencing a store path in a Fixed-Output Derivation is now more verbose, listing the offending paths.
+This allows for better pinpointing where the issue might be.
+
+An offender is the following derivation:
+
+```nix
+pkgs.stdenv.mkDerivation {
+  name = "illegal-fod";
+
+  dontUnpack = true;
+  dontBuild = true;
+
+  installPhase = ''
+    cp -R ${pkgs.hello} $out
+  '';
+
+  outputHashMode = "recursive";
+  outputHashAlgo = "sha256";
+  outputHash = pkgs.lib.fakeHash;
+}
+```
+
+The previous error shown would have been:
+
+```
+error: illegal path references in fixed-output derivation '/nix/store/rpq4m1y79s2nhs1hj7k47yiyykxykiqa-illegal-fod.drv'
+```
+
+and is now:
+
+```
+error: the fixed-output derivation '/nix/store/rpq4m1y79s2nhs1hj7k47yiyykxykiqa-illegal-fod.drv' must not reference store paths but 2 such references were found:
+         /nix/store/1q8w6gl1ll0mwfkqc3c2yx005s6wwfrl-hello-2.12.1
+         /nix/store/wn7v2vhyyyi6clcyn0s9ixvl7d4d87ic-glibc-2.40-36
+```

lix/libstore/build/local-derivation-goal.cc

@@ -1861,10 +1861,21 @@ try {
                             wanted.to_string(Base::SRI, true),
                             got.to_string(Base::SRI, true)));
                 }
-                if (!newInfo0.references.empty())
-                    delayedException = std::make_exception_ptr(
-                        BuildError("illegal path references in fixed-output derivation '%s'",
-                            worker.store.printStorePath(drvPath)));
+                if (!newInfo0.references.empty()) {
+                    std::string references;
+
+                    for (StorePath r : newInfo0.references) {
+                        references.append("\n  " + worker.store.printStorePath(r));
+                    }
+
+                    delayedException = std::make_exception_ptr(BuildError(
+                        "the fixed-output derivation '%s' must not reference store paths but "
+                        "%d such references were found:%s",
+                        worker.store.printStorePath(drvPath),
+                        newInfo0.references.size(),
+                        references
+                    ));
+                }
 
                 return newInfo0;
             },

tests/functional/fixed.nix

@@ -57,6 +57,18 @@ rec {
     outputHashMode = "flat";
   };
 
+  illegalReferences = mkDerivation {
+    name = "illegal-reference";
+    ref = builtins.head good;
+
+    builder = builtins.toFile "builder.sh" ''
+      mkdir $out
+      cp -R $ref $out
+    '';
+    outputHashMode = "recursive";
+    outputHash = "sha256-AAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAAA=";
+  };
+
   # Test for building two derivations in parallel that produce the
   # same output path because they're fixed-output derivations.
   parallelSame = [

tests/functional/fixed.sh

@@ -31,6 +31,13 @@ if isDaemonNewer "2.20pre20240108"; then
     expectStderr 1 nix-build fixed.nix -A badReferences | grepQuiet "not allowed to refer to other store paths"
 fi
 
+echo 'testing illegal references...'
+# Fixed FOD hashes cannot be asserted because:
+# - the store directory varies between the "Lix build sandbox environment" and a user test run
+# - *-darwin has a different store location on the top of this in the sandbox (/private/tmp/...) causing further changes.
+# Regex matching is the best we can afford.
+expectStderr 102 nix-build fixed.nix -A illegalReferences | grep -z "the fixed-output derivation '$TEST_ROOT/store/[a-z0-9]*-illegal-reference.drv' must not reference store paths but 1 such references were found:.*$TEST_ROOT/store/[a-z0-9]*-fixed" > /dev/null
+
 # While we're at it, check attribute selection a bit more.
 echo 'testing attribute selection...'
 test $(nix-instantiate fixed.nix -A good.1 | wc -l) = 1