From 7c3463adf4a51b79e9b76329308f2d7bf93dfeeb Mon Sep 17 00:00:00 2001
From: Ignas Anikevicius <240938+aignas@users.noreply.github.com>
Date: Thu, 3 Apr 2025 09:24:17 +0900
Subject: [PATCH] fix(pypi): correctly fallback to pip for git direct URLs

Whilst integrating #2695 I introduced a regression and here I add a test
for that and fix it. The code that was getting the filename from the URL
was too eager and would break if there was a git ref as noted in the
test.

Before this commit and #2695 the code was not handling all of the cases
that are tested now either, so I think now we are in a good place. I am
not sure how we should handle the `git_repository` URLs. Maybe having
`http_archive` and `git_repository` usage would be nice, but I am not sure
how we can introduce it at the moment.

Work towards #2363
---
 python/private/pypi/parse_requirements.bzl |  6 +++
 tests/pypi/extension/extension_tests.bzl   | 50 +++++++++++++++++++++-
 2 files changed, 55 insertions(+), 1 deletion(-)

diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl
index 3280ce8df1..d2014a7eb9 100644
--- a/python/private/pypi/parse_requirements.bzl
+++ b/python/private/pypi/parse_requirements.bzl
@@ -297,6 +297,12 @@ def _add_dists(*, requirement, index_urls, logger = None):
     if requirement.srcs.url:
         url = requirement.srcs.url
         _, _, filename = url.rpartition("/")
+        if "." not in filename:
+            # detected filename has no extension, it might be an sdist ref
+            # TODO @aignas 2025-04-03: should be handled if the following is fixed:
+            # https://github.com/bazel-contrib/rules_python/issues/2363
+            return [], None
+
         direct_url_dist = struct(
             url = url,
             filename = filename,
diff --git a/tests/pypi/extension/extension_tests.bzl b/tests/pypi/extension/extension_tests.bzl
index 3a91c7b108..ab7a1358ad 100644
--- a/tests/pypi/extension/extension_tests.bzl
+++ b/tests/pypi/extension/extension_tests.bzl
@@ -662,6 +662,8 @@ some_pkg==0.0.1 @ example-direct.org/some_pkg-0.0.1-py3-none-any.whl \
 direct_without_sha==0.0.1 @ example-direct.org/direct_without_sha-0.0.1-py3-none-any.whl
 some_other_pkg==0.0.1
 pip_fallback==0.0.1
+direct_sdist_without_sha @ some-archive/any-name.tar.gz
+git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef
 """,
             }[x],
         ),
@@ -672,10 +674,28 @@ pip_fallback==0.0.1
     )
 
     pypi.is_reproducible().equals(False)
-    pypi.exposed_packages().contains_exactly({"pypi": ["direct_without_sha", "pip_fallback", "simple", "some_other_pkg", "some_pkg"]})
+    pypi.exposed_packages().contains_exactly({"pypi": [
+        "direct_sdist_without_sha",
+        "direct_without_sha",
+        "git_dep",
+        "pip_fallback",
+        "simple",
+        "some_other_pkg",
+        "some_pkg",
+    ]})
     pypi.hub_group_map().contains_exactly({"pypi": {}})
     pypi.hub_whl_map().contains_exactly({
         "pypi": {
+            "direct_sdist_without_sha": {
+                "pypi_315_any_name": [
+                    struct(
+                        config_setting = None,
+                        filename = "any-name.tar.gz",
+                        target_platforms = None,
+                        version = "3.15",
+                    ),
+                ],
+            },
             "direct_without_sha": {
                 "pypi_315_direct_without_sha_0_0_1_py3_none_any": [
                     struct(
@@ -686,6 +706,16 @@ pip_fallback==0.0.1
                     ),
                 ],
             },
+            "git_dep": {
+                "pypi_315_git_dep": [
+                    struct(
+                        config_setting = None,
+                        filename = None,
+                        target_platforms = None,
+                        version = "3.15",
+                    ),
+                ],
+            },
             "pip_fallback": {
                 "pypi_315_pip_fallback": [
                     struct(
@@ -737,6 +767,17 @@ pip_fallback==0.0.1
         },
     })
     pypi.whl_libraries().contains_exactly({
+        "pypi_315_any_name": {
+            "dep_template": "@pypi//{name}:{target}",
+            "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"],
+            "extra_pip_args": ["--extra-args-for-sdist-building"],
+            "filename": "any-name.tar.gz",
+            "python_interpreter_target": "unit_test_interpreter_target",
+            "repo": "pypi_315",
+            "requirement": "direct_sdist_without_sha @ some-archive/any-name.tar.gz",
+            "sha256": "",
+            "urls": ["some-archive/any-name.tar.gz"],
+        },
         "pypi_315_direct_without_sha_0_0_1_py3_none_any": {
             "dep_template": "@pypi//{name}:{target}",
             "experimental_target_platforms": ["cp315_linux_aarch64", "cp315_linux_arm", "cp315_linux_ppc", "cp315_linux_s390x", "cp315_linux_x86_64", "cp315_osx_aarch64", "cp315_osx_x86_64", "cp315_windows_x86_64"],
@@ -747,6 +788,13 @@ pip_fallback==0.0.1
             "sha256": "",
             "urls": ["example-direct.org/direct_without_sha-0.0.1-py3-none-any.whl"],
         },
+        "pypi_315_git_dep": {
+            "dep_template": "@pypi//{name}:{target}",
+            "extra_pip_args": ["--extra-args-for-sdist-building"],
+            "python_interpreter_target": "unit_test_interpreter_target",
+            "repo": "pypi_315",
+            "requirement": "git_dep @ git+https://git.server/repo/project@deadbeefdeadbeef",
+        },
         "pypi_315_pip_fallback": {
             "dep_template": "@pypi//{name}:{target}",
             "extra_pip_args": ["--extra-args-for-sdist-building"],