Trailing slashes not removed from pip index URLs in environment variables before concatenation.

[WillMorrisonEnspi]

🐞 bug report

Affected Rule

The simpleapi_download function called by the pip.parse repo rule.

Is this a regression?

No

Description

Environment variables are substituted after stripping trailing / characters, meaning that an environment variable containing a trailing slash in experimental_index_url will not have that trailing slash stripped. The index URL is concatenated with packages and results in paths with double slashes where there should be single ones. This in turn leads to 404 errors when attempting to fetch index URLs.

Given that the existing code tries to handle index URLs with trailing slashes already, and that the Simple API spec says "All URLs which respond with an HTML5 page MUST end with a /" and "the root URL MUST be a valid HTML5 page", it seems reasonable to consider this a bug within rules_python rather than just making users manually remove trailing slashes from $PIP_EXTRA_INDEX_URL themselves.

🔬 Minimal Reproduction

MODULE.bazel:

bazel_dep(name = "rules_python", version = "1.0.0")

python = use_extension("@rules_python//python/extensions:python.bzl", "python")
python.toolchain(
    is_default = True,
    python_version = "3.11.5",
)
pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
pip.parse(
    hub_name = "reqs",
    envsubst = ["PIP_EXTRA_INDEX_URL"],
    experimental_index_url = "$PIP_EXTRA_INDEX_URL",
    python_version = "3.11.5",
    requirements_lock = "//:requirements.txt",
)
use_repo(pip, "reqs")

export PIP_EXTRA_INDEX_URL='https://<redacted>.d.codeartifact.eu-central-1.amazonaws.com/pypi/packages-prod/simple/'
bazel fetch '//...'

🔥 Exception or Error

WARNING: Download from https://.d.codeartifact.eu-central-1.amazonaws.com/pypi/packages-prod/simple//msal/ failed: class java.io.FileNotFoundException GET returned 404 Not Found
ERROR: /home/wmorrison/.cache/bazel/_bazel_wmorrison/0c20006a190ede26aadee6a87e8d620f/external/rules_python+/python/private/pypi/simpleapi_download.bzl:120:17: Traceback (most recent call last):
        File "/home/wmorrison/.cache/bazel/_bazel_wmorrison/0c20006a190ede26aadee6a87e8d620f/external/rules_python+/python/private/pypi/extension.bzl", line 604, column 25, in _pip_impl
                mods = parse_modules(module_ctx)
        File "/home/wmorrison/.cache/bazel/_bazel_wmorrison/0c20006a190ede26aadee6a87e8d620f/external/rules_python+/python/private/pypi/extension.bzl", line 478, column 36, in parse_modules
                out = _create_whl_repos(
        File "/home/wmorrison/.cache/bazel/_bazel_wmorrison/0c20006a190ede26aadee6a87e8d620f/external/rules_python+/python/private/pypi/extension.bzl", line 182, column 50, in _create_whl_repos
                requirements_by_platform = parse_requirements(
        File "/home/wmorrison/.cache/bazel/_bazel_wmorrison/0c20006a190ede26aadee6a87e8d620f/external/rules_python+/python/private/pypi/parse_requirements.bzl", line 174, column 36, in parse_requirements
                index_urls = get_index_urls(
        File "/home/wmorrison/.cache/bazel/_bazel_wmorrison/0c20006a190ede26aadee6a87e8d620f/external/rules_python+/python/private/pypi/extension.bzl", line 166, column 71, in lambda
                get_index_urls = lambda ctx, distributions: simpleapi_download(
        File "/home/wmorrison/.cache/bazel/_bazel_wmorrison/0c20006a190ede26aadee6a87e8d620f/external/rules_python+/python/private/pypi/simpleapi_download.bzl", line 120, column 17, in simpleapi_download
                fail("Failed to download metadata from urls: {}".format(
Error in fail: Failed to download metadata from urls: $PIP_EXTRA_INDEX_URL
ERROR: error evaluating module extension pip in @@rules_python+//python/extensions:pip.bzl
INFO: Elapsed time: 2.313s
INFO: 0 processes.
ERROR: Build did NOT complete successfully
ERROR: Fetching some target dependencies failed with errors: error evaluating module extension pip in @@rules_python+//python/extensions:pip.bzl

🌍 Your Environment

Operating System:

  
Linux
  

Output of bazel version:

  
Bazelisk version: v1.20.0
Build label: 8.0.0
Build target: @@//src/main/java/com/google/devtools/build/lib/bazel:BazelServer
Build time: Mon Dec 09 18:00:43 2024 (1733767243)
Build timestamp: 1733767243
Build timestamp as int: 1733767243
  

Rules_python version:

  
1.0.0
  

Anything else relevant?

Related to #1357

[aignas]

Thank you for the report. Yeah, I think we should replace double slashes with single ones after substitution.

The unit tests for this should be in //tests/pypi/simpleapi_download

Would you mind submitting a PR for this?