Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .bazelci/presubmit.yml
Original file line number Diff line number Diff line change
Expand Up @@ -36,5 +36,6 @@ 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"
- "-//tests:pip_deps_example"
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

why doesn't it work on Windows?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I am not sure. I guessed it was because bazel_integration_test did not work well on Windows, which is likely wrong.

The error is caused by os.chdir("external\\unpinned_pip") reporting no such directory. I do not have a Windows machine to debug it at the moment. Would you mind take a look if possible?

test_flags:
- "--test_tag_filters=-fix-windows"
4 changes: 2 additions & 2 deletions .bazelrc
Original file line number Diff line number Diff line change
Expand Up @@ -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_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
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,tests/pip_deps
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,tests/pip_deps

test --test_output=errors

Expand Down
9 changes: 7 additions & 2 deletions python/pip_install/pip_compile.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,12 @@
sys.exit(1)

requirements_in = os.path.relpath(sys.argv.pop(1))
requirements_txt = sys.argv.pop(1)
requirements_txt = os.path.relpath(sys.argv.pop(1))
parts = requirements_in.split(os.path.sep, 2)
if parts[0] == "external":
requirements_in = parts[2]
requirements_txt = requirements_txt if "BUILD_WORKSPACE_DIRECTORY" in os.environ else os.path.join("..", "..", requirements_txt)
os.chdir(os.path.join(parts[0], parts[1]))
update_target_label = sys.argv.pop(1)

