Skip to content

Commit

Permalink
fix(bzlmod): correctly wire the extra_pip_args (#2258)
Browse files Browse the repository at this point in the history
Before this PR we were just dropping the `extra_pip_args` passed to
`pip.parse` and were just using the args passed through the requirements
file. Thanks to @swarren12 for pointing this out.

This PR also passes `extra_pip_args` to `sdist` `whl_library` instances
so that users can build the `sdists` correctly when using
`experimental_index_url` feature.

Summary:
- pass `extra_pip_args` when building sdists in experimental mode
- join `extra_pip_args` from the file and the pip.parse attr
- test: add a test to ensure that the extra args are joined

Fixes #2239
Closes #2254
  • Loading branch information
aignas authored Oct 3, 2024
1 parent 413690f commit 4f38119
Show file tree
Hide file tree
Showing 5 changed files with 58 additions and 5 deletions.
5 changes: 5 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,11 @@ A brief description of the categories of changes:
* Nothing yet

### Fixed
* (bzlmod) correctly wire the {attr}`pip.parse.extra_pip_args` all the
way to {obj}`whl_library`. What is more we will pass the `extra_pip_args` to
{obj}`whl_library` for `sdist` distributions when using
{attr}`pip.parse.experimental_index_url`. See
[#2239](https://github.com/bazelbuild/rules_python/issues/2239).
* (whl_filegroup): Provide per default also the `RECORD` file

### Added
Expand Down
4 changes: 2 additions & 2 deletions examples/bzlmod/MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

10 changes: 8 additions & 2 deletions python/private/pypi/extension.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -182,6 +182,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
python_version = major_minor,
logger = logger,
),
extra_pip_args = pip_attr.extra_pip_args,
get_index_urls = get_index_urls,
# NOTE @aignas 2024-08-02: , we will execute any interpreter that we find either
# in the PATH or if specified as a label. We will configure the env
Expand Down Expand Up @@ -275,8 +276,13 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
if pip_attr.auth_patterns:
whl_library_args["auth_patterns"] = pip_attr.auth_patterns

# pip is not used to download wheels and the python `whl_library` helpers are only extracting things
whl_library_args.pop("extra_pip_args", None)
if distribution.filename.endswith(".whl"):
# pip is not used to download wheels and the python `whl_library` helpers are only extracting things
whl_library_args.pop("extra_pip_args", None)
else:
# For sdists, they will be built by `pip`, so we still
# need to pass the extra args there.
pass

# This is no-op because pip is not used to download the wheel.
whl_library_args.pop("download_only", None)
Expand Down
2 changes: 1 addition & 1 deletion python/private/pypi/parse_requirements.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ def parse_requirements(
different package versions (or different packages) for different
os, arch combinations.
extra_pip_args (string list): Extra pip arguments to perform extra validations and to
be joined with args fined in files.
be joined with args found in files.
get_index_urls: Callable[[ctx, list[str]], dict], a callable to get all
of the distribution URLs from a PyPI index. Accepts ctx and
distribution names to query.
Expand Down
42 changes: 42 additions & 0 deletions tests/pypi/parse_requirements/parse_requirements_tests.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,11 @@ def _mock_ctx():
testdata = {
"requirements_direct": """\
foo[extra] @ https://some-url
""",
"requirements_extra_args": """\
--index-url=example.org
foo[extra]==0.0.1 --hash=sha256:deadbeef
""",
"requirements_linux": """\
foo==0.0.3 --hash=sha256:deadbaaf
Expand Down Expand Up @@ -93,6 +98,43 @@ def _test_simple(env):

_tests.append(_test_simple)

def _test_extra_pip_args(env):
got = parse_requirements(
ctx = _mock_ctx(),
requirements_by_platform = {
"requirements_extra_args": ["linux_x86_64"],
},
extra_pip_args = ["--trusted-host=example.org"],
)
env.expect.that_dict(got).contains_exactly({
"foo": [
struct(
distribution = "foo",
extra_pip_args = ["--index-url=example.org", "--trusted-host=example.org"],
requirement_line = "foo[extra]==0.0.1 --hash=sha256:deadbeef",
srcs = struct(
requirement = "foo[extra]==0.0.1",
shas = ["deadbeef"],
version = "0.0.1",
),
target_platforms = [
"linux_x86_64",
],
whls = [],
sdist = None,
is_exposed = True,
),
],
})
env.expect.that_str(
select_requirement(
got["foo"],
platform = "linux_x86_64",
).srcs.version,
).equals("0.0.1")

_tests.append(_test_extra_pip_args)

def _test_dupe_requirements(env):
got = parse_requirements(
ctx = _mock_ctx(),
Expand Down

0 comments on commit 4f38119

Please sign in to comment.