Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

ParsedURL: Remove 'url' and 'base' fields #12149

Merged
merged 2 commits into from
Jan 7, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion src/libfetchers/fetchers.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
2 changes: 1 addition & 1 deletion src/libfetchers/git.cc
Original file line number Diff line number Diff line change
Expand Up @@ -426,7 +426,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.
Expand Down
16 changes: 8 additions & 8 deletions src/libfetchers/github.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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++) {
Expand All @@ -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() });
Expand Down
8 changes: 4 additions & 4 deletions src/libfetchers/indirect.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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))
Expand Down
2 changes: 1 addition & 1 deletion src/libfetchers/mercurial.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
6 changes: 3 additions & 3 deletions src/libfetchers/path.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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");
Expand All @@ -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;
}
Expand Down
6 changes: 0 additions & 6 deletions src/libflake/flake/flakeref.cc
Original file line number Diff line number Diff line change
Expand Up @@ -159,11 +159,7 @@ 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,
Expand Down Expand Up @@ -220,8 +216,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],
Expand Down
33 changes: 0 additions & 33 deletions src/libutil-tests/url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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",
Expand All @@ -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 = "",
Expand All @@ -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 = "",
Expand All @@ -185,8 +158,6 @@ namespace nix {
auto parsed = parseURL(s);

ParsedURL expected {
.url = "http://user:[email protected]/file.tar.gz",
.base = "http://user:[email protected]/file.tar.gz",
.scheme = "http",
.authority = "user:[email protected]:8080",
.path = "/file.tar.gz",
Expand All @@ -203,8 +174,6 @@ namespace nix {
auto parsed = parseURL(s);

ParsedURL expected {
.url = "",
.base = "",
.scheme = "file",
.authority = "",
.path = "/none/of//your/business",
Expand All @@ -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",
Expand Down
8 changes: 6 additions & 2 deletions src/libutil/url.cc
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,6 @@ ParsedURL parseURL(const std::string & url)
path = "/";

return ParsedURL{
.url = url,
.base = base,
.scheme = scheme,
.authority = authority,
.path = percentDecode(path),
Expand Down Expand Up @@ -136,6 +134,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
Expand Down
5 changes: 2 additions & 3 deletions src/libutil/url.hh
Original file line number Diff line number Diff line change
Expand Up @@ -7,9 +7,6 @@ namespace nix {

struct ParsedURL
{
std::string url;
/// URL without query/fragment
std::string base;
std::string scheme;
std::optional<std::string> authority;
std::string path;
Expand All @@ -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);
Expand Down
Loading