fix: Fix incorrectly generated `Required-Dist` when specifying requirements with markers in extra_requires in py_wheel rule (#2200)
Currently, if extra requirements are provided via a file, it is allowed
to contain markers, but if it is passed via extra_requires field, it
will run into error since the extra in this case is blindly added in the
following line in `py_wheel.bzl`
```
metadata_contents.append(
"Requires-Dist: %s; extra == '%s'" % (requirement, option),
)
```
Thus, this PR adds a post-process for this case, to make it possible to
pass something like
```
extra_requires = {"example": [
"pyyaml>=6.0.0,!=6.0.1",
'toml; (python_version == "3.11" or python_version == "3.12") and python_version != "3.8"',
'wheel; python_version == "3.11" or python_version == "3.12" ',
]},
```
to `py_wheel`diff --git a/CHANGELOG.md b/CHANGELOG.md
index d410575..e2c54e3 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -51,6 +51,8 @@
{obj}`--bootstrap_impl=script`. This fixes invocations using non-sandboxed
test execution with `--enable_runfiles=false --build_runfile_manifests=true`.
([#2186](https://github.com/bazelbuild/rules_python/issues/2186)).
+* (py_wheel) Fix incorrectly generated `Required-Dist` when specifying requirements with markers
+ in extra_requires in py_wheel rule.
### Added
diff --git a/examples/wheel/BUILD.bazel b/examples/wheel/BUILD.bazel
index aa063ce..1eaf035 100644
--- a/examples/wheel/BUILD.bazel
+++ b/examples/wheel/BUILD.bazel
@@ -333,6 +333,27 @@
version = "0.0.1",
)
+py_wheel(
+ name = "extra_requires",
+ distribution = "extra_requires",
+ extra_requires = {"example": [
+ "pyyaml>=6.0.0,!=6.0.1",
+ 'toml; (python_version == "3.11" or python_version == "3.12") and python_version != "3.8"',
+ 'wheel; python_version == "3.11" or python_version == "3.12" ',
+ ]},
+ python_tag = "py3",
+ # py_wheel can use text files to specify their requirements. This
+ # can be convenient for users of `compile_pip_requirements` who have
+ # granular `requirements.in` files per package.
+ requires = [
+ "tomli>=2.0.0",
+ "starlark",
+ 'pytest; python_version != "3.8"',
+ ],
+ version = "0.0.1",
+ deps = [":example_pkg"],
+)
+
py_test(
name = "wheel_test",
srcs = ["wheel_test.py"],
@@ -341,6 +362,7 @@
":custom_package_root_multi_prefix",
":custom_package_root_multi_prefix_reverse_order",
":customized",
+ ":extra_requires",
":filename_escaping",
":minimal_data_files",
":minimal_with_py_library",
diff --git a/examples/wheel/wheel_test.py b/examples/wheel/wheel_test.py
index 66ebd50..7212423 100644
--- a/examples/wheel/wheel_test.py
+++ b/examples/wheel/wheel_test.py
@@ -489,6 +489,36 @@
],
)
+ def test_extra_requires(self):
+ filename = self._get_path("extra_requires-0.0.1-py3-none-any.whl")
+
+ with zipfile.ZipFile(filename) as zf:
+ self.assertAllEntriesHasReproducibleMetadata(zf)
+ metadata_file = None
+ for f in zf.namelist():
+ if os.path.basename(f) == "METADATA":
+ metadata_file = f
+ self.assertIsNotNone(metadata_file)
+
+ requires = []
+ with zf.open(metadata_file) as fp:
+ for line in fp:
+ if line.startswith(b"Requires-Dist:"):
+ requires.append(line.decode("utf-8").strip())
+
+ print(requires)
+ self.assertEqual(
+ [
+ "Requires-Dist: tomli>=2.0.0",
+ "Requires-Dist: starlark",
+ 'Requires-Dist: pytest; python_version != "3.8"',
+ "Requires-Dist: pyyaml!=6.0.1,>=6.0.0; extra == 'example'",
+ 'Requires-Dist: toml; ((python_version == "3.11" or python_version == "3.12") and python_version != "3.8") and extra == \'example\'',
+ 'Requires-Dist: wheel; (python_version == "3.11" or python_version == "3.12") and extra == \'example\'',
+ ],
+ requires,
+ )
+
if __name__ == "__main__":
unittest.main()
diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py
index 8fa3e02..68578b8 100644
--- a/tools/wheelmaker.py
+++ b/tools/wheelmaker.py
@@ -537,9 +537,34 @@
# Search for any `Requires-Dist` entries that refer to other files and
# expand them.
+
+ def get_new_requirement_line(reqs_text, extra):
+ req = Requirement(reqs_text.strip())
+ if req.marker:
+ if extra:
+ return f"Requires-Dist: {req.name}{req.specifier}; ({req.marker}) and {extra}"
+ else:
+ return f"Requires-Dist: {req.name}{req.specifier}; {req.marker}"
+ else:
+ return f"Requires-Dist: {req.name}{req.specifier}; {extra}".strip(" ;")
+
for meta_line in metadata.splitlines():
- if not meta_line.startswith("Requires-Dist: @"):
+ if not meta_line.startswith("Requires-Dist: "):
continue
+
+ if not meta_line[len("Requires-Dist: ") :].startswith("@"):
+ # This is a normal requirement.
+ package, _, extra = meta_line[len("Requires-Dist: ") :].rpartition(";")
+ if not package:
+ # This is when the package requirement does not have markers.
+ continue
+ extra = extra.strip()
+ metadata = metadata.replace(
+ meta_line, get_new_requirement_line(package, extra)
+ )
+ continue
+
+ # This is a requirement that refers to a file.
file, _, extra = meta_line[len("Requires-Dist: @") :].partition(";")
extra = extra.strip()
@@ -552,20 +577,7 @@
# Strip any comments
reqs_text, _, _ = reqs_text.partition("#")
- req = Requirement(reqs_text.strip())
- if req.marker:
- if extra:
- reqs.append(
- f"Requires-Dist: {req.name}{req.specifier}; ({req.marker}) and {extra}"
- )
- else:
- reqs.append(
- f"Requires-Dist: {req.name}{req.specifier}; {req.marker}"
- )
- else:
- reqs.append(
- f"Requires-Dist: {req.name}{req.specifier}; {extra}".strip(" ;")
- )
+ reqs.append(get_new_requirement_line(reqs_text, extra))
metadata = metadata.replace(meta_line, "\n".join(reqs))
