parsePathFlakeRefWithFragment(): Handle 'path?query' without a fragment (backport #12157)

src/libfetchers/fetchers.cc

@@ -66,7 +66,7 @@ Input Input::fromURL(
         }
     }
 
-    throw Error("input '%s' is unsupported", url.url);
+    throw Error("input '%s' is unsupported", url);
 }
 
 Input Input::fromAttrs(const Settings & settings, Attrs && attrs)

src/libfetchers/git.cc

@@ -425,7 +425,7 @@ struct GitInputScheme : InputScheme
         auto url = parseURL(getStrAttr(input.attrs, "url"));
         bool isBareRepository = url.scheme == "file" && !pathExists(url.path + "/.git");
         repoInfo.isLocal = url.scheme == "file" && !forceHttp && !isBareRepository;
-        repoInfo.url = repoInfo.isLocal ? url.path : url.base;
+        repoInfo.url = repoInfo.isLocal ? url.path : url.to_string();
 
         // If this is a local directory and no ref or revision is
         // given, then allow the use of an unclean working tree.

src/libfetchers/github.cc

@@ -50,7 +50,7 @@ struct GitArchiveInputScheme : InputScheme
             else if (std::regex_match(path[2], refRegex))
                 ref = path[2];
             else
-                throw BadURL("in URL '%s', '%s' is not a commit hash or branch/tag name", url.url, path[2]);
+                throw BadURL("in URL '%s', '%s' is not a commit hash or branch/tag name", url, path[2]);
         } else if (size > 3) {
             std::string rs;
             for (auto i = std::next(path.begin(), 2); i != path.end(); i++) {
@@ -63,34 +63,34 @@ struct GitArchiveInputScheme : InputScheme
             if (std::regex_match(rs, refRegex)) {
                 ref = rs;
             } else {
-                throw BadURL("in URL '%s', '%s' is not a branch/tag name", url.url, rs);
+                throw BadURL("in URL '%s', '%s' is not a branch/tag name", url, rs);
             }
         } else if (size < 2)
-            throw BadURL("URL '%s' is invalid", url.url);
+            throw BadURL("URL '%s' is invalid", url);
 
         for (auto &[name, value] : url.query) {
             if (name == "rev") {
                 if (rev)
-                    throw BadURL("URL '%s' contains multiple commit hashes", url.url);
+                    throw BadURL("URL '%s' contains multiple commit hashes", url);
                 rev = Hash::parseAny(value, HashAlgorithm::SHA1);
             }
             else if (name == "ref") {
                 if (!std::regex_match(value, refRegex))
-                    throw BadURL("URL '%s' contains an invalid branch/tag name", url.url);
+                    throw BadURL("URL '%s' contains an invalid branch/tag name", url);
                 if (ref)
-                    throw BadURL("URL '%s' contains multiple branch/tag names", url.url);
+                    throw BadURL("URL '%s' contains multiple branch/tag names", url);
                 ref = value;
             }
             else if (name == "host") {
                 if (!std::regex_match(value, hostRegex))
-                    throw BadURL("URL '%s' contains an invalid instance host", url.url);
+                    throw BadURL("URL '%s' contains an invalid instance host", url);
                 host_url = value;
             }
             // FIXME: barf on unsupported attributes
         }
 
         if (ref && rev)
-            throw BadURL("URL '%s' contains both a commit hash and a branch/tag name %s %s", url.url, *ref, rev->gitRev());
+            throw BadURL("URL '%s' contains both a commit hash and a branch/tag name %s %s", url, *ref, rev->gitRev());
 
         Input input{settings};
         input.attrs.insert_or_assign("type", std::string { schemeName() });

src/libfetchers/indirect.cc

@@ -26,16 +26,16 @@ struct IndirectInputScheme : InputScheme
             else if (std::regex_match(path[1], refRegex))
                 ref = path[1];
             else
-                throw BadURL("in flake URL '%s', '%s' is not a commit hash or branch/tag name", url.url, path[1]);
+                throw BadURL("in flake URL '%s', '%s' is not a commit hash or branch/tag name", url, path[1]);
         } else if (path.size() == 3) {
             if (!std::regex_match(path[1], refRegex))
-                throw BadURL("in flake URL '%s', '%s' is not a branch/tag name", url.url, path[1]);
+                throw BadURL("in flake URL '%s', '%s' is not a branch/tag name", url, path[1]);
             ref = path[1];
             if (!std::regex_match(path[2], revRegex))
-                throw BadURL("in flake URL '%s', '%s' is not a commit hash", url.url, path[2]);
+                throw BadURL("in flake URL '%s', '%s' is not a commit hash", url, path[2]);
             rev = Hash::parseAny(path[2], HashAlgorithm::SHA1);
         } else
-            throw BadURL("GitHub URL '%s' is invalid", url.url);
+            throw BadURL("GitHub URL '%s' is invalid", url);
 
         std::string id = path[0];
         if (!std::regex_match(id, flakeRegex))

