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/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/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/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/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-tests/flakeref.cc b/src/libflake-tests/flakeref.cc index d704a26d3..2b1f5124b 100644 --- a/src/libflake-tests/flakeref.cc +++ b/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); } } diff --git a/src/libflake/flake/flakeref.cc b/src/libflake/flake/flakeref.cc index 01fe747f9..e08278e4e 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 {FlakeRef(fetchers::Input::fromURL(fetchSettings, parsedURL, isFlake), dir), fragment}; +} + std::pair parsePathFlakeRefWithFragment( const fetchers::Settings & fetchSettings, const std::string & url, @@ -74,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 @@ -144,15 +151,12 @@ std::pair 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 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 parsePathFlakeRefWithFragment( path = canonPath(path + "/" + getOr(query, "dir", "")); } - fetchers::Attrs attrs; - attrs.insert_or_assign("type", "path"); - attrs.insert_or_assign("path", path); + return fromParsedURL(fetchSettings, { + .scheme = "path", + .authority = "", + .path = path, + .query = query, + .fragment = fragment + }, isFlake); +} - 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, @@ -205,8 +210,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 = "", .path = match[1], @@ -227,22 +230,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( diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index 7d08f467e..7e1d2aa15 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -20,24 +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", .path = "/file.tar.gz", @@ -53,8 +40,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", .path = "/file.tar.gz", @@ -70,8 +55,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", .path = "/file.tar.gz", @@ -87,8 +70,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", .path = "/file.tar.gz", @@ -104,8 +85,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", .path = "/video.mp4", @@ -126,8 +105,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", .path = "/file.tar.gz", @@ -143,8 +120,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", .path = "", @@ -161,8 +136,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", .path = "", @@ -185,8 +158,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", .path = "/file.tar.gz", @@ -203,8 +174,6 @@ namespace nix { auto parsed = parseURL(s); ParsedURL expected { - .url = "", - .base = "", .scheme = "file", .authority = "", .path = "/none/of//your/business", @@ -228,8 +197,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", .path = "/downloads/nixos.iso", 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 diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 9ed49dcbe..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; @@ -40,8 +39,6 @@ ParsedURL parseURL(const std::string & url) path = "/"; return ParsedURL{ - .url = url, - .base = base, .scheme = scheme, .authority = authority, .path = percentDecode(path), @@ -136,6 +133,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..2b12f5af2 100644 --- a/src/libutil/url.hh +++ b/src/libutil/url.hh @@ -7,9 +7,6 @@ namespace nix { struct ParsedURL { - std::string url; - /// URL without query/fragment - std::string base; std::string scheme; std::optional authority; std::string path; @@ -26,6 +23,8 @@ struct ParsedURL ParsedURL canonicalise(); }; +std::ostream & operator << (std::ostream & os, const ParsedURL & url); + MakeError(BadURL, Error); std::string percentDecode(std::string_view in); 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 <