change approach for vendoring parsed requirements

.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"

.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
 

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**

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",

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",
+)

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

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()

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
+        )

examples/pip_parse_vendored/requirements.in

@@ -0,0 +1 @@
+requests

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

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

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,

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)

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),

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