# Before loading click, set the locale for its parser.
Expand Down Expand Up @@ -49,7 +54,7 @@
#
# Changing to the WORKSPACE root avoids 'file not found' errors when the `.update` target is run
# from different directories within the WORKSPACE.
os.chdir(os.environ["BUILD_WORKSPACE_DIRECTORY"])
requirements_txt = os.path.join(os.environ["BUILD_WORKSPACE_DIRECTORY"], requirements_txt)
else:
err_msg = (
"Expected to find BUILD_WORKSPACE_DIRECTORY (running under `bazel run`) or "
Expand Down
5 changes: 5 additions & 0 deletions tests/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -8,3 +8,8 @@ bazel_integration_test(
name = "pip_repository_entry_points_example",
timeout = "long",
)

bazel_integration_test(
name = "pip_deps_example",
timeout = "long",
)
19 changes: 19 additions & 0 deletions tests/pip_deps/BUILD
Original file line number Diff line number Diff line change
@@ -0,0 +1,19 @@
filegroup(
name = "requirements_txt",
srcs = ["requirements.txt"],
visibility = ["//visibility:public"],
)

test_suite(
name = "external_tests",
tests = [
"@unpinned_pip//:pin_test",
],
)

sh_test(
name = "no_external_test",
srcs = ["test.sh"],
args = ["$(rootpath :requirements_txt)"],
data = [":requirements_txt"],
)
1 change: 1 addition & 0 deletions tests/pip_deps/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1 @@
Run `bazel run @unpinned_pip//:pin.update` to keep `PIP_PACKAGES` in `//:WORKSPACE` in sync with `//:requirements.txt`.
26 changes: 26 additions & 0 deletions tests/pip_deps/WORKSPACE
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
workspace(name = "pip_deps")

local_repository(
name = "rules_python",
path = "../..",
)

load("@rules_python//python:repositories.bzl", "python_register_toolchains")

# This toolchain is explicitly 3.10 while `rules_python` is 3.9 to act as
# a regression test, ensuring 3.10 is functional
python_register_toolchains(
name = "python310",
python_version = "3.10",
)

load("@python310//:defs.bzl", "interpreter")
load("//:pip_deps.bzl", "pip_deps")

PIP_PACKAGES = {"sphinx": "4.5.0"}

pip_deps(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the intent of the pip_parse_vendored example introduced in 12662b6

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

this was the intent of the pip_parse_vendored example introduced in 12662b6

The major issue with pip_parse_vendored and the approach used for golang is that the meta data is unknown to Bazel. With pip_deps, one can save all the metadata in a .bzl file so that the downstream can consume it, e.g., use it to resolve diamond dependency.

I did something similar with golang dependencies at https://github.com/BoleynSu-Org/monorepo/blob/main/configs/deps/go_deps.bzl.

could you just update that example instead of introducing another e2e test? They are kinda slow so it would be better not to accumulate more.

Sure, I think I can merge it into another e2e test.

Also this doc 12662b6#diff-b92f94a67df862cde9714ff224afa676e39fe02da4fd4c6993daf684fbbc7cbdR182-R186 should maybe be updated?

name = "pip",
packages = PIP_PACKAGES,
python_interpreter_target = interpreter,
)
55 changes: 55 additions & 0 deletions tests/pip_deps/pip_deps.bzl
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
""" A demo implementation for pip_deps which provides @unpinned_pip//:pin. """

load("@rules_python//python:pip.bzl", "pip_parse")

def _requirements_in_impl(repository_ctx):
repository_ctx.file(
"requirements.in",
content = "".join(["{package} == {version}\n".format(
package = package,
version = version,
) for (package, version) in repository_ctx.attr.packages.items()]),
)
repository_ctx.file("WORKSPACE", content = "")
repository_ctx.file("BUILD", content = """
load("@rules_python//python:pip.bzl", "compile_pip_requirements")
compile_pip_requirements(
name = "pin",
extra_args = ["--allow-unsafe"],
requirements_in = "requirements.in",
requirements_txt = "@{workspace_name}//{package}:{name}",
)
""".format(
workspace_name = repository_ctx.attr.requirements_lock.workspace_name,
package = repository_ctx.attr.requirements_lock.package,
name = repository_ctx.attr.requirements_lock.name,
))

_requirements_in = repository_rule(
implementation = _requirements_in_impl,
attrs = {
"packages": attr.string_dict(),
"requirements_lock": attr.label(allow_single_file = True),
},
)

def pip_deps(
*,
name = "pip",
packages = {},
requirements_lock_target = Label("//:requirements_txt"),
requirements_lock_file = Label("//:requirements.txt"),
python_interpreter_target = None,
**kwargs):
_requirements_in(
name = "unpinned_" + name,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

Copy link
Contributor Author

@BoleynSu BoleynSu May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think the term "unpinned" may be inaccurate here. I'm not sure how it compares with rules_jvm_external, but here, the existing locked requirements file is still an input to pip-compile, so any already-pinned transitive dependencies will remain pinned to the same version. Only versions which no longer satisfy the constraints in requirements.in will actually change.

The input to _requirements_in is packages and output is a requirements.in file. None of them have anything to do with the locked requirements file.

If the behavior is different from rules_jvm_external, then maybe another name would be better?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

There's a requirements_lock attribute here that gets passed to pip_compile. Am I missing something?

Copy link
Contributor Author

@BoleynSu BoleynSu May 5, 2022

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see. Yes, the locked requirements file is also part of the input.

It seems that [1] we can pass --upgrade to achive this. It only read the locked file when there is no --upgrade. I am also wondering why --upgrade is not passed already? I think people are expecting this. We would better document it somewhere.

Also, in my usecase the requirements.txt is always generated so I did not ever notice it.

[1] https://github.com/jazzband/pip-tools/blob/master/piptools/scripts/compile.py#L352

packages = packages,
requirements_lock = requirements_lock_target,
)
pip_parse(
name = name,
requirements_lock = requirements_lock_file,
python_interpreter_target = python_interpreter_target,
**kwargs
)
136 changes: 136 additions & 0 deletions tests/pip_deps/requirements.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
#
# This file is autogenerated by pip-compile with python 3.10
# To update, run:
#
# bazel run //:pin.update
#
alabaster==0.7.12 \
--hash=sha256:446438bdcca0e05bd45ea2de1668c1d9b032e1a9154c2c259092d77031ddd359 \
--hash=sha256:a661d72d58e6ea8a57f7a86e37d86716863ee5e92788398526d58b26a4e4dc02
# via sphinx
babel==2.10.1 \
--hash=sha256:3f349e85ad3154559ac4930c3918247d319f21910d5ce4b25d439ed8693b98d2 \
--hash=sha256:98aeaca086133efb3e1e2aad0396987490c8425929ddbcfe0550184fdc54cd13
# via sphinx
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
docutils==0.17.1 \
--hash=sha256:686577d2e4c32380bb50cbb22f575ed742d58168cee37e99117a854bcd88f125 \
--hash=sha256:cf316c8370a737a022b72b56874f6602acf974a37a9fba42ec2876387549fc61
# via sphinx
idna==3.3 \
--hash=sha256:84d9dd047ffa80596e0f246e2eab0b391788b0503584e8945f2368256d2735ff \
--hash=sha256:9d643ff0a55b762d5cdb124b8eaa99c66322e2157b69160bc32796e824360e6d
# via requests
imagesize==1.3.0 \
--hash=sha256:1db2f82529e53c3e929e8926a1fa9235aa82d0bd0c580359c67ec31b2fddaa8c \
--hash=sha256:cd1750d452385ca327479d45b64d9c7729ecf0b3969a58148298c77092261f9d
# via sphinx
jinja2==3.1.2 \
--hash=sha256:31351a702a408a9e7595a8fc6150fc3f43bb6bf7e319770cbc0db9df9437e852 \
--hash=sha256:6088930bfe239f0e6710546ab9c19c9ef35e29792895fed6e6e31a023a182a61
# via sphinx
markupsafe==2.1.1 \
--hash=sha256:0212a68688482dc52b2d45013df70d169f542b7394fc744c02a57374a4207003 \
--hash=sha256:089cf3dbf0cd6c100f02945abeb18484bd1ee57a079aefd52cffd17fba910b88 \
--hash=sha256:10c1bfff05d95783da83491be968e8fe789263689c02724e0c691933c52994f5 \
--hash=sha256:33b74d289bd2f5e527beadcaa3f401e0df0a89927c1559c8566c066fa4248ab7 \
--hash=sha256:3799351e2336dc91ea70b034983ee71cf2f9533cdff7c14c90ea126bfd95d65a \
--hash=sha256:3ce11ee3f23f79dbd06fb3d63e2f6af7b12db1d46932fe7bd8afa259a5996603 \
--hash=sha256:421be9fbf0ffe9ffd7a378aafebbf6f4602d564d34be190fc19a193232fd12b1 \
--hash=sha256:43093fb83d8343aac0b1baa75516da6092f58f41200907ef92448ecab8825135 \
--hash=sha256:46d00d6cfecdde84d40e572d63735ef81423ad31184100411e6e3388d405e247 \
--hash=sha256:4a33dea2b688b3190ee12bd7cfa29d39c9ed176bda40bfa11099a3ce5d3a7ac6 \
--hash=sha256:4b9fe39a2ccc108a4accc2676e77da025ce383c108593d65cc909add5c3bd601 \
--hash=sha256:56442863ed2b06d19c37f94d999035e15ee982988920e12a5b4ba29b62ad1f77 \
--hash=sha256:671cd1187ed5e62818414afe79ed29da836dde67166a9fac6d435873c44fdd02 \
--hash=sha256:694deca8d702d5db21ec83983ce0bb4b26a578e71fbdbd4fdcd387daa90e4d5e \
--hash=sha256:6a074d34ee7a5ce3effbc526b7083ec9731bb3cbf921bbe1d3005d4d2bdb3a63 \
--hash=sha256:6d0072fea50feec76a4c418096652f2c3238eaa014b2f94aeb1d56a66b41403f \
--hash=sha256:6fbf47b5d3728c6aea2abb0589b5d30459e369baa772e0f37a0320185e87c980 \
--hash=sha256:7f91197cc9e48f989d12e4e6fbc46495c446636dfc81b9ccf50bb0ec74b91d4b \
--hash=sha256:86b1f75c4e7c2ac2ccdaec2b9022845dbb81880ca318bb7a0a01fbf7813e3812 \
--hash=sha256:8dc1c72a69aa7e082593c4a203dcf94ddb74bb5c8a731e4e1eb68d031e8498ff \
--hash=sha256:8e3dcf21f367459434c18e71b2a9532d96547aef8a871872a5bd69a715c15f96 \
--hash=sha256:8e576a51ad59e4bfaac456023a78f6b5e6e7651dcd383bcc3e18d06f9b55d6d1 \
--hash=sha256:96e37a3dc86e80bf81758c152fe66dbf60ed5eca3d26305edf01892257049925 \
--hash=sha256:97a68e6ada378df82bc9f16b800ab77cbf4b2fada0081794318520138c088e4a \
--hash=sha256:99a2a507ed3ac881b975a2976d59f38c19386d128e7a9a18b7df6fff1fd4c1d6 \
--hash=sha256:a49907dd8420c5685cfa064a1335b6754b74541bbb3706c259c02ed65b644b3e \
--hash=sha256:b09bf97215625a311f669476f44b8b318b075847b49316d3e28c08e41a7a573f \
--hash=sha256:b7bd98b796e2b6553da7225aeb61f447f80a1ca64f41d83612e6139ca5213aa4 \
--hash=sha256:b87db4360013327109564f0e591bd2a3b318547bcef31b468a92ee504d07ae4f \
--hash=sha256:bcb3ed405ed3222f9904899563d6fc492ff75cce56cba05e32eff40e6acbeaa3 \
--hash=sha256:d4306c36ca495956b6d568d276ac11fdd9c30a36f1b6eb928070dc5360b22e1c \
--hash=sha256:d5ee4f386140395a2c818d149221149c54849dfcfcb9f1debfe07a8b8bd63f9a \
--hash=sha256:dda30ba7e87fbbb7eab1ec9f58678558fd9a6b8b853530e176eabd064da81417 \
--hash=sha256:e04e26803c9c3851c931eac40c695602c6295b8d432cbe78609649ad9bd2da8a \
--hash=sha256:e1c0b87e09fa55a220f058d1d49d3fb8df88fbfab58558f1198e08c1e1de842a \
--hash=sha256:e72591e9ecd94d7feb70c1cbd7be7b3ebea3f548870aa91e2732960fa4d57a37 \
--hash=sha256:e8c843bbcda3a2f1e3c2ab25913c80a3c5376cd00c6e8c4a86a89a28c8dc5452 \
--hash=sha256:efc1913fd2ca4f334418481c7e595c00aad186563bbc1ec76067848c7ca0a933 \
--hash=sha256:f121a1420d4e173a5d96e47e9a0c0dcff965afdf1626d28de1460815f7c4ee7a \
--hash=sha256:fc7b548b17d238737688817ab67deebb30e8073c95749d55538ed473130ec0c7
# via jinja2
packaging==21.3 \
--hash=sha256:dd47c42927d89ab911e606518907cc2d3a1f38bbd026385970643f9c5b8ecfeb \
--hash=sha256:ef103e05f519cdc783ae24ea4e2e0f508a9c99b2d4969652eed6a2e1ea5bd522
# via sphinx
pygments==2.12.0 \
--hash=sha256:5eb116118f9612ff1ee89ac96437bb6b49e8f04d8a13b514ba26f620208e26eb \
--hash=sha256:dc9c10fb40944260f6ed4c688ece0cd2048414940f1cea51b8b226318411c519
# via sphinx
pyparsing==3.0.8 \
--hash=sha256:7bf433498c016c4314268d95df76c81b842a4cb2b276fa3312cfb1e1d85f6954 \
--hash=sha256:ef7b523f6356f763771559412c0d7134753f037822dad1b16945b7b846f7ad06
# via packaging
pytz==2022.1 \
--hash=sha256:1e760e2fe6a8163bc0b3d9a19c4f84342afa0a2affebfaa84b01b978a02ecaa7 \
--hash=sha256:e68985985296d9a66a881eb3193b0906246245294a881e7c8afe623866ac6a5c
# via babel
requests==2.27.1 \
--hash=sha256:68d7c56fd5a8999887728ef304a6d12edc7be74f1cfa47714fc8b414525c9a61 \
--hash=sha256:f22fa1e554c9ddfd16e6e41ac79759e17be9e492b3587efa038054674760e72d
# via sphinx
snowballstemmer==2.2.0 \
--hash=sha256:09b16deb8547d3412ad7b590689584cd0fe25ec8db3be37788be3810cbf19cb1 \
--hash=sha256:c8e1716e83cc398ae16824e5572ae04e0d9fc2c6b985fb0f900f5f0c96ecba1a
# via sphinx
sphinx==4.5.0 \
--hash=sha256:7bf8ca9637a4ee15af412d1a1d9689fec70523a68ca9bb9127c2f3eeb344e2e6 \
--hash=sha256:ebf612653238bcc8f4359627a9b7ce44ede6fdd75d9d30f68255c7383d3a6226
# via -r requirements.in
sphinxcontrib-applehelp==1.0.2 \
--hash=sha256:806111e5e962be97c29ec4c1e7fe277bfd19e9652fb1a4392105b43e01af885a \
--hash=sha256:a072735ec80e7675e3f432fcae8610ecf509c5f1869d17e2eecff44389cdbc58
# via sphinx
sphinxcontrib-devhelp==1.0.2 \
--hash=sha256:8165223f9a335cc1af7ffe1ed31d2871f325254c0423bc0c4c7cd1c1e4734a2e \
--hash=sha256:ff7f1afa7b9642e7060379360a67e9c41e8f3121f2ce9164266f61b9f4b338e4
# via sphinx
sphinxcontrib-htmlhelp==2.0.0 \
--hash=sha256:d412243dfb797ae3ec2b59eca0e52dac12e75a241bf0e4eb861e450d06c6ed07 \
--hash=sha256:f5f8bb2d0d629f398bf47d0d69c07bc13b65f75a81ad9e2f71a63d4b7a2f6db2
# via sphinx
sphinxcontrib-jsmath==1.0.1 \
--hash=sha256:2ec2eaebfb78f3f2078e73666b1415417a116cc848b72e5172e596c871103178 \
--hash=sha256:a9925e4a4587247ed2191a22df5f6970656cb8ca2bd6284309578f2153e0c4b8
# via sphinx
sphinxcontrib-qthelp==1.0.3 \
--hash=sha256:4c33767ee058b70dba89a6fc5c1892c0d57a54be67ddd3e7875a18d14cba5a72 \
--hash=sha256:bd9fc24bcb748a8d51fd4ecaade681350aa63009a347a8c14e637895444dfab6
# via sphinx
sphinxcontrib-serializinghtml==1.1.5 \
--hash=sha256:352a9a00ae864471d3a7ead8d7d79f5fc0b57e8b3f95e9867eb9eb28999b92fd \
--hash=sha256:aa5f6de5dfdf809ef505c4895e51ef5c9eac17d0f287933eb49ec495280b6952
# via sphinx
urllib3==1.26.9 \
--hash=sha256:44ece4d53fb1706f667c9bd1c648f5469a2ec925fcf3a776667042d645472c14 \
--hash=sha256:aabaf16477806a5e1dd19aa41f8c2b7950dd3c746362d7e3223dbe6de6ac448e
# via requests
4 changes: 4 additions & 0 deletions tests/pip_deps/test.sh
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
#!/bin/bash
set -euo pipefail

! grep external "$1"