src/libfetchers/mercurial.cc

@@ -161,7 +161,7 @@ struct MercurialInputScheme : InputScheme
     {
         auto url = parseURL(getStrAttr(input.attrs, "url"));
         bool isLocal = url.scheme == "file";
-        return {isLocal, isLocal ? url.path : url.base};
+        return {isLocal, isLocal ? url.path : url.to_string()};
     }
 
     StorePath fetchToStore(ref<Store> store, Input & input) const

src/libfetchers/path.cc

@@ -14,7 +14,7 @@ struct PathInputScheme : InputScheme
         if (url.scheme != "path") return {};
 
         if (url.authority && *url.authority != "")
-            throw Error("path URL '%s' should not have an authority ('%s')", url.url, *url.authority);
+            throw Error("path URL '%s' should not have an authority ('%s')", url, *url.authority);
 
         Input input{settings};
         input.attrs.insert_or_assign("type", "path");
@@ -27,10 +27,10 @@ struct PathInputScheme : InputScheme
                 if (auto n = string2Int<uint64_t>(value))
                     input.attrs.insert_or_assign(name, *n);
                 else
-                    throw Error("path URL '%s' has invalid parameter '%s'", url.to_string(), name);
+                    throw Error("path URL '%s' has invalid parameter '%s'", url, name);
             }
             else
-                throw Error("path URL '%s' has unsupported parameter '%s'", url.to_string(), name);
+                throw Error("path URL '%s' has unsupported parameter '%s'", url, name);
 
         return input;
     }

src/libflake-tests/flakeref.cc

@@ -7,18 +7,60 @@ namespace nix {
 
 /* ----------- tests for flake/flakeref.hh --------------------------------------------------*/
 
-    /* ----------------------------------------------------------------------------
-     * to_string
-     * --------------------------------------------------------------------------*/
+    TEST(parseFlakeRef, path) {
+        experimentalFeatureSettings.experimentalFeatures.get().insert(Xp::Flakes);
+
+        fetchers::Settings fetchSettings;
+
+        {
+            auto s = "/foo/bar";
+            auto flakeref = parseFlakeRef(fetchSettings, s);
+            ASSERT_EQ(flakeref.to_string(), "path:/foo/bar");
+        }
+
+        {
+            auto s = "/foo/bar?revCount=123&rev=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa";
+            auto flakeref = parseFlakeRef(fetchSettings, s);
+            ASSERT_EQ(flakeref.to_string(), "path:/foo/bar?rev=aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa&revCount=123");
+        }
+
+        {
+            auto s = "/foo/bar?xyzzy=123";
+            EXPECT_THROW(
+                parseFlakeRef(fetchSettings, s),
+                Error);
+        }
+
+        {
+            auto s = "/foo/bar#bla";
+            EXPECT_THROW(
+                parseFlakeRef(fetchSettings, s),
+                Error);
+        }
+
+        {
+            auto s = "/foo/bar#bla";
+            auto [flakeref, fragment] = parseFlakeRefWithFragment(fetchSettings, s);
+            ASSERT_EQ(flakeref.to_string(), "path:/foo/bar");
+            ASSERT_EQ(fragment, "bla");
+        }
+
+        {
+            auto s = "/foo/bar?revCount=123#bla";
+            auto [flakeref, fragment] = parseFlakeRefWithFragment(fetchSettings, s);
+            ASSERT_EQ(flakeref.to_string(), "path:/foo/bar?revCount=123");
+            ASSERT_EQ(fragment, "bla");
+        }
+    }
 
     TEST(to_string, doesntReencodeUrl) {
         fetchers::Settings fetchSettings;
         auto s = "http://localhost:8181/test/+3d.tar.gz";
         auto flakeref = parseFlakeRef(fetchSettings, s);
-        auto parsed = flakeref.to_string();
+        auto unparsed = flakeref.to_string();
         auto expected = "http://localhost:8181/test/%2B3d.tar.gz";
 
-        ASSERT_EQ(parsed, expected);
+        ASSERT_EQ(unparsed, expected);
     }
 
 }

src/libflake/flake/flakeref.cc

@@ -67,30 +67,37 @@ std::optional<FlakeRef> maybeParseFlakeRef(
     }
 }
 
