From 0c9c7d5d440ec9ec22d62eaefc2b1edaa2ce89fd Mon Sep 17 00:00:00 2001
From: Alex Shabalin <alex.shabalin@tweag.io>
Date: Mon, 22 Nov 2021 16:28:10 +0100
Subject: [PATCH] Print worktree path instead of store path for flake errors

Closes https://github.com/NixOS/nix/issues/5425
---
 src/libcmd/installables.cc            | 68 ++++++++++++++++++++-------
 src/libutil/error.hh                  |  6 +++
 tests/flake-print-local-path-error.sh | 14 ++++++
 tests/local.mk                        |  1 +
 4 files changed, 71 insertions(+), 18 deletions(-)
 create mode 100644 tests/flake-print-local-path-error.sh

diff --git a/src/libcmd/installables.cc b/src/libcmd/installables.cc
index 5758b52ad..0349ce6e7 100644
--- a/src/libcmd/installables.cc
+++ b/src/libcmd/installables.cc
@@ -521,33 +521,65 @@ InstallableFlake::InstallableFlake(
         throw UsageError("'--arg' and '--argstr' are incompatible with flakes");
 }
 
+namespace {
+std::optional<Path> getSourceFilePathFromInStorePath(Store & store, const Path & file, const fetchers::Input & flakeInput) {
+    if (!store.isInStore(file)) {
+        return std::nullopt;
+    }
+
+    auto [sourceStorePath, relPath] = store.toStorePath(file);
+    auto realPath = store.toRealPath(sourceStorePath);
+
+    auto maybeFlakeSource = flakeInput.getSourcePath();
+    if (!maybeFlakeSource) {
+        return std::nullopt;
+    }
+
+    auto betterFilePath = replaceStrings(file, realPath, *maybeFlakeSource);
+    if (!pathExists(betterFilePath)) {
+        return std::nullopt;
+    }
+
+    return betterFilePath;
+}
+}
+
 std::tuple<std::string, FlakeRef, InstallableValue::DerivationInfo> InstallableFlake::toDerivation()
 {
-    auto lockedFlake = getLockedFlake();
+    try {
+        auto lockedFlake = getLockedFlake();
 
-    auto cache = openEvalCache(*state, lockedFlake);
-    auto root = cache->getRoot();
+        auto cache = openEvalCache(*state, lockedFlake);
+        auto root = cache->getRoot();
 
-    for (auto & attrPath : getActualAttrPaths()) {
-        auto attr = root->findAlongAttrPath(
-            parseAttrPath(*state, attrPath),
-            true
-        );
+        for (auto & attrPath : getActualAttrPaths()) {
+            auto attr = root->findAlongAttrPath(
+                parseAttrPath(*state, attrPath),
+                true
+            );
 
-        if (!attr) continue;
+            if (!attr) continue;
 
-        if (!attr->isDerivation())
-            throw Error("flake output attribute '%s' is not a derivation", attrPath);
+            if (!attr->isDerivation())
+                throw Error("flake output attribute '%s' is not a derivation", attrPath);
 
-        auto drvPath = attr->forceDerivation();
+            auto drvPath = attr->forceDerivation();
 
-        auto drvInfo = DerivationInfo{
-            std::move(drvPath),
-            state->store->maybeParseStorePath(attr->getAttr(state->sOutPath)->getString()),
-            attr->getAttr(state->sOutputName)->getString()
-        };
+            auto drvInfo = DerivationInfo{
+                std::move(drvPath),
+                state->store->maybeParseStorePath(attr->getAttr(state->sOutPath)->getString()),
+                attr->getAttr(state->sOutputName)->getString()
+            };
 
-        return {attrPath, lockedFlake->flake.lockedRef, std::move(drvInfo)};
+            return {attrPath, lockedFlake->flake.lockedRef, std::move(drvInfo)};
+        }
+    } catch (BaseError & e) {
+        if (auto pos = e.info().errPos) {
+            if (auto sourceFilePath = getSourceFilePathFromInStorePath(*state->store, pos->file, flakeRef.input)) {
+                e.setBetterErrPosFile(*sourceFilePath);
+            }
+        }
+        throw;
     }
 
     throw Error("flake '%s' does not provide attribute %s",
diff --git a/src/libutil/error.hh b/src/libutil/error.hh
index ff58d3e00..4622fe4c6 100644
--- a/src/libutil/error.hh
+++ b/src/libutil/error.hh
@@ -164,6 +164,12 @@ public:
 
     const string & msg() const { return calcWhat(); }
     const ErrorInfo & info() const { calcWhat(); return err; }
+    
+    void setBetterErrPosFile(std::string file) {
+        if (err.errPos) {
+            err.errPos->file = std::move(file);
+        }
+    }
 
     template<typename... Args>
     BaseError & addTrace(std::optional<ErrPos> e, const string &fs, const Args & ... args)
diff --git a/tests/flake-print-local-path-error.sh b/tests/flake-print-local-path-error.sh
new file mode 100644
index 000000000..ff1576a04
--- /dev/null
+++ b/tests/flake-print-local-path-error.sh
@@ -0,0 +1,14 @@
+source common.sh
+
+clearStore
+rm -rf $TEST_HOME/.cache $TEST_HOME/.config $TEST_HOME/.local
+
+cd $TEST_HOME
+
+cat <<EOF > flake.nix
+{
+    outputs = give me an error here;
+}
+EOF
+
+nix build |& grep $TEST_HOME || fail "Path should point to home, not to the store"
diff --git a/tests/local.mk b/tests/local.mk
index 6f38853bc..5071009ab 100644
--- a/tests/local.mk
+++ b/tests/local.mk
@@ -47,6 +47,7 @@ nix_tests = \
   describe-stores.sh \
   flakes.sh \
   flake-local-settings.sh \
+  flake-print-local-path-error.sh \
   build.sh \
   compute-levels.sh \
   repl.sh ca/repl.sh \