From 996655053f8fb88b6cb50924b9b08fb6a599ff92 Mon Sep 17 00:00:00 2001 From: Alex Eagle Date: Thu, 31 Mar 2022 17:00:22 -0700 Subject: [PATCH] change approach for vendoring parsed requirements Instead of printing something different in the pip_parse tooling, just suggest that users continue to run the repository rule, but check the output into their repo, then load from that instead of from the generated repository. This is simpler than trying to work out what correct arguments to pass to the tool when running it outside of the pip_parse starlark context. --- .bazelci/presubmit.yml | 2 + .bazelrc | 4 +- docs/pip.md | 18 ++----- examples/BUILD | 6 +++ examples/pip_parse_vendored/BUILD | 50 ++++++++++++++++++ examples/pip_parse_vendored/README.md | 11 ++++ examples/pip_parse_vendored/WORKSPACE | 44 ++++++++++++++++ examples/pip_parse_vendored/requirements.bzl | 51 +++++++++++++++++++ examples/pip_parse_vendored/requirements.in | 1 + examples/pip_parse_vendored/requirements.txt | 26 ++++++++++ python/pip.bzl | 18 ++----- .../parse_requirements_to_bzl/__init__.py | 23 +++++++-- .../parse_requirements_to_bzl/__main__.py | 11 +--- python/pip_install/pip_repository.bzl | 2 + .../update_deleted_packages.sh | 2 +- 15 files changed, 226 insertions(+), 43 deletions(-) create mode 100644 examples/pip_parse_vendored/BUILD create mode 100644 examples/pip_parse_vendored/README.md create mode 100644 examples/pip_parse_vendored/WORKSPACE create mode 100644 examples/pip_parse_vendored/requirements.bzl create mode 100644 examples/pip_parse_vendored/requirements.in create mode 100644 examples/pip_parse_vendored/requirements.txt diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml index 75157c01fa..1cc121a4e7 100644 --- a/.bazelci/presubmit.yml +++ b/.bazelci/presubmit.yml @@ -36,3 +36,5 @@ platforms: - "-//gazelle/..." # The dependencies needed for this test are not cross-platform: https://github.com/bazelbuild/rules_python/issues/260 - "-//tests:pip_repository_entry_points_example" + test_flags: + - "--test_tag_filters=-fix-windows" \ No newline at end of file diff --git a/.bazelrc b/.bazelrc index 4345f50815..634d29cc48 100644 --- a/.bazelrc +++ b/.bazelrc @@ -3,8 +3,8 @@ # This lets us glob() up all the files inside the examples to make them inputs to tests # (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it) # To update these lines, run tools/bazel_integration_test/update_deleted_packages.sh -build --deleted_packages=examples/build_file_generation,examples/pip_install,examples/pip_parse,examples/pip_repository_annotations,examples/py_import,examples/relative_requirements,tests/pip_repository_entry_points -query --deleted_packages=examples/build_file_generation,examples/pip_install,examples/pip_parse,examples/pip_repository_annotations,examples/py_import,examples/relative_requirements,tests/pip_repository_entry_points +build --deleted_packages=examples/build_file_generation,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_import,examples/relative_requirements,tests/pip_repository_entry_points +query --deleted_packages=examples/build_file_generation,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_import,examples/relative_requirements,tests/pip_repository_entry_points test --test_output=errors diff --git a/docs/pip.md b/docs/pip.md index 74248cef7c..73ed79d1da 100644 --- a/docs/pip.md +++ b/docs/pip.md @@ -230,19 +230,11 @@ See https://github.com/bazelbuild/rules_python/issues/608 This is the same workflow as Gazelle, which creates `go_repository` rules with [`update-repos`](https://github.com/bazelbuild/bazel-gazelle#update-repos) -Simply run the same tool that the `pip_parse` repository rule calls. -You can find the arguments in the generated BUILD file in the pip_parse repo, -for example in `$(bazel info output_base)/external/pypi/BUILD.bazel` for a repo -named `pypi`. - -The command will look like this: -```shell -bazel run -- @rules_python//python/pip_install/parse_requirements_to_bzl \ - --requirements_lock ./requirements_lock.txt \ - --quiet False --timeout 120 --repo pypi --repo-prefix pypi_ > requirements.bzl -``` - -Then load the requirements.bzl file directly, without using `pip_parse` in the WORKSPACE. +To do this, use the "write to source file" pattern documented in +https://blog.aspect.dev/bazel-can-write-to-the-source-folder +to put a copy of the generated requirements.bzl into your project. +Then load the requirements.bzl file directly rather than from the generated repository. +See the example in rules_python/examples/pip_parse_vendored. **PARAMETERS** diff --git a/examples/BUILD b/examples/BUILD index 44147e5c4a..c6a57c4d7e 100644 --- a/examples/BUILD +++ b/examples/BUILD @@ -27,6 +27,12 @@ bazel_integration_test( timeout = "long", ) +bazel_integration_test( + name = "pip_parse_vendored_example", + timeout = "long", + tags = ["fix-windows"], +) + bazel_integration_test( name = "pip_repository_annotations_example", timeout = "long", diff --git a/examples/pip_parse_vendored/BUILD b/examples/pip_parse_vendored/BUILD new file mode 100644 index 0000000000..b5a85295e3 --- /dev/null +++ b/examples/pip_parse_vendored/BUILD @@ -0,0 +1,50 @@ +load("@bazel_skylib//rules:diff_test.bzl", "diff_test") +load("@bazel_skylib//rules:write_file.bzl", "write_file") +load("@rules_python//python:pip.bzl", "compile_pip_requirements") + +# This rule adds a convenient way to update the requirements.txt +# lockfile based on the requirements.in. +compile_pip_requirements(name = "requirements") + +# The requirements.bzl file is generated with a reference to the interpreter for the host platform. +# In order to check in a platform-agnostic file, we have to replace that reference with the symbol +# loaded from our python toolchain. +genrule( + name = "make_platform_agnostic", + srcs = ["@pip//:requirements.bzl"], + outs = ["requirements.clean.bzl"], + cmd = " | ".join([ + "cat $<", + # Insert our load statement after the existing one so we don't produce a file with buildifier warnings + """sed -e '/^load.*/i\\'$$'\\n''load("@python39//:defs.bzl", "interpreter")'""", + """tr "'" '"' """, + """sed 's#"@python39_.*//:bin/python3"#interpreter#' >$@""", + ]), +) + +write_file( + name = "gen_update", + out = "update.sh", + content = [ + # This depends on bash, would need tweaks for Windows + "#!/usr/bin/env bash", + # Bazel gives us a way to access the source folder! + "cd $BUILD_WORKSPACE_DIRECTORY", + "cp -fv bazel-bin/requirements.clean.bzl requirements.bzl", + ], +) + +sh_binary( + name = "vendor_requirements", + srcs = ["update.sh"], + data = [":make_platform_agnostic"], +) + +# Similarly ensures that the requirements.bzl file is updated +# based on the requirements.txt lockfile. +diff_test( + name = "test_vendored", + failure_message = "Please run: bazel run //:vendor_requirements", + file1 = "requirements.bzl", + file2 = ":make_platform_agnostic", +) diff --git a/examples/pip_parse_vendored/README.md b/examples/pip_parse_vendored/README.md new file mode 100644 index 0000000000..616e291409 --- /dev/null +++ b/examples/pip_parse_vendored/README.md @@ -0,0 +1,11 @@ +# pip_parse vendored + +This example is like pip_parse, however we avoid loading from the generated file. +See https://github.com/bazelbuild/rules_python/issues/608 +and https://blog.aspect.dev/avoid-eager-fetches. + +The requirements now form a triple: + +- requirements.in - human editable, expresses only direct dependencies and load-bearing version constraints +- requirements.txt - lockfile produced by pip-compile or other means +- requirements.bzl - the "parsed" version of the lockfile readable by Bazel downloader diff --git a/examples/pip_parse_vendored/WORKSPACE b/examples/pip_parse_vendored/WORKSPACE new file mode 100644 index 0000000000..2f0bfb183a --- /dev/null +++ b/examples/pip_parse_vendored/WORKSPACE @@ -0,0 +1,44 @@ +workspace(name = "pip_repository_annotations_example") + +load("@bazel_tools//tools/build_defs/repo:http.bzl", "http_archive") + +local_repository( + name = "rules_python", + path = "../..", +) + +http_archive( + name = "bazel_skylib", + sha256 = "c6966ec828da198c5d9adbaa94c05e3a1c7f21bd012a0b29ba8ddbccb2c93b0d", + urls = [ + "https://github.com/bazelbuild/bazel-skylib/releases/download/1.1.1/bazel-skylib-1.1.1.tar.gz", + "https://mirror.bazel.build/github.com/bazelbuild/bazel-skylib/releases/download/1.1.1/bazel-skylib-1.1.1.tar.gz", + ], +) + +load("@rules_python//python:repositories.bzl", "python_register_toolchains") + +python_register_toolchains( + name = "python39", + python_version = "3.9", +) + +load("@python39//:defs.bzl", "interpreter") +load("@rules_python//python:pip.bzl", "pip_parse") + +# This repository isn't referenced, except by our test that asserts the requirements.bzl is updated. +# It also wouldn't be needed by users of this ruleset. +pip_parse( + name = "pip", + python_interpreter_target = interpreter, + requirements_lock = "//:requirements.txt", +) + +# This example vendors the file produced by `pip_parse` above into the repo. +# This way our Bazel doesn't eagerly fetch and install the pip_parse'd +# repository for builds that don't need it. +# See discussion of the trade-offs in the pip_parse documentation +# and the "vendor_requirements" target in the BUILD file. +load("//:requirements.bzl", "install_deps") + +install_deps() diff --git a/examples/pip_parse_vendored/requirements.bzl b/examples/pip_parse_vendored/requirements.bzl new file mode 100644 index 0000000000..038b612309 --- /dev/null +++ b/examples/pip_parse_vendored/requirements.bzl @@ -0,0 +1,51 @@ +"""Starlark representation of locked requirements. + +@generated by rules_python pip_parse repository rule +from //:requirements.txt +""" + +load("@python39//:defs.bzl", "interpreter") +load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library") + +all_requirements = ["@pip_certifi//:pkg", "@pip_charset_normalizer//:pkg", "@pip_idna//:pkg", "@pip_requests//:pkg", "@pip_urllib3//:pkg"] + +all_whl_requirements = ["@pip_certifi//:whl", "@pip_charset_normalizer//:whl", "@pip_idna//:whl", "@pip_requests//:whl", "@pip_urllib3//:whl"] + +_packages = [("pip_certifi", "certifi==2021.10.8 --hash=sha256:78884e7c1d4b00ce3cea67b44566851c4343c120abd683433ce934a68ea58872 --hash=sha256:d62a0163eb4c2344ac042ab2bdf75399a71a2d8c7d47eac2e2ee91b9d6339569"), ("pip_charset_normalizer", "charset-normalizer==2.0.12 --hash=sha256:2857e29ff0d34db842cd7ca3230549d1a697f96ee6d3fb071cfa6c7393832597 --hash=sha256:6881edbebdb17b39b4eaaa821b438bf6eddffb4468cf344f09f89def34a8b1df"), ("pip_idna", "idna==3.3 --hash=sha256:84d9dd047ffa80596e0f246e2eab0b391788b0503584e8945f2368256d2735ff --hash=sha256:9d643ff0a55b762d5cdb124b8eaa99c66322e2157b69160bc32796e824360e6d"), ("pip_requests", "requests==2.27.1 --hash=sha256:68d7c56fd5a8999887728ef304a6d12edc7be74f1cfa47714fc8b414525c9a61 --hash=sha256:f22fa1e554c9ddfd16e6e41ac79759e17be9e492b3587efa038054674760e72d"), ("pip_urllib3", "urllib3==1.26.9 --hash=sha256:44ece4d53fb1706f667c9bd1c648f5469a2ec925fcf3a776667042d645472c14 --hash=sha256:aabaf16477806a5e1dd19aa41f8c2b7950dd3c746362d7e3223dbe6de6ac448e")] +_config = {"enable_implicit_namespace_pkgs": False, "environment": {}, "extra_pip_args": [], "isolated": True, "pip_data_exclude": [], "python_interpreter": "python3", "python_interpreter_target": interpreter, "quiet": True, "repo": "pip", "repo_prefix": "pip_", "timeout": 600} +_annotations = {} + +def _clean_name(name): + return name.replace("-", "_").replace(".", "_").lower() + +def requirement(name): + return "@pip_" + _clean_name(name) + "//:pkg" + +def whl_requirement(name): + return "@pip_" + _clean_name(name) + "//:whl" + +def data_requirement(name): + return "@pip_" + _clean_name(name) + "//:data" + +def dist_info_requirement(name): + return "@pip_" + _clean_name(name) + "//:dist_info" + +def entry_point(pkg, script = None): + if not script: + script = pkg + return "@pip_" + _clean_name(pkg) + "//:rules_python_wheel_entry_point_" + script + +def _get_annotation(requirement): + # This expects to parse `setuptools==58.2.0 --hash=sha256:2551203ae6955b9876741a26ab3e767bb3242dafe86a32a749ea0d78b6792f11` + # down wo `setuptools`. + name = requirement.split(" ")[0].split("=")[0] + return _annotations.get(name) + +def install_deps(): + for name, requirement in _packages: + whl_library( + name = name, + requirement = requirement, + annotation = _get_annotation(requirement), + **_config + ) diff --git a/examples/pip_parse_vendored/requirements.in b/examples/pip_parse_vendored/requirements.in new file mode 100644 index 0000000000..f2293605cf --- /dev/null +++ b/examples/pip_parse_vendored/requirements.in @@ -0,0 +1 @@ +requests diff --git a/examples/pip_parse_vendored/requirements.txt b/examples/pip_parse_vendored/requirements.txt new file mode 100644 index 0000000000..62f65ea5d5 --- /dev/null +++ b/examples/pip_parse_vendored/requirements.txt @@ -0,0 +1,26 @@ +# +# This file is autogenerated by pip-compile +# To update, run: +# +# bazel run //:requirements.update +# +certifi==2021.10.8 \ + --hash=sha256:78884e7c1d4b00ce3cea67b44566851c4343c120abd683433ce934a68ea58872 \ + --hash=sha256:d62a0163eb4c2344ac042ab2bdf75399a71a2d8c7d47eac2e2ee91b9d6339569 + # via requests +charset-normalizer==2.0.12 \ + --hash=sha256:2857e29ff0d34db842cd7ca3230549d1a697f96ee6d3fb071cfa6c7393832597 \ + --hash=sha256:6881edbebdb17b39b4eaaa821b438bf6eddffb4468cf344f09f89def34a8b1df + # via requests +idna==3.3 \ + --hash=sha256:84d9dd047ffa80596e0f246e2eab0b391788b0503584e8945f2368256d2735ff \ + --hash=sha256:9d643ff0a55b762d5cdb124b8eaa99c66322e2157b69160bc32796e824360e6d + # via requests +requests==2.27.1 \ + --hash=sha256:68d7c56fd5a8999887728ef304a6d12edc7be74f1cfa47714fc8b414525c9a61 \ + --hash=sha256:f22fa1e554c9ddfd16e6e41ac79759e17be9e492b3587efa038054674760e72d + # via -r requirements.in +urllib3==1.26.9 \ + --hash=sha256:44ece4d53fb1706f667c9bd1c648f5469a2ec925fcf3a776667042d645472c14 \ + --hash=sha256:aabaf16477806a5e1dd19aa41f8c2b7950dd3c746362d7e3223dbe6de6ac448e + # via requests diff --git a/python/pip.bzl b/python/pip.bzl index b0e13194b0..93096a86a4 100644 --- a/python/pip.bzl +++ b/python/pip.bzl @@ -179,19 +179,11 @@ def pip_parse(requirements_lock, name = "pip_parsed_deps", **kwargs): This is the same workflow as Gazelle, which creates `go_repository` rules with [`update-repos`](https://github.com/bazelbuild/bazel-gazelle#update-repos) - Simply run the same tool that the `pip_parse` repository rule calls. - You can find the arguments in the generated BUILD file in the pip_parse repo, - for example in `$(bazel info output_base)/external/pypi/BUILD.bazel` for a repo - named `pypi`. - - The command will look like this: - ```shell - bazel run -- @rules_python//python/pip_install/parse_requirements_to_bzl \\ - --requirements_lock ./requirements_lock.txt \\ - --quiet False --timeout 120 --repo pypi --repo-prefix pypi_ > requirements.bzl - ``` - - Then load the requirements.bzl file directly, without using `pip_parse` in the WORKSPACE. + To do this, use the "write to source file" pattern documented in + https://blog.aspect.dev/bazel-can-write-to-the-source-folder + to put a copy of the generated requirements.bzl into your project. + Then load the requirements.bzl file directly rather than from the generated repository. + See the example in rules_python/examples/pip_parse_vendored. Args: requirements_lock (Label): A fully resolved 'requirements.txt' pip requirement file diff --git a/python/pip_install/parse_requirements_to_bzl/__init__.py b/python/pip_install/parse_requirements_to_bzl/__init__.py index 252cd26e00..9519d62031 100644 --- a/python/pip_install/parse_requirements_to_bzl/__init__.py +++ b/python/pip_install/parse_requirements_to_bzl/__init__.py @@ -62,7 +62,7 @@ def parse_whl_library_args(args: argparse.Namespace) -> Dict[str, Any]: whl_library_args.setdefault("python_interpreter", sys.executable) # These arguments are not used by `whl_library` - for arg in ("requirements_lock", "annotations"): + for arg in ("requirements_lock", "requirements_lock_label", "annotations"): if arg in whl_library_args: whl_library_args.pop(arg) @@ -102,6 +102,7 @@ def generate_parsed_requirements_contents( ) return textwrap.dedent( """\ + load("@rules_python//python/pip_install:pip_repository.bzl", "whl_library") all_requirements = [{all_requirements}] @@ -116,10 +117,10 @@ def _clean_name(name): return name.replace("-", "_").replace(".", "_").lower() def requirement(name): - return "@{repo_prefix}" + _clean_name(name) + "//:{py_library_label}" + return "@{repo_prefix}" + _clean_name(name) + "//:{py_library_label}" def whl_requirement(name): - return "@{repo_prefix}" + _clean_name(name) + "//:{wheel_file_label}" + return "@{repo_prefix}" + _clean_name(name) + "//:{wheel_file_label}" def data_requirement(name): return "@{repo_prefix}" + _clean_name(name) + "//:{data_label}" @@ -144,13 +145,13 @@ def install_deps(): name = name, requirement = requirement, annotation = _get_annotation(requirement), - **_config, + **_config ) """.format( all_requirements=all_requirements, all_whl_requirements=all_whl_requirements, annotations=json.dumps(annotations), - args=whl_library_args, + args=dict(sorted(whl_library_args.items())), data_label=bazel.DATA_LABEL, dist_info_label=bazel.DIST_INFO_LABEL, entry_point_prefix=bazel.WHEEL_ENTRY_POINT_PREFIX, @@ -181,6 +182,10 @@ def main(output: TextIO) -> None: required=True, help="Path to fully resolved requirements.txt to use as the source of repos.", ) + parser.add_argument( + "--requirements_lock_label", + help="Label used to declare the requirements.lock, included in comments in the file.", + ) parser.add_argument( "--python_interpreter", help="The python interpreter that will be used to download and unpack the wheels.", @@ -235,6 +240,14 @@ def main(output: TextIO) -> None: } ) + output.write(textwrap.dedent( + """\ + \"\"\"Starlark representation of locked requirements. + + @generated by rules_python pip_parse repository rule + from {} + \"\"\" + """.format(args.requirements_lock_label))) output.write( generate_parsed_requirements_contents( requirements_lock=args.requirements_lock, diff --git a/python/pip_install/parse_requirements_to_bzl/__main__.py b/python/pip_install/parse_requirements_to_bzl/__main__.py index 4ca73867a7..aaa6bf4500 100644 --- a/python/pip_install/parse_requirements_to_bzl/__main__.py +++ b/python/pip_install/parse_requirements_to_bzl/__main__.py @@ -5,12 +5,5 @@ from python.pip_install.parse_requirements_to_bzl import main if __name__ == "__main__": - # Under `bazel run`, just print the generated starlark code. - # This allows users to check that into their repository rather than - # call pip_parse to generate as a repository rule. - if "BUILD_WORKING_DIRECTORY" in os.environ: - os.chdir(os.environ["BUILD_WORKING_DIRECTORY"]) - main(sys.stdout) - else: - with open("requirements.bzl", "w") as requirement_file: - main(requirement_file) + with open("requirements.bzl", "w") as requirement_file: + main(requirement_file) diff --git a/python/pip_install/pip_repository.bzl b/python/pip_install/pip_repository.bzl index e587f34c73..4b03892d16 100644 --- a/python/pip_install/pip_repository.bzl +++ b/python/pip_install/pip_repository.bzl @@ -137,6 +137,8 @@ def _pip_repository_impl(rctx): "python.pip_install.parse_requirements_to_bzl", "--requirements_lock", rctx.path(rctx.attr.requirements_lock), + "--requirements_lock_label", + str(rctx.attr.requirements_lock), # pass quiet and timeout args through to child repos. "--quiet", str(rctx.attr.quiet), diff --git a/tools/bazel_integration_test/update_deleted_packages.sh b/tools/bazel_integration_test/update_deleted_packages.sh index 95a850073a..8a215c6d4a 100755 --- a/tools/bazel_integration_test/update_deleted_packages.sh +++ b/tools/bazel_integration_test/update_deleted_packages.sh @@ -7,5 +7,5 @@ set -eux DIR="$(dirname $0)/../.." # The sed -i.bak pattern is compatible between macos and linux sed -i.bak "/^[^#].*--deleted_packages/s#=.*#=$(\ - find examples/*/* \( -name BUILD -or -name BUILD.bazel \) | xargs -n 1 dirname | paste -sd, -\ + find examples/*/* tests/*/* \( -name BUILD -or -name BUILD.bazel \) | xargs -n 1 dirname | paste -sd, -\ )#" $DIR/.bazelrc && rm .bazelrc.bak