From 108c051fd826dabf9b011c9db01b75f7a7724eec Mon Sep 17 00:00:00 2001
From: Sergei Trofimovich <slyich@gmail.com>
Date: Wed, 25 Dec 2024 21:09:58 +0000
Subject: [PATCH] local-derivation-goal: improve "illegal reference" error

Before the change "illegal reference" was hard to interpret as it did
not mention what derivation actually hits it.

Today's `nixpkgs` example:

Before the change:

    $ nix build --no-link -f. postgresql_14
    ...
    error: derivation contains an illegal reference specifier 'man'

After the change:

    $ nix build --no-link -f. postgresql_14
    ...
    error: derivation '/nix/store/bxp6g57limvwiga61vdlyvhy7i8rp6wd-postgresql-14.15.drv' output check for 'lib' contains an illegal reference specifier 'man', expected store path or output name (one of [debug, dev, doc, lib, out])

Co-authored-by: Robert Hensing <roberth@users.noreply.github.com>
Co-authored-by: Maximilian Bosch <maximilian@mbosch.me>

(cherry picked from commit ec46a7e4dea8c568677d3d98588810bcd178f048)
Change-Id: I36e3e951c282123e780a920d5bef59de74de9fe0
---
 lix/libstore/build/local-derivation-goal.cc | 20 ++++++++++++++++++--
 tests/functional/check-refs.nix             |  6 ++++++
 tests/functional/check-refs.sh              |  4 ++++
 3 files changed, 28 insertions(+), 2 deletions(-)

diff --git a/lix/libstore/build/local-derivation-goal.cc b/lix/libstore/build/local-derivation-goal.cc
index 625f33cba..e332104fd 100644
--- a/lix/libstore/build/local-derivation-goal.cc
+++ b/lix/libstore/build/local-derivation-goal.cc
@@ -2516,8 +2516,24 @@ void LocalDerivationGoal::checkOutputs(const std::map<std::string, ValidPathInfo
                         spec.insert(output->path);
                     else if (auto storePath = get(alreadyRegisteredOutputs, i))
                         spec.insert(*storePath);
-                    else
-                        throw BuildError("derivation contains an illegal reference specifier '%s'", i);
+                    else {
+                        std::string outputsListing = concatMapStringsSep(
+                            ", ",
+                            newlyBuiltOutputs,
+                            [](auto & o) { return o.first; }
+                        );
+                        if (!alreadyRegisteredOutputs.empty()) {
+                            outputsListing.append(outputsListing.empty() ? "" : ", ");
+                            outputsListing.append(concatMapStringsSep(
+                                ", ",
+                                alreadyRegisteredOutputs,
+                                [](auto & o) { return o.first; })
+                            );
+                        }
+                        throw BuildError("derivation '%s' output check for '%s' contains an illegal reference specifier '%s',"
+                            " expected store path or output name (one of [%s])",
+                            worker.store.printStorePath(drvPath), outputName, i, outputsListing);
+                    }
                 }
 
                 auto used = recursive
diff --git a/tests/functional/check-refs.nix b/tests/functional/check-refs.nix
index 89690e456..1a900830e 100644
--- a/tests/functional/check-refs.nix
+++ b/tests/functional/check-refs.nix
@@ -74,4 +74,10 @@ rec {
     buildCommand = ''echo ${dep} > "''${outputs[out]}"'';
   };
 
+  test12 = makeTest 12 {
+    builder = builtins.toFile "builder.sh" "mkdir $out $lib";
+    outputs = ["out" "lib"];
+    disallowedReferences = ["dev"];
+  };
+
 }
diff --git a/tests/functional/check-refs.sh b/tests/functional/check-refs.sh
index 3b587d1e5..d4a5aa29f 100644
--- a/tests/functional/check-refs.sh
+++ b/tests/functional/check-refs.sh
@@ -51,3 +51,7 @@ if isDaemonNewer 2.12pre20230103; then
     test11=$(nix-build -o $RESULT check-refs.nix -A test11)
     [[ -z $(nix-store -q --references "$test11") ]]
 fi
+
+# test12 should fail (syntactically invalid).
+expectStderr 1 nix-build -vvv -o "$RESULT" check-refs.nix -A test12 >"$TEST_ROOT/test12.stderr"
+grepQuiet -F "output check for 'lib' contains an illegal reference specifier 'dev', expected store path or output name (one of [lib, out])" < "$TEST_ROOT/test12.stderr"