Merge pull request #79 from DeterminateSystems/path-position-info

Include the source location when warning about inefficient double copies

Author: grahamc

src/libexpr/eval.cc

@@ -149,6 +149,8 @@ PosIdx Value::determinePos(const PosIdx pos) const
     // Allow selecting a subset of enum values
     #pragma GCC diagnostic push
     #pragma GCC diagnostic ignored "-Wswitch-enum"
+    if (this->pos != 0)
+        return PosIdx(this->pos);
     switch (internalType) {
         case tAttrs: return attrs()->pos;
         case tLambda: return payload.lambda.fun->pos;
@@ -906,7 +908,7 @@ void Value::mkStringMove(const char * s, const NixStringContext & context)
 
 void Value::mkPath(const SourcePath & path)
 {
-    mkPath(&*path.accessor, makeImmutableString(path.path.abs()));
+    mkPath(&*path.accessor, makeImmutableString(path.path.abs()), noPos.get());
 }
 
 
@@ -2356,7 +2358,7 @@ BackedStringView EvalState::coerceToString(
               // slash, as in /foo/${x}.
               v.payload.path.path
             : copyToStore
-            ? store->printStorePath(copyPathToStore(context, v.path()))
+            ? store->printStorePath(copyPathToStore(context, v.path(), v.determinePos(pos)))
             : ({
                 auto path = v.path();
                 if (path.accessor == rootFS && store->isInStore(path.path.abs())) {
@@ -2434,7 +2436,7 @@ BackedStringView EvalState::coerceToString(
 }
 
 
-StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePath & path)
+StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePath & path, PosIdx pos)
 {
     if (nix::isDerivation(path.path.abs()))
         error<EvalError>("file names are not allowed to end in '%1%'", drvExtension).debugThrow();
@@ -2448,7 +2450,7 @@ StorePath EvalState::copyPathToStore(NixStringContext & context, const SourcePat
                 *store,
                 path.resolveSymlinks(SymlinkResolution::Ancestors),
                 settings.readOnlyMode ? FetchMode::DryRun : FetchMode::Copy,
-                computeBaseName(path),
+                computeBaseName(path, pos),
                 ContentAddressMethod::Raw::NixArchive,
                 nullptr,
                 repair);

src/libexpr/include/nix/expr/eval.hh

@@ -594,7 +594,7 @@ public:
         bool coerceMore = false, bool copyToStore = true,
         bool canonicalizePath = true);
 
-    StorePath copyPathToStore(NixStringContext & context, const SourcePath & path);
+    StorePath copyPathToStore(NixStringContext & context, const SourcePath & path, PosIdx pos);
 
 
     /**
@@ -607,7 +607,7 @@ public:
      * materialize /nix/store/<hash2>-source though. Still, this
      * requires reading/hashing the path twice.
      */
-    std::string computeBaseName(const SourcePath & path);
+    std::string computeBaseName(const SourcePath & path, PosIdx pos);
 
     /**
      * Path coercion.

src/libexpr/include/nix/expr/nixexpr.hh

@@ -138,9 +138,9 @@ struct ExprPath : Expr
     ref<SourceAccessor> accessor;
     std::string s;
     Value v;
-    ExprPath(ref<SourceAccessor> accessor, std::string s) : accessor(accessor), s(std::move(s))
+    ExprPath(ref<SourceAccessor> accessor, std::string s, PosIdx pos) : accessor(accessor), s(std::move(s))
     {
-        v.mkPath(&*accessor, this->s.c_str());
+        v.mkPath(&*accessor, this->s.c_str(), pos.get());
     }
     Value * maybeThunk(EvalState & state, Env & env) override;
     COMMON_METHODS

src/libexpr/include/nix/expr/value.hh

@@ -167,6 +167,7 @@ struct Value
 {
 private:
     InternalType internalType = tUninitialized;
+    uint32_t pos{0};
 
     friend std::string showType(const Value & v);
 
@@ -289,10 +290,11 @@ public:
             unreachable();
     }
 
-    inline void finishValue(InternalType newType, Payload newPayload)
+    inline void finishValue(InternalType newType, Payload newPayload, uint32_t newPos = 0)
     {
         payload = newPayload;
         internalType = newType;
+        pos = newPos;
     }
 
     /**
@@ -339,9 +341,9 @@ public:
     void mkPath(const SourcePath & path);
     void mkPath(std::string_view path);
 
-    inline void mkPath(SourceAccessor * accessor, const char * path)
+    inline void mkPath(SourceAccessor * accessor, const char * path, uint32_t pos)
     {
-        finishValue(tPath, { .path = { .accessor = accessor, .path = path } });
+        finishValue(tPath, { .path = { .accessor = accessor, .path = path } }, pos);
     }
 
     inline void mkNull()
@@ -482,6 +484,9 @@ public:
 
     NixFloat fpoint() const
     { return payload.fpoint; }
+
+    inline uint32_t getPos() const
+    { return pos; }
 };
 
 

src/libexpr/parser.y

@@ -374,8 +374,8 @@ path_start
            root filesystem accessor, rather than the accessor of the
            current Nix expression. */
         literal.front() == '/'
-        ? new ExprPath(state->rootFS, std::move(path))
-        : new ExprPath(state->basePath.accessor, std::move(path));
+        ? new ExprPath(state->rootFS, std::move(path), CUR_POS)
+        : new ExprPath(state->basePath.accessor, std::move(path), CUR_POS);
   }
   | HPATH {
     if (state->settings.pureEval) {
@@ -385,7 +385,7 @@ path_start
         );
     }
     Path path(getHome() + std::string($1.p + 1, $1.l - 1));
-    $$ = new ExprPath(ref<SourceAccessor>(state->rootFS), std::move(path));
+    $$ = new ExprPath(ref<SourceAccessor>(state->rootFS), std::move(path), CUR_POS);
   }
   ;
 

src/libexpr/paths.cc

@@ -52,15 +52,17 @@ std::string EvalState::devirtualize(std::string_view s, const NixStringContext &
     return rewriteStrings(std::string(s), rewrites);
 }
 
-std::string EvalState::computeBaseName(const SourcePath & path)
+std::string EvalState::computeBaseName(const SourcePath & path, PosIdx pos)
 {
     if (path.accessor == rootFS) {
         if (auto storePath = store->maybeParseStorePath(path.path.abs())) {
             warn(
-                "Performing inefficient double copy of path '%s' to the store. "
-                "This can typically be avoided by rewriting an attribute like `src = ./.` "
-                "to `src = builtins.path { path = ./.; name = \"source\"; }`.",
-                path);
+                "Copying '%s' to the store again\n"
+                "You can make Nix evaluate faster and copy fewer files by replacing `./.` with the `self` flake input, "
+                "or `builtins.path { path = ./.; name = \"source\"; }`\n\n"
+                "Location: %s\n",
+                path,
+                positions[pos]);
             return std::string(fetchToStore(*store, path, FetchMode::DryRun, storePath->name()).to_string());
         }
     }

src/libexpr/primops.cc

@@ -2620,7 +2620,7 @@ static void prim_filterSource(EvalState & state, const PosIdx pos, Value * * arg
         "while evaluating the second argument (the path to filter) passed to 'builtins.filterSource'");
     state.forceFunction(*args[0], pos, "while evaluating the first argument passed to builtins.filterSource");
 
-    addPath(state, pos, state.computeBaseName(path), path, args[0], ContentAddressMethod::Raw::NixArchive, std::nullopt, v, context);
+    addPath(state, pos, state.computeBaseName(path, pos), path, args[0], ContentAddressMethod::Raw::NixArchive, std::nullopt, v, context);
 }
 
 static RegisterPrimOp primop_filterSource({

src/libexpr/value-to-json.cc

@@ -39,7 +39,7 @@ json printValueAsJSON(EvalState & state, bool strict,
         case nPath:
             if (copyToStore)
                 out = state.store->printStorePath(
-                    state.copyPathToStore(context, v.path()));
+                    state.copyPathToStore(context, v.path(), v.determinePos(pos)));
             else
                 out = v.path().path.abs();
             break;

src/libutil/include/nix/util/pos-idx.hh

@@ -15,12 +15,12 @@ class PosIdx
 private:
     uint32_t id;
 
+public:
     explicit PosIdx(uint32_t id)
         : id(id)
     {
     }
 
-public:
     PosIdx()
         : id(0)
     {
@@ -45,6 +45,11 @@ public:
     {
         return std::hash<uint32_t>{}(id);
     }
+
+    uint32_t get() const
+    {
+        return id;
+    }
 };
 
 inline PosIdx noPos = {};