From 5c819b2477b85846cda8ec51cc4c4826aa51b94e Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 27 Sep 2024 09:06:09 +0900 Subject: [PATCH 1/5] fix(bzlmod): pass extra_pip_args when building sdists in experimental mode --- examples/bzlmod/MODULE.bazel.lock | 4 ++-- python/private/pypi/extension.bzl | 9 +++++++-- 2 files changed, 9 insertions(+), 4 deletions(-) diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock index cb8fbe287c..462bfc5a57 100644 --- a/examples/bzlmod/MODULE.bazel.lock +++ b/examples/bzlmod/MODULE.bazel.lock @@ -1231,7 +1231,7 @@ }, "@@rules_python~//python/extensions:pip.bzl%pip": { "general": { - "bzlTransitiveDigest": "ED3oUrLQz/MTptq8JOZ03sjD7HZ3naUeFS3XFpxz4tg=", + "bzlTransitiveDigest": "sVt1fNBD2C152o2HSv9ICm8hK+GPiHOmIX2x/t4exJE=", "usagesDigest": "MChlcSw99EuW3K7OOoMcXQIdcJnEh6YmfyjJm+9mxIg=", "recordedFileInputs": { "@@other_module~//requirements_lock_3_11.txt": "a7d0061366569043d5efcf80e34a32c732679367cb3c831c4cdc606adc36d314", @@ -6140,7 +6140,7 @@ }, "@@rules_python~//python/private/pypi:pip.bzl%pip_internal": { "general": { - "bzlTransitiveDigest": "vEOIMpxlh8qbHkABunGFRr+IDbabjCM/hUF0V3GGTus=", + "bzlTransitiveDigest": "XDXuel/vKL4fY3VIVU7oavrTOV0ovaUtX8cDdpYH7fE=", "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=", "recordedFileInputs": { "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a", diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index 77a477899e..4eae3be76d 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -275,8 +275,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) From fe50515fd5bce249aa90bdce2f4962223d49474b Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 27 Sep 2024 09:10:13 +0900 Subject: [PATCH 2/5] fix(bzlmod): join extra_pip_args from the file and the pip.parse attr --- examples/bzlmod/MODULE.bazel.lock | 4 ++-- python/private/pypi/parse_requirements.bzl | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock index 462bfc5a57..9854736f94 100644 --- a/examples/bzlmod/MODULE.bazel.lock +++ b/examples/bzlmod/MODULE.bazel.lock @@ -1231,7 +1231,7 @@ }, "@@rules_python~//python/extensions:pip.bzl%pip": { "general": { - "bzlTransitiveDigest": "sVt1fNBD2C152o2HSv9ICm8hK+GPiHOmIX2x/t4exJE=", + "bzlTransitiveDigest": "VcAOo9TyCjejBkUOwMuF+Nu8dm4RQFTyH/DtXVu2nOk=", "usagesDigest": "MChlcSw99EuW3K7OOoMcXQIdcJnEh6YmfyjJm+9mxIg=", "recordedFileInputs": { "@@other_module~//requirements_lock_3_11.txt": "a7d0061366569043d5efcf80e34a32c732679367cb3c831c4cdc606adc36d314", @@ -6140,7 +6140,7 @@ }, "@@rules_python~//python/private/pypi:pip.bzl%pip_internal": { "general": { - "bzlTransitiveDigest": "XDXuel/vKL4fY3VIVU7oavrTOV0ovaUtX8cDdpYH7fE=", + "bzlTransitiveDigest": "Q1CxstNTZB6NQtQd8rLxRkY3GFpFvHfNig9P4Jh/J3c=", "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=", "recordedFileInputs": { "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a", diff --git a/python/private/pypi/parse_requirements.bzl b/python/private/pypi/parse_requirements.bzl index eee97d7019..c72f5d43f8 100644 --- a/python/private/pypi/parse_requirements.bzl +++ b/python/private/pypi/parse_requirements.bzl @@ -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. From b90dd589c8a30d324e3be6aa5ebad9eac5940b76 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 27 Sep 2024 09:13:40 +0900 Subject: [PATCH 3/5] test: add a test to ensure that the extra args are joined However this is not a test for the code in question as those tests probably need to wait for #2253 to land to make the test writing easier. --- .../parse_requirements_tests.bzl | 42 +++++++++++++++++++ 1 file changed, 42 insertions(+) diff --git a/tests/pypi/parse_requirements/parse_requirements_tests.bzl b/tests/pypi/parse_requirements/parse_requirements_tests.bzl index 25d2961a34..c719ad6972 100644 --- a/tests/pypi/parse_requirements/parse_requirements_tests.bzl +++ b/tests/pypi/parse_requirements/parse_requirements_tests.bzl @@ -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 @@ -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(), From 82a232ef9cefb00993dea1bb6a16f8408aeb8cbe Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Fri, 27 Sep 2024 09:19:40 +0900 Subject: [PATCH 4/5] doc: changelog --- CHANGELOG.md | 6 +++++- 1 file changed, 5 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8ea956574d..9f3e53848d 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,7 +28,11 @@ A brief description of the categories of changes: * Nothing yet ### Fixed -* Nothing yet +* (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). ### Added * Nothing yet From eb4cb5c20920dc0954bf6f7b4d42963aae75d597 Mon Sep 17 00:00:00 2001 From: Ignas Anikevicius <240938+aignas@users.noreply.github.com> Date: Sat, 28 Sep 2024 12:35:28 +0900 Subject: [PATCH 5/5] add the line back - the test is not testing the extension code yet --- examples/bzlmod/MODULE.bazel.lock | 4 ++-- python/private/pypi/extension.bzl | 1 + 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock index 9854736f94..604aef17c6 100644 --- a/examples/bzlmod/MODULE.bazel.lock +++ b/examples/bzlmod/MODULE.bazel.lock @@ -1231,7 +1231,7 @@ }, "@@rules_python~//python/extensions:pip.bzl%pip": { "general": { - "bzlTransitiveDigest": "VcAOo9TyCjejBkUOwMuF+Nu8dm4RQFTyH/DtXVu2nOk=", + "bzlTransitiveDigest": "W8FWi7aL0uqh7djg6csTMzkL1RB6WGRgfO/MOcbqYZI=", "usagesDigest": "MChlcSw99EuW3K7OOoMcXQIdcJnEh6YmfyjJm+9mxIg=", "recordedFileInputs": { "@@other_module~//requirements_lock_3_11.txt": "a7d0061366569043d5efcf80e34a32c732679367cb3c831c4cdc606adc36d314", @@ -6140,7 +6140,7 @@ }, "@@rules_python~//python/private/pypi:pip.bzl%pip_internal": { "general": { - "bzlTransitiveDigest": "Q1CxstNTZB6NQtQd8rLxRkY3GFpFvHfNig9P4Jh/J3c=", + "bzlTransitiveDigest": "9aEp4XU4yVL6sJqODxio5iSCoqf37Ro3o+Mt1izDnq8=", "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=", "recordedFileInputs": { "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a", diff --git a/python/private/pypi/extension.bzl b/python/private/pypi/extension.bzl index 4eae3be76d..712d2fefda 100644 --- a/python/private/pypi/extension.bzl +++ b/python/private/pypi/extension.bzl @@ -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