Merge pull request #14492 from NixOS/fix-14429

fetchGit: Drop `git+` from the `url` attribute

Author: edolstra

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 <gtest/gtest.h>
+
+#include <string>
+
+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<InputFromAttrsTestCase>, 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://[email protected]/NixOS/nixpkgs")},
+                    {"type", Attr("git")},
+                },
+            .expectedUrl = "git+ssh://[email protected]/NixOS/nixpkgs",
+            .description = "strips_git_plus_prefix",
+            .expectedAttrs =
+                {
+                    {"url", Attr("ssh://[email protected]/NixOS/nixpkgs")},
+                    {"type", Attr("git")},
+                },
+        }),
+    [](const ::testing::TestParamInfo<InputFromAttrsTestCase> & info) { return info.param.description; });
+
+} // namespace nix

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',
 )

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;

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",

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. [email protected]: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. `[email protected]: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.

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<std::vector<std::string>>(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