fix: Fix incorrectly generated Required-Dist when specifying requirements with markers in extra_requires in py_wheel rule

CHANGELOG.md

@@ -48,6 +48,8 @@ A brief description of the categories of changes:
   {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

examples/wheel/BUILD.bazel

@@ -333,6 +333,27 @@ py_wheel(
     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 @@ py_test(
         ":custom_package_root_multi_prefix",
         ":custom_package_root_multi_prefix_reverse_order",
         ":customized",
+        ":extra_requires",
         ":filename_escaping",
         ":minimal_data_files",
         ":minimal_with_py_library",

examples/wheel/wheel_test.py

@@ -489,6 +489,36 @@ def test_minimal_data_files(self):
                 ],
             )
 
+    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()

tools/wheelmaker.py

@@ -537,9 +537,34 @@ def main() -> None:
 
         # 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 @@ def main() -> None:
                 # 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))