Make 'nix edit' etc. work again

src/libcmd/command.cc

@@ -207,8 +207,11 @@ void StorePathCommand::run(ref<Store> store, std::vector<StorePath> && storePath
     run(store, *storePaths.begin());
 }
 
-Strings editorFor(const Path & file, uint32_t line)
+Strings editorFor(const SourcePath & file, uint32_t line)
 {
+    auto path = file.getPhysicalPath();
+    if (!path)
+        throw Error("cannot open '%s' in an editor because it has no physical path", file);
     auto editor = getEnv("EDITOR").value_or("cat");
     auto args = tokenizeString<Strings>(editor);
     if (line > 0 && (
@@ -217,7 +220,7 @@ Strings editorFor(const Path & file, uint32_t line)
         editor.find("vim") != std::string::npos ||
         editor.find("kak") != std::string::npos))
         args.push_back(fmt("+%d", line));
-    args.push_back(file);
+    args.push_back(path->abs());
     return args;
 }
 

src/libcmd/command.hh

@@ -229,7 +229,7 @@ static RegisterCommand registerCommand2(std::vector<std::string> && name)
 
 /* Helper function to generate args that invoke $EDITOR on
    filename:lineno. */
-Strings editorFor(const Path & file, uint32_t line);
+Strings editorFor(const SourcePath & file, uint32_t line);
 
 struct MixProfile : virtual StoreCommand
 {

src/libcmd/repl.cc

@@ -573,17 +573,17 @@ bool NixRepl::processLine(std::string line)
         Value v;
         evalString(arg, v);
 
-        const auto [file, line] = [&] () -> std::pair<std::string, uint32_t> {
+        const auto [path, line] = [&] () -> std::pair<SourcePath, uint32_t> {
             if (v.type() == nPath || v.type() == nString) {
                 PathSet context;
-                auto filename = state->coerceToString(noPos, v, context).toOwned();
+                auto path = state->coerceToPath(noPos, v, context);
-                state->symbols.create(filename);
+                return {path, 0};
-                return {filename, 0};
             } else if (v.isLambda()) {
                 auto pos = state->positions[v.lambda.fun->pos];
-                // FIXME
+                if (auto path = std::get_if<SourcePath>(&pos.origin))
-                abort();
+                    return {*path, pos.line};
-                //return {pos.file, pos.line};
+                else
+                    throw Error("'%s' cannot be shown in an editor", pos);
             } else {
                 // assume it's a derivation
                 return findPackageFilename(*state, v, arg);
@@ -591,7 +591,7 @@ bool NixRepl::processLine(std::string line)
         }();
 
         // Open in EDITOR
-        auto args = editorFor(file, line);
+        auto args = editorFor(path, line);
         auto editor = args.front();
         args.pop_front();
 

src/libexpr/attr-path.cc

@@ -106,7 +106,7 @@ std::pair<Value *, PosIdx> findAlongAttrPath(EvalState & state, const std::strin
 }
 
 
-std::pair<std::string, uint32_t> findPackageFilename(EvalState & state, Value & v, std::string what)
+std::pair<SourcePath, uint32_t> findPackageFilename(EvalState & state, Value & v, std::string what)
 {
     Value * v2;
     try {
@@ -120,19 +120,41 @@ std::pair<std::string, uint32_t> findPackageFilename(EvalState & state, Value &
     //        toString + parsing?
     auto pos = state.forceString(*v2);
 
-    auto colon = pos.rfind(':');
+    auto fail = [pos]() {
-    if (colon == std::string::npos)
+        throw ParseError("cannot parse 'meta.position' attribute '%s'", pos);
-        throw ParseError("cannot parse meta.position attribute '%s'", pos);
+    };
 
-    std::string filename(pos, 0, colon);
-    unsigned int lineno;
     try {
-        lineno = std::stoi(std::string(pos, colon + 1, std::string::npos));
+        std::string_view prefix = "/virtual/";
-    } catch (std::invalid_argument & e) {
+
-        throw ParseError("cannot parse line number '%s'", pos);
+        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);
+
+        std::shared_ptr<InputAccessor> accessor;
+        for (auto & i : state.inputAccessors)
+            if (i.second->number == number) {
+                accessor = i.second;
+                break;
             }
 
-    return { std::move(filename), lineno };
+        if (!accessor) fail();
+
+        auto colon = pos.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{ref(accessor), CanonPath(filename)}, lineno};
+
+    } catch (std::invalid_argument & e) {
+        fail();
+        abort();
+    }
 }
 
 

src/libexpr/attr-path.hh

@@ -17,7 +17,7 @@ std::pair<Value *, PosIdx> findAlongAttrPath(
     Value & vIn);
 
 /* Heuristic to find the filename and lineno or a nix value. */
-std::pair<std::string, uint32_t> findPackageFilename(EvalState & state, Value & v, std::string what);
+std::pair<SourcePath, uint32_t> findPackageFilename(EvalState & state, Value & v, std::string what);
 
 std::vector<Symbol> parseAttrPath(EvalState & state, std::string_view s);
 

src/libexpr/eval.cc

@@ -1097,7 +1097,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(path->path.abs());
+        attrs.alloc(sFile).mkString(fmt("/virtual/%d%s", path->accessor->number, path->path.abs()));
         attrs.alloc(sLine).mkInt(pos.line);
         attrs.alloc(sColumn).mkInt(pos.column);
         v.mkAttrs(attrs);

src/libexpr/flake/flake.cc

@@ -431,10 +431,10 @@ LockedFlake lockFlake(
                        flakerefs relative to the parent flake. */
                     auto getInputFlake = [&]()
                     {
-                        if (input.ref->input.isRelative()) {
+                        if (auto relativePath = input.ref->input.isRelative()) {
                             SourcePath inputSourcePath {
                                 overridenSourcePath.accessor,
-                                CanonPath(*input.ref->input.getSourcePath(), *overridenSourcePath.path.parent())
+                                *overridenSourcePath.path.parent() + *relativePath
                             };
                             return readFlake(state, *input.ref, *input.ref, *input.ref, inputSourcePath, inputPath);
                         } else
@@ -621,7 +621,7 @@ LockedFlake lockFlake(
             auto diff = LockFile::diff(oldLockFile, newLockFile);
 
             if (lockFlags.writeLockFile) {
-                if (auto sourcePath = topRef.input.getSourcePath()) {
+                if (auto sourcePath = topRef.input.getAccessor(state.store).first->root().getPhysicalPath()) {
                     if (auto unlockedInput = newLockFile.isUnlocked()) {
                         if (fetchSettings.warnDirty)
                             warn("will not write lock file of flake '%s' because it has an unlocked input ('%s')", topRef, *unlockedInput);
@@ -629,11 +629,13 @@ LockedFlake lockFlake(
                         if (!lockFlags.updateLockFile)
                             throw Error("flake '%s' requires lock file changes but they're not allowed due to '--no-update-lock-file'", topRef);
 
-                        auto relPath = (topRef.subdir == "" ? "" : topRef.subdir + "/") + "flake.lock";
+                        CanonPath flakeDir(*sourcePath);
 
-                        auto path = *sourcePath + "/" + relPath;
+                        auto relPath = flakeDir + "flake.lock";
 
-                        bool lockFileExists = pathExists(path);
+                        auto path = flakeDir + "flake.lock";
+
+                        bool lockFileExists = pathExists(path.abs());
 
                         if (lockFileExists) {
                             auto s = chomp(diff);
@@ -644,7 +646,7 @@ LockedFlake lockFlake(
                         } else
                             warn("creating lock file '%s'", path);
 
-                        newLockFile.write(path);
+                        newLockFile.write(path.abs());
 
                         std::optional<std::string> commitMessage = std::nullopt;
                         if (lockFlags.commitLockFile) {

src/libfetchers/fetchers.cc

@@ -87,8 +87,9 @@ Attrs Input::toAttrs() const
     return attrs;
 }
 
-bool Input::isRelative() const
+std::optional<CanonPath> Input::isRelative() const
 {
+    assert(scheme);
     return scheme->isRelative(*this);
 }
 
@@ -186,12 +187,6 @@ void Input::clone(const Path & destDir) const
     scheme->clone(*this, destDir);
 }
 
-std::optional<Path> Input::getSourcePath() const
-{
-    assert(scheme);
-    return scheme->getSourcePath(*this);
-}
-
 void Input::markChangedFile(
     std::string_view file,
     std::optional<std::string> commitMsg) const
@@ -283,11 +278,6 @@ Input InputScheme::applyOverrides(
     return input;
 }
 
-std::optional<Path> InputScheme::getSourcePath(const Input & input)
-{
-    return {};
-}
-
 void InputScheme::markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg)
 {
     assert(false);

src/libfetchers/fetchers.hh

@@ -54,7 +54,9 @@ public:
        one that contains a commit hash or content hash. */
     bool isLocked() const { return locked; }
 
-    bool isRelative() const;
+    /* Only for relative path flakes, i.e. 'path:./foo', returns the
+       relative path, i.e. './foo'. */
+    std::optional<CanonPath> isRelative() const;
 
     bool hasAllInfo() const;
 
@@ -77,8 +79,6 @@ public:
 
     void clone(const Path & destDir) const;
 
-    std::optional<Path> getSourcePath() const;
-
     void markChangedFile(
         std::string_view file,
         std::optional<std::string> commitMsg) const;
@@ -130,8 +130,6 @@ struct InputScheme
 
     virtual void clone(const Input & input, const Path & destDir);
 
-    virtual std::optional<Path> getSourcePath(const Input & input);
-
     virtual void markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg);
 
     /* Note: the default implementations of fetchToStore() and
@@ -142,8 +140,8 @@ struct InputScheme
 
     virtual std::pair<ref<InputAccessor>, Input> getAccessor(ref<Store> store, const Input & input);
 
-    virtual bool isRelative(const Input & input) const
+    virtual std::optional<CanonPath> isRelative(const Input & input) const
-    { return false; }
+    { return std::nullopt; }
 
     virtual std::optional<std::string> getFingerprint(ref<Store> store, const Input & input) const
     { return std::nullopt; }

src/libfetchers/git.cc

@@ -245,26 +245,17 @@ struct GitInputScheme : InputScheme
         runProgram("git", true, args);
     }
 
-    std::optional<Path> getSourcePath(const Input & input) override
-    {
-        auto url = parseURL(getStrAttr(input.attrs, "url"));
-        if (url.scheme == "file" && !input.getRef() && !input.getRev())
-            return url.path;
-        return {};
-    }
-
     void markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg) override
     {
-        auto sourcePath = getSourcePath(input);
+        auto repoInfo = getRepoInfo(input);
-        assert(sourcePath);
+        assert(repoInfo.isLocal);
-        auto gitDir = ".git";
 
         runProgram("git", true,
-            { "-C", *sourcePath, "--git-dir", gitDir, "add", "--force", "--intent-to-add", "--", std::string(file) });
+            { "-C", repoInfo.url, "--git-dir", repoInfo.gitDir, "add", "--force", "--intent-to-add", "--", std::string(file) });
 
         if (commitMsg)
             runProgram("git", true,
-                { "-C", *sourcePath, "--git-dir", gitDir, "commit", std::string(file), "-m", *commitMsg });
+                { "-C", repoInfo.url, "--git-dir", repoInfo.gitDir, "commit", std::string(file), "-m", *commitMsg });
     }
 
     struct RepoInfo

src/libfetchers/input-accessor.cc

@@ -104,6 +104,11 @@ std::string InputAccessor::showPath(const CanonPath & path)
     return displayPrefix + path.abs() + displaySuffix;
 }
 
+SourcePath InputAccessor::root()
+{
+    return {ref(shared_from_this()), CanonPath::root};
+}
+
 struct FSInputAccessorImpl : FSInputAccessor
 {
     CanonPath root;
@@ -211,6 +216,15 @@ struct FSInputAccessorImpl : FSInputAccessor
     {
         return (bool) allowedPaths;
     }
+
+    std::optional<CanonPath> getPhysicalPath(const CanonPath & path) override
+    {
+        auto absPath = makeAbsPath(path);
+        if (isAllowed(absPath))
+            return absPath;
+        else
+            return std::nullopt;
+    }
 };
 
 ref<FSInputAccessor> makeFSInputAccessor(

src/libfetchers/input-accessor.hh

@@ -9,6 +9,8 @@ namespace nix {
 
 MakeError(RestrictedPathError, Error);
 
+struct SourcePath;
+
 struct InputAccessor : public std::enable_shared_from_this<InputAccessor>
 {
     const size_t number;
@@ -50,6 +52,12 @@ struct InputAccessor : public std::enable_shared_from_this<InputAccessor>
         Sink & sink,
         PathFilter & filter = defaultPathFilter);
 
+    /* Return a corresponding path in the root filesystem, if
+       possible. This is only possible for inputs that are
+       materialized in the root filesystem. */
+    virtual std::optional<CanonPath> getPhysicalPath(const CanonPath & path)
+    { return std::nullopt; }
+
     bool operator == (const InputAccessor & x) const
     {
         return number == x.number;
@@ -63,6 +71,8 @@ struct InputAccessor : public std::enable_shared_from_this<InputAccessor>
     void setPathDisplay(std::string displayPrefix, std::string displaySuffix = "");
 
     virtual std::string showPath(const CanonPath & path);
+
+    SourcePath root();
 };
 
 struct FSInputAccessor : InputAccessor
@@ -128,6 +138,9 @@ struct SourcePath
         PathFilter & filter = defaultPathFilter) const
     { return accessor->dumpPath(path, sink, filter); }
 
+    std::optional<CanonPath> getPhysicalPath() const
+    { return accessor->getPhysicalPath(path); }
+
     std::string to_string() const
     { return accessor->showPath(path); }
 

src/libfetchers/mercurial.cc

@@ -116,26 +116,18 @@ struct MercurialInputScheme : InputScheme
         return res;
     }
 
-    std::optional<Path> getSourcePath(const Input & input) override
-    {
-        auto url = parseURL(getStrAttr(input.attrs, "url"));
-        if (url.scheme == "file" && !input.getRef() && !input.getRev())
-            return url.path;
-        return {};
-    }
-
     void markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg) override
     {
-        auto sourcePath = getSourcePath(input);
+        auto [isLocal, path] = getActualUrl(input);
-        assert(sourcePath);
+        assert(isLocal);
 
         // FIXME: shut up if file is already tracked.
         runHg(
-            { "add", *sourcePath + "/" + std::string(file) });
+            { "add", path + "/" + file });
 
         if (commitMsg)
             runHg(
-                { "commit", *sourcePath + "/" + std::string(file), "-m", *commitMsg });
+                { "commit", path + "/" + file, "-m", *commitMsg });
     }
 
     std::pair<bool, std::string> getActualUrl(const Input & input) const

src/libfetchers/path.cc

@@ -66,9 +66,13 @@ struct PathInputScheme : InputScheme
         };
     }
 
-    bool isRelative(const Input & input) const override
+    std::optional<CanonPath> isRelative(const Input & input) const override
     {
-        return !hasPrefix(*input.getSourcePath(), "/");
+        auto path = getStrAttr(input.attrs, "path");
+        if (hasPrefix(path, "/"))
+            return std::nullopt;
+        else
+            return CanonPath(path);
     }
 
     bool hasAllInfo(const Input & input) override
@@ -76,11 +80,6 @@ struct PathInputScheme : InputScheme
         return true;
     }
 
-    std::optional<Path> getSourcePath(const Input & input) override
-    {
-        return getStrAttr(input.attrs, "path");
-    }
-
     void markChangedFile(const Input & input, std::string_view file, std::optional<std::string> commitMsg) override
     {
         // nothing to do

src/nix/develop.cc

@@ -558,7 +558,9 @@ struct CmdDevelop : Common, MixEnvironment
             // chdir if installable is a flake of type git+file or path
             auto installableFlake = std::dynamic_pointer_cast<InstallableFlake>(installable);
             if (installableFlake) {
-                auto sourcePath = installableFlake->getLockedFlake()->flake.resolvedRef.input.getSourcePath();
+                auto sourcePath = installableFlake->getLockedFlake()
+                    ->flake.resolvedRef.input.getAccessor(store).first
+                    ->root().getPhysicalPath();
                 if (sourcePath) {
                     if (chdir(sourcePath->c_str()) == -1) {
                         throw SysError("chdir to '%s' failed", *sourcePath);