From 8b4a89f4bb8a8fc9c51521fa4383e0a6950fbda0 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 19 Nov 2024 16:50:13 +0100 Subject: [PATCH 1/9] Clean up flakeref parsing This factors out some commonality in calling fromURL() and handling the "dir" parameter into a fromParsedURL() helper function. (cherry picked from commit 850281908cd65b7ccfdfe17b1e4a43f8ec59ef9a) --- src/libflake/flake/flakeref.cc | 43 ++++++++++++++++++---------------- 1 file changed, 23 insertions(+), 20 deletions(-) diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index 01fe747f9..ed47ad737 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -67,6 +67,20 @@ std::optional maybeParseFlakeRef( } } +static std::pair 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 std::make_pair(FlakeRef(fetchers::Input::fromURL(fetchSettings, parsedURL, isFlake), dir), fragment); +} + std::pair parsePathFlakeRefWithFragment( const fetchers::Settings & fetchSettings, const std::string & url, @@ -89,7 +103,7 @@ std::pair parsePathFlakeRefWithFragment( 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)); + query = decodeQuery(url.substr(pathEnd + 1, fragmentStart - pathEnd - 1)); } if (baseDir) { @@ -153,6 +167,7 @@ std::pair parsePathFlakeRefWithFragment( .authority = "", .path = flakeRoot, .query = query, + .fragment = fragment, }; if (subdir != "") { @@ -164,9 +179,7 @@ std::pair 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); @@ -185,11 +198,12 @@ std::pair parsePathFlakeRefWithFragment( attrs.insert_or_assign("path", path); return std::make_pair(FlakeRef(fetchers::Input::fromAttrs(fetchSettings, std::move(attrs)), ""), fragment); -}; +} - -/* Check if 'url' is a flake ID. This is an abbreviated syntax for - 'flake:?ref=&rev='. */ +/** + * Check if `url` is a flake ID. This is an abbreviated syntax for + * `flake:?ref=&rev=`. + */ static std::optional> parseFlakeIdRef( const fetchers::Settings & fetchSettings, const std::string & url, @@ -227,22 +241,11 @@ std::optional> 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 parseFlakeRefWithFragment( From aa5246bfe3900d18b79df148c1c33fc0fc55c87b Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Fri, 22 Nov 2024 09:14:01 +0100 Subject: [PATCH 2/9] Drop std::make_pair MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Co-authored-by: Jörg Thalheim (cherry picked from commit ebb19cc1cdc2f04c291e2e8e34f434e5defa5bbd) --- src/libflake/flake/flakeref.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index ed47ad737..cdcdcf87f 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -78,7 +78,7 @@ static std::pair fromParsedURL( std::string fragment; std::swap(fragment, parsedURL.fragment); - return std::make_pair(FlakeRef(fetchers::Input::fromURL(fetchSettings, parsedURL, isFlake), dir), fragment); + return {FlakeRef(fetchers::Input::fromURL(fetchSettings, parsedURL, isFlake), dir), fragment}; } std::pair parsePathFlakeRefWithFragment( From 8856b5f2ca0352d128884ba06195788b79036354 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 Jan 2025 14:45:58 +0100 Subject: [PATCH 3/9] ParsedURL: Remove url field This prevents a 'url' field that is out of sync with the other fields. You can use to_string() to get the full URL. (cherry picked from commit f705ce7f9a95bc37df236d88eef1dbd9e2ae5f31) --- src/libfetchers/fetchers.cc | 2 +- src/libfetchers/github.cc | 16 ++++++++-------- src/libfetchers/indirect.cc | 8 ++++---- src/libfetchers/path.cc | 6 +++--- src/libflake/flake/flakeref.cc | 2 -- src/libutil-tests/url.cc | 22 ---------------------- src/libutil/url.cc | 7 ++++++- src/libutil/url.hh | 5 +++-- 8 files changed, 25 insertions(+), 43 deletions(-) diff --git a/src/libfetchers/fetchers.cc b/src/libfetchers/fetchers.cc index 5c06a6bcb..d69a6c5ad 100644 --- a/src/libfetchers/fetchers.cc +++ b/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) diff --git a/src/libfetchers/github.cc b/src/libfetchers/github.cc index 308cff33a..185941988 100644 --- a/src/libfetchers/github.cc +++ b/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() }); diff --git a/src/libfetchers/indirect.cc b/src/libfetchers/indirect.cc index 2e5cd82c7..0e1b86711 100644 --- a/src/libfetchers/indirect.cc +++ b/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)) diff --git a/src/libfetchers/path.cc b/src/libfetchers/path.cc index 246b68c3a..f628e042c 100644 --- a/src/libfetchers/path.cc +++ b/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(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; } diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index cdcdcf87f..4f5cb0d30 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -161,7 +161,6 @@ std::pair parsePathFlakeRefWithFragment( auto base = std::string("git+file://") + flakeRoot; auto parsedURL = ParsedURL{ - .url = base, // FIXME .base = base, .scheme = "git+file", .authority = "", @@ -219,7 +218,6 @@ static std::optional> parseFlakeIdRef( if (std::regex_match(url, match, flakeRegex)) { auto parsedURL = ParsedURL{ - .url = url, .base = "flake:" + match.str(1), .scheme = "flake", .authority = "", diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index 7d08f467e..6ffae4f3e 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -20,23 +20,11 @@ namespace nix { } - std::ostream& operator<<(std::ostream& os, const ParsedURL& p) { - return os << "\n" - << "url: " << p.url << "\n" - << "base: " << p.base << "\n" - << "scheme: " << p.scheme << "\n" - << "authority: " << p.authority.value() << "\n" - << "path: " << p.path << "\n" - << "query: " << print_map(p.query) << "\n" - << "fragment: " << p.fragment << "\n"; - } - TEST(parseURL, parsesSimpleHttpUrl) { auto s = "http://www.example.org/file.tar.gz"; auto parsed = parseURL(s); ParsedURL expected { - .url = "http://www.example.org/file.tar.gz", .base = "http://www.example.org/file.tar.gz", .scheme = "http", .authority = "www.example.org", @@ -53,7 +41,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "https://www.example.org/file.tar.gz", .base = "https://www.example.org/file.tar.gz", .scheme = "https", .authority = "www.example.org", @@ -70,7 +57,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "https://www.example.org/file.tar.gz", .base = "https://www.example.org/file.tar.gz", .scheme = "https", .authority = "www.example.org", @@ -87,7 +73,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://www.example.org/file.tar.gz", .base = "http://www.example.org/file.tar.gz", .scheme = "http", .authority = "www.example.org", @@ -104,7 +89,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "file+https://www.example.org/video.mp4", .base = "https://www.example.org/video.mp4", .scheme = "file+https", .authority = "www.example.org", @@ -126,7 +110,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://127.0.0.1:8080/file.tar.gz", .base = "https://127.0.0.1:8080/file.tar.gz", .scheme = "http", .authority = "127.0.0.1:8080", @@ -143,7 +126,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://[fe80::818c:da4d:8975:415c\%enp0s25]:8080", .base = "http://[fe80::818c:da4d:8975:415c\%enp0s25]:8080", .scheme = "http", .authority = "[fe80::818c:da4d:8975:415c\%enp0s25]:8080", @@ -161,7 +143,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", .base = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", .scheme = "http", .authority = "[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", @@ -185,7 +166,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "http://user:pass@www.example.org/file.tar.gz", .base = "http://user:pass@www.example.org/file.tar.gz", .scheme = "http", .authority = "user:pass@www.example.org:8080", @@ -203,7 +183,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "", .base = "", .scheme = "file", .authority = "", @@ -228,7 +207,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "ftp://ftp.nixos.org/downloads/nixos.iso", .base = "ftp://ftp.nixos.org/downloads/nixos.iso", .scheme = "ftp", .authority = "ftp.nixos.org", diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 9ed49dcbe..c27887932 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -40,7 +40,6 @@ ParsedURL parseURL(const std::string & url) path = "/"; return ParsedURL{ - .url = url, .base = base, .scheme = scheme, .authority = authority, @@ -136,6 +135,12 @@ std::string ParsedURL::to_string() const + (fragment.empty() ? "" : "#" + percentEncode(fragment)); } +std::ostream & operator << (std::ostream & os, const ParsedURL & url) +{ + os << url.to_string(); + return os; +} + bool ParsedURL::operator ==(const ParsedURL & other) const noexcept { return diff --git a/src/libutil/url.hh b/src/libutil/url.hh index 738ee9f82..d85001185 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -7,9 +7,8 @@ namespace nix { struct ParsedURL { - std::string url; /// URL without query/fragment - std::string base; + std::string base; // FIXME: remove std::string scheme; std::optional authority; std::string path; @@ -26,6 +25,8 @@ struct ParsedURL ParsedURL canonicalise(); }; +std::ostream & operator << (std::ostream & os, const ParsedURL & url); + MakeError(BadURL, Error); std::string percentDecode(std::string_view in); From 200e3be41a0df15f9757b550273bf64dc6fcb5c2 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Tue, 7 Jan 2025 14:52:00 +0100 Subject: [PATCH 4/9] ParsedURL: Remove base field (cherry picked from commit 4077aa43a803db33108ce39e799089608707dcbb) --- src/libfetchers/git.cc | 2 +- src/libfetchers/mercurial.cc | 2 +- src/libflake/flake/flakeref.cc | 4 ---- src/libutil-tests/url.cc | 11 ----------- src/libutil/url.cc | 1 - src/libutil/url.hh | 2 -- 6 files changed, 2 insertions(+), 20 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 99d91919e..3b487a763 100644 --- a/src/libfetchers/git.cc +++ b/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. diff --git a/src/libfetchers/mercurial.cc b/src/libfetchers/mercurial.cc index 2c987f79d..c2fd8139c 100644 --- a/src/libfetchers/mercurial.cc +++ b/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, Input & input) const diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index 4f5cb0d30..377c810ac 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -158,10 +158,7 @@ std::pair parsePathFlakeRefWithFragment( while (flakeRoot != "/") { if (pathExists(flakeRoot + "/.git")) { - auto base = std::string("git+file://") + flakeRoot; - auto parsedURL = ParsedURL{ - .base = base, .scheme = "git+file", .authority = "", .path = flakeRoot, @@ -218,7 +215,6 @@ static std::optional> parseFlakeIdRef( if (std::regex_match(url, match, flakeRegex)) { auto parsedURL = ParsedURL{ - .base = "flake:" + match.str(1), .scheme = "flake", .authority = "", .path = match[1], diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index 6ffae4f3e..7e1d2aa15 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -25,7 +25,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://www.example.org/file.tar.gz", .scheme = "http", .authority = "www.example.org", .path = "/file.tar.gz", @@ -41,7 +40,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "https://www.example.org/file.tar.gz", .scheme = "https", .authority = "www.example.org", .path = "/file.tar.gz", @@ -57,7 +55,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "https://www.example.org/file.tar.gz", .scheme = "https", .authority = "www.example.org", .path = "/file.tar.gz", @@ -73,7 +70,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://www.example.org/file.tar.gz", .scheme = "http", .authority = "www.example.org", .path = "/file.tar.gz", @@ -89,7 +85,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "https://www.example.org/video.mp4", .scheme = "file+https", .authority = "www.example.org", .path = "/video.mp4", @@ -110,7 +105,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "https://127.0.0.1:8080/file.tar.gz", .scheme = "http", .authority = "127.0.0.1:8080", .path = "/file.tar.gz", @@ -126,7 +120,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://[fe80::818c:da4d:8975:415c\%enp0s25]:8080", .scheme = "http", .authority = "[fe80::818c:da4d:8975:415c\%enp0s25]:8080", .path = "", @@ -143,7 +136,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", .scheme = "http", .authority = "[2a02:8071:8192:c100:311d:192d:81ac:11ea]:8080", .path = "", @@ -166,7 +158,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "http://user:pass@www.example.org/file.tar.gz", .scheme = "http", .authority = "user:pass@www.example.org:8080", .path = "/file.tar.gz", @@ -183,7 +174,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "", .scheme = "file", .authority = "", .path = "/none/of//your/business", @@ -207,7 +197,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .base = "ftp://ftp.nixos.org/downloads/nixos.iso", .scheme = "ftp", .authority = "ftp.nixos.org", .path = "/downloads/nixos.iso", diff --git a/src/libutil/url.cc b/src/libutil/url.cc index c27887932..0edf8aa6c 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -40,7 +40,6 @@ ParsedURL parseURL(const std::string & url) path = "/"; return ParsedURL{ - .base = base, .scheme = scheme, .authority = authority, .path = percentDecode(path), diff --git a/src/libutil/url.hh b/src/libutil/url.hh index d85001185..2b12f5af2 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -7,8 +7,6 @@ namespace nix { struct ParsedURL { - /// URL without query/fragment - std::string base; // FIXME: remove std::string scheme; std::optional authority; std::string path; From faecb6e306586fd1461813fa5742b7a08e691858 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Wed, 8 Jan 2025 18:38:53 +0100 Subject: [PATCH 5/9] parsePathFlakeRefWithFragment(): Handle 'path?query' without a fragment Commands like `nix flake metadata '.?submodules=1'` ignored the query part of the URL, while `nix build '.?submodules=1#foo'` did work correctly because of the presence of the fragment part. (cherry picked from commit 28caa35a97ed4837a62cb054f66cf02041292cc5) --- src/libflake/flake/flakeref.cc | 27 +++++++------------ tests/functional/flakes/flake-in-submodule.sh | 13 +++++++++ 2 files changed, 23 insertions(+), 17 deletions(-) diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index 377c810ac..182a5570b 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -88,23 +88,16 @@ std::pair parsePathFlakeRefWithFragment( bool allowMissing, bool isFlake) { - std::string path = url; - std::string fragment = ""; - std::map 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 diff --git a/tests/functional/flakes/flake-in-submodule.sh b/tests/functional/flakes/flake-in-submodule.sh index 817f77783..643d729ff 100755 --- a/tests/functional/flakes/flake-in-submodule.sh +++ b/tests/functional/flakes/flake-in-submodule.sh @@ -63,3 +63,16 @@ flakeref=git+file://$rootRepo\?submodules=1\&dir=submodule echo '"foo"' > "$rootRepo"/submodule/sub.nix [[ $(nix eval --json "$flakeref#sub" ) = '"foo"' ]] [[ $(nix flake metadata --json "$flakeref" | jq -r .locked.rev) = null ]] + +# Test that `nix flake metadata` parses `submodule` correctly. +cat > "$rootRepo"/flake.nix < Date: Thu, 9 Jan 2025 12:17:09 +0100 Subject: [PATCH 6/9] parsePathFlakeRefWithFragment(): Handle query params in the non-git case Backported from lazy-trees. (cherry picked from commit 83ff523865f87f61b0b2046bd3bb2d793e09911d) --- src/libflake/flake/flakeref.cc | 12 +++++++----- 1 file changed, 7 insertions(+), 5 deletions(-) diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index 182a5570b..e08278e4e 100644 --- a/src/libflake/flake/flakeref.cc +++ b/src/libflake/flake/flakeref.cc @@ -182,11 +182,13 @@ std::pair 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); } /** From e1178b8a3945322c2e06065fad6babc07e3923d9 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 9 Jan 2025 12:18:16 +0100 Subject: [PATCH 7/9] parsePathFlakeRefWithFragment(): Add unit tests (cherry picked from commit 5f7b535b819fb2ad5f7f05bf73fee7a2e7f7c9c8) --- src/libflake-tests/flakeref.cc | 50 ++++++++++++++++++++++++++++++---- 1 file changed, 45 insertions(+), 5 deletions(-) diff --git a/src/libflake-tests/flakeref.cc b/src/libflake-tests/flakeref.cc index d704a26d3..c7007a7af 100644 --- a/src/libflake-tests/flakeref.cc +++ b/src/libflake-tests/flakeref.cc @@ -7,18 +7,58 @@ namespace nix { /* ----------- tests for flake/flakeref.hh --------------------------------------------------*/ - /* ---------------------------------------------------------------------------- - * to_string - * --------------------------------------------------------------------------*/ + TEST(parseFlakeRef, path) { + 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); } } From 21fe544ed9f99ce575dfe0c2bdc1d0303224a220 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 9 Jan 2025 16:38:33 +0100 Subject: [PATCH 8/9] Remove unused variable (cherry picked from commit 1a38e62a094f332776ab8b60bd3274e762f7b2cb) --- src/libutil/url.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 0edf8aa6c..9648998e9 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -22,7 +22,6 @@ ParsedURL parseURL(const std::string & url) std::smatch match; if (std::regex_match(url, match, uriRegex)) { - auto & base = match[1]; std::string scheme = match[2]; auto authority = match[3].matched ? std::optional(match[3]) : std::nullopt; From ef1e704707451cb3e58e50ce93de4cbc317905fc Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 9 Jan 2025 16:42:37 +0100 Subject: [PATCH 9/9] Attempt to make the FlakeRef test succeed on macOS (cherry picked from commit 3ad0f45e79bc1abc96a7c05d5979552e5f936d47) --- src/libflake-tests/flakeref.cc | 2 ++ src/libutil/config.hh | 1 + 2 files changed, 3 insertions(+) diff --git a/src/libflake-tests/flakeref.cc b/src/libflake-tests/flakeref.cc index c7007a7af..2b1f5124b 100644 --- a/src/libflake-tests/flakeref.cc +++ b/src/libflake-tests/flakeref.cc @@ -8,6 +8,8 @@ namespace nix { /* ----------- tests for flake/flakeref.hh --------------------------------------------------*/ TEST(parseFlakeRef, path) { + experimentalFeatureSettings.experimentalFeatures.get().insert(Xp::Flakes); + fetchers::Settings fetchSettings; { diff --git a/src/libutil/config.hh b/src/libutil/config.hh index c0c59ac68..a34a745a2 100644 --- a/src/libutil/config.hh +++ b/src/libutil/config.hh @@ -260,6 +260,7 @@ public: operator const T &() const { return value; } operator T &() { return value; } const T & get() const { return value; } + T & get() { return value; } template bool operator ==(const U & v2) const { return value == v2; } template