From 40f600644dbfc18d92c63a1be356bc1fc3664738 Mon Sep 17 00:00:00 2001 From: Eelco Dolstra Date: Thu, 6 Nov 2025 16:34:19 +0100 Subject: [PATCH 1/2] fetchGit: Drop `git+` from the `url` attribute This was already dropped in `inputFromURL()`, but not in `inputFromAttrs()`. Now it's done in `fixGitURL()`, which is used by both. In principle, `git+` shouldn't be used in the `url` attribute, since we already know that it's a Git URL. But since it currently works, we don't want to break it. Fixes #14429. --- src/libfetchers/git.cc | 2 -- src/libutil-tests/url.cc | 4 ++-- src/libutil/include/nix/util/url.hh | 11 +++++++---- src/libutil/url.cc | 16 +++++++++------- 4 files changed, 18 insertions(+), 15 deletions(-) diff --git a/src/libfetchers/git.cc b/src/libfetchers/git.cc index 710d2f315..7247442ee 100644 --- a/src/libfetchers/git.cc +++ b/src/libfetchers/git.cc @@ -168,8 +168,6 @@ struct GitInputScheme : InputScheme return {}; auto url2(url); - if (hasPrefix(url2.scheme, "git+")) - url2.scheme = std::string(url2.scheme, 4); url2.query.clear(); Attrs attrs; diff --git a/src/libutil-tests/url.cc b/src/libutil-tests/url.cc index cd6816096..356a134dc 100644 --- a/src/libutil-tests/url.cc +++ b/src/libutil-tests/url.cc @@ -35,10 +35,10 @@ INSTANTIATE_TEST_SUITE_P( // Already proper URL with git+ssh FixGitURLParam{ .input = "git+ssh://user@domain:1234/path", - .expected = "git+ssh://user@domain:1234/path", + .expected = "ssh://user@domain:1234/path", .parsed = ParsedURL{ - .scheme = "git+ssh", + .scheme = "ssh", .authority = ParsedURL::Authority{ .host = "domain", diff --git a/src/libutil/include/nix/util/url.hh b/src/libutil/include/nix/util/url.hh index 1fc8c3f2b..55c475df6 100644 --- a/src/libutil/include/nix/util/url.hh +++ b/src/libutil/include/nix/util/url.hh @@ -330,10 +330,13 @@ struct ParsedUrlScheme ParsedUrlScheme parseUrlScheme(std::string_view scheme); -/* Detects scp-style uris (e.g. git@github.com:NixOS/nix) and fixes - them by removing the `:` and assuming a scheme of `ssh://`. Also - changes absolute paths into file:// URLs. */ -ParsedURL fixGitURL(const std::string & url); +/** + * Detects scp-style uris (e.g. `git@github.com:NixOS/nix`) and fixes + * them by removing the `:` and assuming a scheme of `ssh://`. Also + * drops `git+` from the scheme (e.g. `git+https://` to `https://`) + * and changes absolute paths into `file://` URLs. + */ +ParsedURL fixGitURL(std::string url); /** * Whether a string is valid as RFC 3986 scheme name. diff --git a/src/libutil/url.cc b/src/libutil/url.cc index 538792463..72042901c 100644 --- a/src/libutil/url.cc +++ b/src/libutil/url.cc @@ -409,21 +409,23 @@ ParsedUrlScheme parseUrlScheme(std::string_view scheme) }; } -ParsedURL fixGitURL(const std::string & url) +ParsedURL fixGitURL(std::string url) { std::regex scpRegex("([^/]*)@(.*):(.*)"); if (!hasPrefix(url, "/") && std::regex_match(url, scpRegex)) - return parseURL(std::regex_replace(url, scpRegex, "ssh://$1@$2/$3")); - if (hasPrefix(url, "file:")) - return parseURL(url); - if (url.find("://") == std::string::npos) { + url = std::regex_replace(url, scpRegex, "ssh://$1@$2/$3"); + if (!hasPrefix(url, "file:") && !hasPrefix(url, "git+file:") && url.find("://") == std::string::npos) return ParsedURL{ .scheme = "file", .authority = ParsedURL::Authority{}, .path = splitString>(url, "/"), }; - } - return parseURL(url); + auto parsed = parseURL(url); + // Drop the superfluous "git+" from the scheme. + auto scheme = parseUrlScheme(parsed.scheme); + if (scheme.application == "git") + parsed.scheme = scheme.transport; + return parsed; } // https://www.rfc-editor.org/rfc/rfc3986#section-3.1 From d6fc64ac3876dd144a7ca4604c0a371d2c43f624 Mon Sep 17 00:00:00 2001 From: Sergei Zimmerman Date: Sat, 8 Nov 2025 00:07:50 +0300 Subject: [PATCH 2/2] libfetchers-tests: Add InputFromAttrsTest for #14429 Previous commit fixed an issue. This commit adds a test to validate that. --- src/libfetchers-tests/input.cc | 61 +++++++++++++++++++++++++++++++ src/libfetchers-tests/meson.build | 1 + 2 files changed, 62 insertions(+) create mode 100644 src/libfetchers-tests/input.cc diff --git a/src/libfetchers-tests/input.cc b/src/libfetchers-tests/input.cc new file mode 100644 index 000000000..faff55f2c --- /dev/null +++ b/src/libfetchers-tests/input.cc @@ -0,0 +1,61 @@ +#include "nix/fetchers/fetch-settings.hh" +#include "nix/fetchers/attrs.hh" +#include "nix/fetchers/fetchers.hh" + +#include + +#include + +namespace nix { + +using fetchers::Attr; + +struct InputFromAttrsTestCase +{ + fetchers::Attrs attrs; + std::string expectedUrl; + std::string description; + fetchers::Attrs expectedAttrs = attrs; +}; + +class InputFromAttrsTest : public ::testing::WithParamInterface, public ::testing::Test +{}; + +TEST_P(InputFromAttrsTest, attrsAreCorrectAndRoundTrips) +{ + fetchers::Settings fetchSettings; + + const auto & testCase = GetParam(); + + auto input = fetchers::Input::fromAttrs(fetchSettings, fetchers::Attrs(testCase.attrs)); + + EXPECT_EQ(input.toAttrs(), testCase.expectedAttrs); + EXPECT_EQ(input.toURLString(), testCase.expectedUrl); + + auto input2 = fetchers::Input::fromAttrs(fetchSettings, input.toAttrs()); + EXPECT_EQ(input, input2); + EXPECT_EQ(input.toAttrs(), input2.toAttrs()); +} + +INSTANTIATE_TEST_SUITE_P( + InputFromAttrs, + InputFromAttrsTest, + ::testing::Values( + // Test for issue #14429. + InputFromAttrsTestCase{ + .attrs = + { + {"url", Attr("git+ssh://git@github.com/NixOS/nixpkgs")}, + {"type", Attr("git")}, + }, + .expectedUrl = "git+ssh://git@github.com/NixOS/nixpkgs", + .description = "strips_git_plus_prefix", + .expectedAttrs = + { + {"url", Attr("ssh://git@github.com/NixOS/nixpkgs")}, + {"type", Attr("git")}, + }, + }), + [](const ::testing::TestParamInfo & info) { return info.param.description; }); + +} // namespace nix diff --git a/src/libfetchers-tests/meson.build b/src/libfetchers-tests/meson.build index a18f64d79..6bccdb05c 100644 --- a/src/libfetchers-tests/meson.build +++ b/src/libfetchers-tests/meson.build @@ -42,6 +42,7 @@ sources = files( 'access-tokens.cc', 'git-utils.cc', 'git.cc', + 'input.cc', 'nix_api_fetchers.cc', 'public-key.cc', )