From b99bb612bbffe2259b6ecd2d41995df7a6f78fde Mon Sep 17 00:00:00 2001 From: "Elvis M. Wianda" <7077790+ewianda@users.noreply.github.com> Date: Sat, 24 Aug 2024 05:07:36 -0400 Subject: [PATCH] fix(whl_library): remove `--no-index` and add `--no-build-isolation` when build sdist (#2126) Building sdist results in `Could not find a version that satisfies the requirement setuptool` this regressed when a fix in parameter handling got introduced in #2091. Before this change the building from sdist when using `experimental_index_url` would break because `--no-index` is passed to `pip`. This means that `pip` would fail to locate build time dependencies needed for the packages and would just not work. In `whl_library` we setup `PYTHONPATH` to have some build dependencies available (like `setuptools`) and we could use them during building from `sdist` and to do so we need to add `--no-build-isolation` flag. However, for some cases we need to also add other build-time dependencies (e.g. `flit_core`) so that the building of the wheel in the `repository_rule` context is successfuly. Removing `--no-index` allows `pip` to silently fetch the needed build dependencies from PyPI if they are missing and continue with the build. This is not a perfect solution, but it does unblock users to use the `sdist` distributions with the experimental feature enabled by using `experimental_index_url` (see #260 for tracking of the completion). Fixes #2118 Fixes #2152 --------- Co-authored-by: aignas <240938+aignas@users.noreply.github.com> --- CHANGELOG.md | 3 +++ examples/bzlmod/MODULE.bazel.lock | 4 ++-- python/private/pypi/whl_library.bzl | 6 ++++-- tests/pypi/evaluate_markers/BUILD.bazel | 7 ------- tests/pypi/integration/BUILD.bazel | 20 ++++++++++++++++++++ tests/pypi/integration/transitions.bzl | 24 ++++++++++++++++++++++++ 6 files changed, 53 insertions(+), 11 deletions(-) delete mode 100644 tests/pypi/evaluate_markers/BUILD.bazel create mode 100644 tests/pypi/integration/BUILD.bazel create mode 100644 tests/pypi/integration/transitions.bzl diff --git a/CHANGELOG.md b/CHANGELOG.md index 029de8ae1..9e59564b9 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -28,6 +28,9 @@ A brief description of the categories of changes: * (gazelle): Update error messages when unable to resolve a dependency to be more human-friendly. ### Fixed +* (whl_library): Remove `--no-index` and add `--no-build-isolation` to the + `pip install` command when installing a wheel from a local file, which happens + when `experimental_index_url` flag is used. * (bzlmod) get the path to the host python interpreter in a way that results in platform non-dependent hashes in the lock file when the requirement markers need to be evaluated. diff --git a/examples/bzlmod/MODULE.bazel.lock b/examples/bzlmod/MODULE.bazel.lock index e542f43b0..4deef01be 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": "iJtgc0fsxL6yWzjazyklge5csIWtbgQ8+wgW8P4jfL0=", + "bzlTransitiveDigest": "eWzi4IV0AzQ+cpVkMkdU/wOc7BUQdo0hAAQcCh8C4uU=", "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": "vTGpUAA3aBLYLx1iJ020kpJsuvO9GLbl11swsDXNKRI=", + "bzlTransitiveDigest": "RyEJxfGmNQVzqInjjGrR29yqfFPKe9DKgODI1mxd8wA=", "usagesDigest": "Y8ihY+R57BAFhalrVLVdJFrpwlbsiKz9JPJ99ljF7HA=", "recordedFileInputs": { "@@rules_python~//tools/publish/requirements.txt": "031e35d03dde03ae6305fe4b3d1f58ad7bdad857379752deede0f93649991b8a", diff --git a/python/private/pypi/whl_library.bzl b/python/private/pypi/whl_library.bzl index 2300eb359..5b14151be 100644 --- a/python/private/pypi/whl_library.bzl +++ b/python/private/pypi/whl_library.bzl @@ -227,8 +227,10 @@ def _whl_library_impl(rctx): whl_path = rctx.path(rctx.attr.filename) else: # It is an sdist and we need to tell PyPI to use a file in this directory - # and not use any indexes. - extra_pip_args.extend(["--no-index", "--find-links", "."]) + # and, allow getting build dependencies from PYTHONPATH, which we + # setup in this repository rule, but still download any necessary + # build deps from PyPI (e.g. `flit_core`) if they are missing. + extra_pip_args.extend(["--no-build-isolation", "--find-links", "."]) args = _parse_optional_attrs(rctx, args, extra_pip_args) diff --git a/tests/pypi/evaluate_markers/BUILD.bazel b/tests/pypi/evaluate_markers/BUILD.bazel deleted file mode 100644 index aba926495..000000000 --- a/tests/pypi/evaluate_markers/BUILD.bazel +++ /dev/null @@ -1,7 +0,0 @@ -load("@bazel_skylib//rules:build_test.bzl", "build_test") -load("@dev_pip//:requirements.bzl", "all_whl_requirements") - -build_test( - name = "all_dev_wheels", - targets = all_whl_requirements, -) diff --git a/tests/pypi/integration/BUILD.bazel b/tests/pypi/integration/BUILD.bazel new file mode 100644 index 000000000..f846bfb12 --- /dev/null +++ b/tests/pypi/integration/BUILD.bazel @@ -0,0 +1,20 @@ +load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@dev_pip//:requirements.bzl", "all_requirements") +load(":transitions.bzl", "transition_rule") + +build_test( + name = "all_requirements_build_test", + targets = all_requirements, +) + +# Rule that transitions dependencies to be built from sdist +transition_rule( + name = "all_requirements_from_sdist", + testonly = True, + deps = all_requirements, +) + +build_test( + name = "all_requirements_from_sdist_build_test", + targets = ["all_requirements_from_sdist"], +) diff --git a/tests/pypi/integration/transitions.bzl b/tests/pypi/integration/transitions.bzl new file mode 100644 index 000000000..b121446bb --- /dev/null +++ b/tests/pypi/integration/transitions.bzl @@ -0,0 +1,24 @@ +""" Define a custom transition that sets the pip_whl flag to no """ + +def _flag_transition_impl(_settings, _ctx): + return {"//python/config_settings:pip_whl": "no"} + +flag_transition = transition( + implementation = _flag_transition_impl, + inputs = [], + outputs = ["//python/config_settings:pip_whl"], +) + +# Define a rule that applies the transition to dependencies +def _transition_rule_impl(_ctx): + return [DefaultInfo()] + +transition_rule = rule( + implementation = _transition_rule_impl, + attrs = { + "deps": attr.label_list(cfg = flag_transition), + "_allowlist_function_transition": attr.label( + default = "@bazel_tools//tools/allowlists/function_transition_allowlist", + ), + }, +)