+static std::pair<FlakeRef, std::string> fromParsedURL(
+    const fetchers::Settings & fetchSettings,
+    ParsedURL && parsedURL,
+    bool isFlake)
+{
+    auto dir = getOr(parsedURL.query, "dir", "");
+    parsedURL.query.erase("dir");
+
+    std::string fragment;
+    std::swap(fragment, parsedURL.fragment);
+
+    return {FlakeRef(fetchers::Input::fromURL(fetchSettings, parsedURL, isFlake), dir), fragment};
+}
+
 std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
     const fetchers::Settings & fetchSettings,
     const std::string & url,
     const std::optional<Path> & baseDir,
     bool allowMissing,
     bool isFlake)
 {
-    std::string path = url;
-    std::string fragment = "";
-    std::map<std::string, std::string> query;
-    auto pathEnd = url.find_first_of("#?");
-    auto fragmentStart = pathEnd;
-    if (pathEnd != std::string::npos && url[pathEnd] == '?') {
-        fragmentStart = url.find("#");
-    }
-    if (pathEnd != std::string::npos) {
-        path = url.substr(0, pathEnd);
-    }
-    if (fragmentStart != std::string::npos) {
-        fragment = percentDecode(url.substr(fragmentStart+1));
-    }
-    if (pathEnd != std::string::npos && fragmentStart != std::string::npos && url[pathEnd] == '?') {
-        query = decodeQuery(url.substr(pathEnd+1, fragmentStart-pathEnd-1));
-    }
+    static std::regex pathFlakeRegex(
+        R"(([^?#]*)(\?([^#]*))?(#(.*))?)",
+        std::regex::ECMAScript);
+
+    std::smatch match;
+    auto succeeds = std::regex_match(url, match, pathFlakeRegex);
+    assert(succeeds);
+    auto path = match[1].str();
+    auto query = decodeQuery(match[3]);
+    auto fragment = percentDecode(match[5].str());
 
     if (baseDir) {
         /* Check if 'url' is a path (either absolute or relative
@@ -144,15 +151,12 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
 
             while (flakeRoot != "/") {
                 if (pathExists(flakeRoot + "/.git")) {
-                    auto base = std::string("git+file://") + flakeRoot;
-
                     auto parsedURL = ParsedURL{
-                        .url = base, // FIXME
-                        .base = base,
                         .scheme = "git+file",
                         .authority = "",
                         .path = flakeRoot,
                         .query = query,
+                        .fragment = fragment,
                     };
 
                     if (subdir != "") {
@@ -164,9 +168,7 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
                     if (pathExists(flakeRoot + "/.git/shallow"))
                         parsedURL.query.insert_or_assign("shallow", "1");
 
-                    return std::make_pair(
-                        FlakeRef(fetchers::Input::fromURL(fetchSettings, parsedURL), getOr(parsedURL.query, "dir", "")),
-                        fragment);
+                    return fromParsedURL(fetchSettings, std::move(parsedURL), isFlake);
                 }
 
                 subdir = std::string(baseNameOf(flakeRoot)) + (subdir.empty() ? "" : "/" + subdir);
@@ -180,16 +182,19 @@ std::pair<FlakeRef, std::string> parsePathFlakeRefWithFragment(
         path = canonPath(path + "/" + getOr(query, "dir", ""));
     }
 
-    fetchers::Attrs attrs;
-    attrs.insert_or_assign("type", "path");
-    attrs.insert_or_assign("path", path);
-
-    return std::make_pair(FlakeRef(fetchers::Input::fromAttrs(fetchSettings, std::move(attrs)), ""), fragment);
-};
-
+    return fromParsedURL(fetchSettings, {
+        .scheme = "path",
+        .authority = "",
+        .path = path,
+        .query = query,
+        .fragment = fragment
+    }, isFlake);
+}
 
-/* Check if 'url' is a flake ID. This is an abbreviated syntax for
-    'flake:<flake-id>?ref=<ref>&rev=<rev>'. */
+/**
+ * Check if `url` is a flake ID. This is an abbreviated syntax for
+ * `flake:<flake-id>?ref=<ref>&rev=<rev>`.
+ */
 static std::optional<std::pair<FlakeRef, std::string>> parseFlakeIdRef(
     const fetchers::Settings & fetchSettings,
     const std::string & url,
@@ -205,8 +210,6 @@ static std::optional<std::pair<FlakeRef, std::string>> parseFlakeIdRef(
 
     if (std::regex_match(url, match, flakeRegex)) {
         auto parsedURL = ParsedURL{
-            .url = url,
-            .base = "flake:" + match.str(1),
             .scheme = "flake",
             .authority = "",
             .path = match[1],
@@ -227,22 +230,11 @@ std::optional<std::pair<FlakeRef, std::string>> parseURLFlakeRef(
     bool isFlake
 )
 {
-    ParsedURL parsedURL;
     try {
-        parsedURL = parseURL(url);
+        return fromParsedURL(fetchSettings, parseURL(url), isFlake);
     } catch (BadURL &) {
         return std::nullopt;
     }
-
-    std::string fragment;
-    std::swap(fragment, parsedURL.fragment);
-
-    auto input = fetchers::Input::fromURL(fetchSettings, parsedURL, isFlake);
-    input.parent = baseDir;
-
-    return std::make_pair(
-        FlakeRef(std::move(input), getOr(parsedURL.query, "dir", "")),
-        fragment);
 }
 
 std::pair<FlakeRef, std::string> parseFlakeRefWithFragment(