Fix accessing 'toString path'

src/libexpr/attr-path.cc

@@ -118,33 +118,21 @@ std::pair<SourcePath, uint32_t> findPackageFilename(EvalState & state, Value & v
 
     // FIXME: is it possible to extract the Pos object instead of doing this
     //        toString + parsing?
-    auto pos = state.forceString(*v2);
+    PathSet context;
+    auto path = state.coerceToPath(noPos, *v2, context);
 
-    auto fail = [pos]() {
-        throw ParseError("cannot parse 'meta.position' attribute '%s'", pos);
+    auto fn = path.path.abs();
+
+    auto fail = [fn]() {
+        throw ParseError("cannot parse 'meta.position' attribute '%s'", fn);
     };
 
     try {
-        std::string_view prefix = "/virtual/";
-
-        if (!hasPrefix(pos, prefix)) fail();
-        pos = pos.substr(prefix.size());
-
-        auto slash = pos.find('/');
-        if (slash == std::string::npos) fail();
-        size_t number = std::stoi(std::string(pos, 0, slash));
-        pos = pos.substr(slash);
-
-        auto accessor = state.inputAccessors.find(number);
-        if (accessor == state.inputAccessors.end()) fail();
-
-        auto colon = pos.rfind(':');
+        auto colon = fn.rfind(':');
         if (colon == std::string::npos) fail();
-        std::string filename(pos, 0, colon);
-        auto lineno = std::stoi(std::string(pos, colon + 1, std::string::npos));
-
-        return {SourcePath{accessor->second, CanonPath(filename)}, lineno};
-
+        std::string filename(fn, 0, colon);
+        auto lineno = std::stoi(std::string(fn, colon + 1, std::string::npos));
+        return {SourcePath{path.accessor, CanonPath(fn.substr(0, colon))}, lineno};
     } catch (std::invalid_argument & e) {
         fail();
         abort();

src/libexpr/eval.cc

@@ -1098,7 +1098,7 @@ void EvalState::mkPos(Value & v, PosIdx p)
     auto pos = positions[p];
     if (auto path = std::get_if<SourcePath>(&pos.origin)) {
         auto attrs = buildBindings(3);
-        attrs.alloc(sFile).mkString(fmt("/virtual/%d%s", path->accessor->number, path->path.abs()));
+        attrs.alloc(sFile).mkString(encodePath(*path));
         attrs.alloc(sLine).mkInt(pos.line);
         attrs.alloc(sColumn).mkInt(pos.column);
         v.mkAttrs(attrs);
@@ -1963,8 +1963,12 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
            and none of the strings are allowed to have contexts. */
         if (first) {
             firstType = vTmp->type();
-            if (vTmp->type() == nPath)
+            if (vTmp->type() == nPath) {
                 accessor = vTmp->path().accessor;
+                auto part = vTmp->path().path.abs();
+                sSize += part.size();
+                s.emplace_back(std::move(part));
+            }
         }
 
         if (firstType == nInt) {
@@ -1984,6 +1988,12 @@ void ExprConcatStrings::eval(EvalState & state, Env & env, Value & v)
                 nf += vTmp->fpoint;
             } else
                 state.throwEvalError(i_pos, "cannot add %1% to a float", showType(*vTmp), env, *this);
+        } else if (firstType == nPath) {
+            if (!first) {
+                auto part = state.coerceToString(i_pos, *vTmp, context, false, false);
+                sSize += part->size();
+                s.emplace_back(std::move(part));
+            }
         } else {
             if (s.empty()) s.reserve(es->size());
             auto part = state.coerceToString(i_pos, *vTmp, context, false, firstType == nString);
@@ -2205,7 +2215,7 @@ BackedStringView EvalState::coerceToString(const PosIdx pos, Value & v, PathSet
         auto path = v.path();
         return copyToStore
             ? store->printStorePath(copyPathToStore(context, path))
-            : BackedStringView((Path) path.path.abs());
+            : encodePath(path);
     }
 
     if (v.type() == nAttrs) {
@@ -2275,10 +2285,7 @@ SourcePath EvalState::coerceToPath(const PosIdx pos, Value & v, PathSet & contex
 
     if (v.type() == nString) {
         copyContext(v, context);
-        auto path = v.str();
-        if (path == "" || path[0] != '/')
-            throwEvalError(pos, "string '%1%' doesn't represent an absolute path", path);
-        return {rootFS, CanonPath(path)};
+        return decodePath(v.str(), pos);
     }
 
     if (v.type() == nPath)

src/libexpr/eval.hh

@@ -224,6 +224,13 @@ public:
 
     void registerAccessor(ref<InputAccessor> accessor);
 
+    /* Convert a path to a string representation of the format
+       `/__virtual__/<accessor-number>/<path>`. */
+    std::string encodePath(const SourcePath & path);
+
+    /* Decode a path encoded by `encodePath()`. */
+    SourcePath decodePath(std::string_view s, PosIdx pos = noPos);
+
     /* Allow access to a path. */
     void allowPath(const Path & path);
 

src/libexpr/paths.cc

@@ -14,4 +14,53 @@ void EvalState::registerAccessor(ref<InputAccessor> accessor)
     inputAccessors.emplace(accessor->number, accessor);
 }
 
+static constexpr std::string_view marker = "/__virtual__/";
+
+std::string EvalState::encodePath(const SourcePath & path)
+{
+    /* For backward compatibility, return paths in the root FS
+       normally. Encoding any other path is not very reproducible (due
+       to /__virtual__/<N>) and we should depreceate it eventually. So
+       print a warning about use of an encoded path in
+       decodePath(). */
+    return path.accessor == rootFS
+        ? path.path.abs()
+        : std::string(marker) + std::to_string(path.accessor->number) + path.path.abs();
+}
+
+SourcePath EvalState::decodePath(std::string_view s, PosIdx pos)
+{
+    if (!hasPrefix(s, "/"))
+        throwEvalError(pos, "string '%1%' doesn't represent an absolute path", s);
+
+    if (hasPrefix(s, marker)) {
+        auto fail = [s]() {
+            throw Error("cannot decode virtual path '%s'", s);
+        };
+
+        s = s.substr(marker.size());
+
+        try {
+            auto slash = s.find('/');
+            if (slash == std::string::npos) fail();
+            size_t number = std::stoi(std::string(s, 0, slash));
+            s = s.substr(slash);
+
+            auto accessor = inputAccessors.find(number);
+            if (accessor == inputAccessors.end()) fail();
+
+            SourcePath path {accessor->second, CanonPath(s)};
+
+            static bool warned = false;
+            warnOnce(warned, fmt("applying 'toString' to path '%s' and then accessing it is deprecated, at %s", path, positions[pos]));
+
+            return path;
+        } catch (std::invalid_argument & e) {
+            fail();
+            abort();
+        }
+    } else
+        return {rootFS, CanonPath(s)};
+}
+
 }

src/libfetchers/input-accessor.cc

@@ -11,7 +11,7 @@ static std::atomic<size_t> nextNumber{0};
 
 InputAccessor::InputAccessor()
     : number(++nextNumber)
-    , displayPrefix{"/virtual/" + std::to_string(number)}
+    , displayPrefix{"«unknown»"}
 {
 }
 

tests/local.mk

@@ -109,7 +109,8 @@ nix_tests = \
   store-ping.sh \
   fetchClosure.sh \
   completions.sh \
-  impure-derivations.sh
+  impure-derivations.sh \
+  toString-path.sh
 
 ifeq ($(HAVE_LIBCPUID), 1)
 	nix_tests += compute-levels.sh

tests/toString-path.sh

@@ -0,0 +1,6 @@
+source common.sh
+
+mkdir -p $TEST_ROOT/foo
+echo bla > $TEST_ROOT/foo/bar
+
+[[ $(nix eval --raw --impure --expr "builtins.readFile (builtins.toString (builtins.fetchTree { type = \"path\"; path = \"$TEST_ROOT/foo\"; } + \"bar\"))") = bla ]]