From 6664a12f4f91118eacae6cb835e76b75f1d25b39 Mon Sep 17 00:00:00 2001 From: Richard Levasseur Date: Fri, 15 Dec 2023 14:05:18 -0800 Subject: [PATCH] fix(bzlmod pip.parse): allow requirements with extras When a package has extras, the requirements parsing discards the extra portion of the name and just returns the base name. When a repository is created for each entry, this means the same name is used for multiple repositories. Under WORKSPACE builds, duplicate repository names aren't an error. It appears the last defined repo takes affect. Under bzlmod, duplicate repo names are an error. To fix, mimic the last-defined-wins behavior in bzlmod by using a map to dedupe the package names. Fixes https://github.com/bazelbuild/rules_python/issues/1615 --- CHANGELOG.md | 2 + examples/bzlmod/MODULE.bazel | 11 +++ .../tests/dupe_requirements/BUILD.bazel | 19 ++++ .../dupe_requirements_test.py | 6 ++ .../tests/dupe_requirements/requirements.in | 2 + .../tests/dupe_requirements/requirements.txt | 97 +++++++++++++++++++ python/private/bzlmod/pip.bzl | 14 ++- 7 files changed, 150 insertions(+), 1 deletion(-) create mode 100644 examples/bzlmod/tests/dupe_requirements/BUILD.bazel create mode 100644 examples/bzlmod/tests/dupe_requirements/dupe_requirements_test.py create mode 100644 examples/bzlmod/tests/dupe_requirements/requirements.in create mode 100644 examples/bzlmod/tests/dupe_requirements/requirements.txt diff --git a/CHANGELOG.md b/CHANGELOG.md index 7cb5fde7a4..a0b4a721d0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -44,6 +44,8 @@ A brief description of the categories of changes: platform using `download_only = True` feature. * (bzlmod pip.parse) The `pip.parse(python_interpreter)` arg now works for specifying a local system interpreter. +* (bzlmod pip.parse) Requirements files with duplicate entries for the same + package (e.g. one for the package, one for an extra) now work. [0.XX.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.XX.0 diff --git a/examples/bzlmod/MODULE.bazel b/examples/bzlmod/MODULE.bazel index 9ce84ee78b..240bb28022 100644 --- a/examples/bzlmod/MODULE.bazel +++ b/examples/bzlmod/MODULE.bazel @@ -167,3 +167,14 @@ local_path_override( module_name = "other_module", path = "other_module", ) + +# ===== +# Config for testing duplicate packages in requirements +# ===== +# +pip.parse( + hub_name = "dupe_requirements", + python_version = "3.9", # Must match whatever is marked is_default=True + requirements_lock = "//tests/dupe_requirements:requirements.txt", +) +use_repo(pip, "dupe_requirements") diff --git a/examples/bzlmod/tests/dupe_requirements/BUILD.bazel b/examples/bzlmod/tests/dupe_requirements/BUILD.bazel new file mode 100644 index 0000000000..72bd46a66a --- /dev/null +++ b/examples/bzlmod/tests/dupe_requirements/BUILD.bazel @@ -0,0 +1,19 @@ +load("@rules_python//python:py_test.bzl", "py_test") +load("@rules_python//python:pip.bzl", "compile_pip_requirements") + +py_test( + name = "dupe_requirements_test", + srcs = ["dupe_requirements_test.py"], + deps = [ + "@dupe_requirements//pyjwt", + ], +) + +compile_pip_requirements( + name = "requirements", + src = "requirements.in", + requirements_txt = "requirements.txt", + # This is to make the requirements diff test not run on CI. The content we + # need in requirements.txt isn't exactly what will be generated. + tags = ["manual"], +) diff --git a/examples/bzlmod/tests/dupe_requirements/dupe_requirements_test.py b/examples/bzlmod/tests/dupe_requirements/dupe_requirements_test.py new file mode 100644 index 0000000000..70fca748cd --- /dev/null +++ b/examples/bzlmod/tests/dupe_requirements/dupe_requirements_test.py @@ -0,0 +1,6 @@ + +# There's nothing to test at runtime. Building indicates success. +# Just import the relevant modules as a basic check. +import jwt +import cryptography + diff --git a/examples/bzlmod/tests/dupe_requirements/requirements.in b/examples/bzlmod/tests/dupe_requirements/requirements.in new file mode 100644 index 0000000000..b1f623395a --- /dev/null +++ b/examples/bzlmod/tests/dupe_requirements/requirements.in @@ -0,0 +1,2 @@ +pyjwt +pyjwt[crypto] diff --git a/examples/bzlmod/tests/dupe_requirements/requirements.txt b/examples/bzlmod/tests/dupe_requirements/requirements.txt new file mode 100644 index 0000000000..785f556624 --- /dev/null +++ b/examples/bzlmod/tests/dupe_requirements/requirements.txt @@ -0,0 +1,97 @@ +# +# This file is manually tweaked output from the automatic generation. +# To generate: +# 1. bazel run //tests/dupe_requirements:requirements.update +# 2. Then copy/paste the pyjtw lines so there are duplicates +# +pyjwt==2.8.0 \ + --hash=sha256:57e28d156e3d5c10088e0c68abb90bfac3df82b40a71bd0daa20c65ccd5c23de \ + --hash=sha256:59127c392cc44c2da5bb3192169a91f429924e17aff6534d70fdc02ab3e04320 + # via -r tests/dupe_requirements/requirements.in +pyjwt[crypto]==2.8.0 \ + --hash=sha256:57e28d156e3d5c10088e0c68abb90bfac3df82b40a71bd0daa20c65ccd5c23de \ + --hash=sha256:59127c392cc44c2da5bb3192169a91f429924e17aff6534d70fdc02ab3e04320 + # via -r tests/dupe_requirements/requirements.in +cffi==1.16.0 \ + --hash=sha256:0c9ef6ff37e974b73c25eecc13952c55bceed9112be2d9d938ded8e856138bcc \ + --hash=sha256:131fd094d1065b19540c3d72594260f118b231090295d8c34e19a7bbcf2e860a \ + --hash=sha256:1b8ebc27c014c59692bb2664c7d13ce7a6e9a629be20e54e7271fa696ff2b417 \ + --hash=sha256:2c56b361916f390cd758a57f2e16233eb4f64bcbeee88a4881ea90fca14dc6ab \ + --hash=sha256:2d92b25dbf6cae33f65005baf472d2c245c050b1ce709cc4588cdcdd5495b520 \ + --hash=sha256:31d13b0f99e0836b7ff893d37af07366ebc90b678b6664c955b54561fc36ef36 \ + --hash=sha256:32c68ef735dbe5857c810328cb2481e24722a59a2003018885514d4c09af9743 \ + --hash=sha256:3686dffb02459559c74dd3d81748269ffb0eb027c39a6fc99502de37d501faa8 \ + --hash=sha256:582215a0e9adbe0e379761260553ba11c58943e4bbe9c36430c4ca6ac74b15ed \ + --hash=sha256:5b50bf3f55561dac5438f8e70bfcdfd74543fd60df5fa5f62d94e5867deca684 \ + --hash=sha256:5bf44d66cdf9e893637896c7faa22298baebcd18d1ddb6d2626a6e39793a1d56 \ + --hash=sha256:6602bc8dc6f3a9e02b6c22c4fc1e47aa50f8f8e6d3f78a5e16ac33ef5fefa324 \ + --hash=sha256:673739cb539f8cdaa07d92d02efa93c9ccf87e345b9a0b556e3ecc666718468d \ + --hash=sha256:68678abf380b42ce21a5f2abde8efee05c114c2fdb2e9eef2efdb0257fba1235 \ + --hash=sha256:68e7c44931cc171c54ccb702482e9fc723192e88d25a0e133edd7aff8fcd1f6e \ + --hash=sha256:6b3d6606d369fc1da4fd8c357d026317fbb9c9b75d36dc16e90e84c26854b088 \ + --hash=sha256:748dcd1e3d3d7cd5443ef03ce8685043294ad6bd7c02a38d1bd367cfd968e000 \ + --hash=sha256:7651c50c8c5ef7bdb41108b7b8c5a83013bfaa8a935590c5d74627c047a583c7 \ + --hash=sha256:7b78010e7b97fef4bee1e896df8a4bbb6712b7f05b7ef630f9d1da00f6444d2e \ + --hash=sha256:7e61e3e4fa664a8588aa25c883eab612a188c725755afff6289454d6362b9673 \ + --hash=sha256:80876338e19c951fdfed6198e70bc88f1c9758b94578d5a7c4c91a87af3cf31c \ + --hash=sha256:8895613bcc094d4a1b2dbe179d88d7fb4a15cee43c052e8885783fac397d91fe \ + --hash=sha256:88e2b3c14bdb32e440be531ade29d3c50a1a59cd4e51b1dd8b0865c54ea5d2e2 \ + --hash=sha256:8f8e709127c6c77446a8c0a8c8bf3c8ee706a06cd44b1e827c3e6a2ee6b8c098 \ + --hash=sha256:9cb4a35b3642fc5c005a6755a5d17c6c8b6bcb6981baf81cea8bfbc8903e8ba8 \ + --hash=sha256:9f90389693731ff1f659e55c7d1640e2ec43ff725cc61b04b2f9c6d8d017df6a \ + --hash=sha256:a09582f178759ee8128d9270cd1344154fd473bb77d94ce0aeb2a93ebf0feaf0 \ + --hash=sha256:a6a14b17d7e17fa0d207ac08642c8820f84f25ce17a442fd15e27ea18d67c59b \ + --hash=sha256:a72e8961a86d19bdb45851d8f1f08b041ea37d2bd8d4fd19903bc3083d80c896 \ + --hash=sha256:abd808f9c129ba2beda4cfc53bde801e5bcf9d6e0f22f095e45327c038bfe68e \ + --hash=sha256:ac0f5edd2360eea2f1daa9e26a41db02dd4b0451b48f7c318e217ee092a213e9 \ + --hash=sha256:b29ebffcf550f9da55bec9e02ad430c992a87e5f512cd63388abb76f1036d8d2 \ + --hash=sha256:b2ca4e77f9f47c55c194982e10f058db063937845bb2b7a86c84a6cfe0aefa8b \ + --hash=sha256:b7be2d771cdba2942e13215c4e340bfd76398e9227ad10402a8767ab1865d2e6 \ + --hash=sha256:b84834d0cf97e7d27dd5b7f3aca7b6e9263c56308ab9dc8aae9784abb774d404 \ + --hash=sha256:b86851a328eedc692acf81fb05444bdf1891747c25af7529e39ddafaf68a4f3f \ + --hash=sha256:bcb3ef43e58665bbda2fb198698fcae6776483e0c4a631aa5647806c25e02cc0 \ + --hash=sha256:c0f31130ebc2d37cdd8e44605fb5fa7ad59049298b3f745c74fa74c62fbfcfc4 \ + --hash=sha256:c6a164aa47843fb1b01e941d385aab7215563bb8816d80ff3a363a9f8448a8dc \ + --hash=sha256:d8a9d3ebe49f084ad71f9269834ceccbf398253c9fac910c4fd7053ff1386936 \ + --hash=sha256:db8e577c19c0fda0beb7e0d4e09e0ba74b1e4c092e0e40bfa12fe05b6f6d75ba \ + --hash=sha256:dc9b18bf40cc75f66f40a7379f6a9513244fe33c0e8aa72e2d56b0196a7ef872 \ + --hash=sha256:e09f3ff613345df5e8c3667da1d918f9149bd623cd9070c983c013792a9a62eb \ + --hash=sha256:e4108df7fe9b707191e55f33efbcb2d81928e10cea45527879a4749cbe472614 \ + --hash=sha256:e6024675e67af929088fda399b2094574609396b1decb609c55fa58b028a32a1 \ + --hash=sha256:e70f54f1796669ef691ca07d046cd81a29cb4deb1e5f942003f401c0c4a2695d \ + --hash=sha256:e715596e683d2ce000574bae5d07bd522c781a822866c20495e52520564f0969 \ + --hash=sha256:e760191dd42581e023a68b758769e2da259b5d52e3103c6060ddc02c9edb8d7b \ + --hash=sha256:ed86a35631f7bfbb28e108dd96773b9d5a6ce4811cf6ea468bb6a359b256b1e4 \ + --hash=sha256:ee07e47c12890ef248766a6e55bd38ebfb2bb8edd4142d56db91b21ea68b7627 \ + --hash=sha256:fa3a0128b152627161ce47201262d3140edb5a5c3da88d73a1b790a959126956 \ + --hash=sha256:fcc8eb6d5902bb1cf6dc4f187ee3ea80a1eba0a89aba40a5cb20a5087d961357 + # via cryptography +cryptography==41.0.7 \ + --hash=sha256:079b85658ea2f59c4f43b70f8119a52414cdb7be34da5d019a77bf96d473b960 \ + --hash=sha256:09616eeaef406f99046553b8a40fbf8b1e70795a91885ba4c96a70793de5504a \ + --hash=sha256:13f93ce9bea8016c253b34afc6bd6a75993e5c40672ed5405a9c832f0d4a00bc \ + --hash=sha256:37a138589b12069efb424220bf78eac59ca68b95696fc622b6ccc1c0a197204a \ + --hash=sha256:3c78451b78313fa81607fa1b3f1ae0a5ddd8014c38a02d9db0616133987b9cdf \ + --hash=sha256:43f2552a2378b44869fe8827aa19e69512e3245a219104438692385b0ee119d1 \ + --hash=sha256:48a0476626da912a44cc078f9893f292f0b3e4c739caf289268168d8f4702a39 \ + --hash=sha256:49f0805fc0b2ac8d4882dd52f4a3b935b210935d500b6b805f321addc8177406 \ + --hash=sha256:5429ec739a29df2e29e15d082f1d9ad683701f0ec7709ca479b3ff2708dae65a \ + --hash=sha256:5a1b41bc97f1ad230a41657d9155113c7521953869ae57ac39ac7f1bb471469a \ + --hash=sha256:68a2dec79deebc5d26d617bfdf6e8aab065a4f34934b22d3b5010df3ba36612c \ + --hash=sha256:7a698cb1dac82c35fcf8fe3417a3aaba97de16a01ac914b89a0889d364d2f6be \ + --hash=sha256:841df4caa01008bad253bce2a6f7b47f86dc9f08df4b433c404def869f590a15 \ + --hash=sha256:90452ba79b8788fa380dfb587cca692976ef4e757b194b093d845e8d99f612f2 \ + --hash=sha256:928258ba5d6f8ae644e764d0f996d61a8777559f72dfeb2eea7e2fe0ad6e782d \ + --hash=sha256:af03b32695b24d85a75d40e1ba39ffe7db7ffcb099fe507b39fd41a565f1b157 \ + --hash=sha256:b640981bf64a3e978a56167594a0e97db71c89a479da8e175d8bb5be5178c003 \ + --hash=sha256:c5ca78485a255e03c32b513f8c2bc39fedb7f5c5f8535545bdc223a03b24f248 \ + --hash=sha256:c7f3201ec47d5207841402594f1d7950879ef890c0c495052fa62f58283fde1a \ + --hash=sha256:d5ec85080cce7b0513cfd233914eb8b7bbd0633f1d1703aa28d1dd5a72f678ec \ + --hash=sha256:d6c391c021ab1f7a82da5d8d0b3cee2f4b2c455ec86c8aebbc84837a631ff309 \ + --hash=sha256:e3114da6d7f95d2dee7d3f4eec16dacff819740bbab931aff8648cb13c5ff5e7 \ + --hash=sha256:f983596065a18a2183e7f79ab3fd4c475205b839e02cbc0efbbf9666c4b3083d + # via pyjwt +pycparser==2.21 \ + --hash=sha256:8ee45429555515e1f6b185e78100aea234072576aa43ab53aefcae078162fca9 \ + --hash=sha256:e644fdec12f7872f86c58ff790da456218b10f863970249516d60a5eaca77206 + # via cffi diff --git a/python/private/bzlmod/pip.bzl b/python/private/bzlmod/pip.bzl index 1b25d341f0..6d45a26d7b 100644 --- a/python/private/bzlmod/pip.bzl +++ b/python/private/bzlmod/pip.bzl @@ -109,7 +109,19 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides): # needed for the whl_libary declarations below. requirements_lock_content = module_ctx.read(requrements_lock) parse_result = parse_requirements(requirements_lock_content) - requirements = parse_result.requirements + + # Replicate a surprising behavior that WORKSPACE builds allowed: + # Defining a repo with the same name multiple times, but only the last + # definition is respected. + # The requirement lines might have duplicate names because lines for extras + # are returned as just the base package name. e.g., `foo[bar]` results + # in an entry like `("foo", "foo[bar] == 1.0 ...")`. + requirements = { + normalize_name(entry[0]): entry + # The WORKSPACE pip_parse sorted entries, so mimic that ordering. + for entry in sorted(parse_result.requirements) + }.values() + extra_pip_args = pip_attr.extra_pip_args + parse_result.options if hub_name not in whl_map: