From 63f0580b8a242f9a0ed4b73c093e6a50a2e864c5 Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:03:08 -0400 Subject: [PATCH 01/12] fix(packaging)!: Format `METADATA` correctly if given empty `requires_file` --- tools/wheelmaker.py | 42 +++++++++++++++++++++++------------------- 1 file changed, 23 insertions(+), 19 deletions(-) diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index 23b18eca5f..96371c5562 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -568,39 +568,43 @@ def get_new_requirement_line(reqs_text, extra): else: return f"Requires-Dist: {req.name}{req.specifier}; {extra}".strip(" ;") + processed_metadata_lines = [] + for meta_line in metadata.splitlines(): if not meta_line.startswith("Requires-Dist: "): - continue + processed_metadata_lines.append(meta_line) - if not meta_line[len("Requires-Dist: ") :].startswith("@"): + else 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 + processed_metadata_lines(meta_line) + else: + extra = extra.strip() + processed_metadata_lines.append(get_new_requirement_line(package, extra)) # This is a requirement that refers to a file. - file, _, extra = meta_line[len("Requires-Dist: @") :].partition(";") - extra = extra.strip() + else: + file, _, extra = meta_line[len("Requires-Dist: @") :].partition(";") + extra = extra.strip() - reqs = [] - for reqs_line in Path(file).read_text(encoding="utf-8").splitlines(): - reqs_text = reqs_line.strip() - if not reqs_text or reqs_text.startswith(("#", "-")): - continue + reqs = [] + for reqs_line in Path(file).read_text(encoding="utf-8").splitlines(): + reqs_text = reqs_line.strip() + if not reqs_text or reqs_text.startswith(("#", "-")): + continue - # Strip any comments - reqs_text, _, _ = reqs_text.partition("#") + # Strip any comments + reqs_text, _, _ = reqs_text.partition("#") - reqs.append(get_new_requirement_line(reqs_text, extra)) + reqs.append(get_new_requirement_line(reqs_text, extra)) - metadata = metadata.replace(meta_line, "\n".join(reqs)) + if reqs: + processed_metadata_lines.append("\n".join(reqs)) + metadata = "\n".join(processed_metadata_lines) + maker.add_metadata( metadata=metadata, name=name, From c0a59095f71504556d6c9f9b14556bf823e12ca0 Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:09:31 -0400 Subject: [PATCH 02/12] syntax --- tools/wheelmaker.py | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index 96371c5562..a4c34fcd2d 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -574,12 +574,12 @@ def get_new_requirement_line(reqs_text, extra): if not meta_line.startswith("Requires-Dist: "): processed_metadata_lines.append(meta_line) - else if not meta_line[len("Requires-Dist: ") :].startswith("@"): + elif 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. - processed_metadata_lines(meta_line) + processed_metadata_lines.append(meta_line) else: extra = extra.strip() processed_metadata_lines.append(get_new_requirement_line(package, extra)) @@ -604,7 +604,7 @@ def get_new_requirement_line(reqs_text, extra): processed_metadata_lines.append("\n".join(reqs)) metadata = "\n".join(processed_metadata_lines) - + maker.add_metadata( metadata=metadata, name=name, From ba7802272c148859e0559bde803ebb9a24207214 Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:16:35 -0400 Subject: [PATCH 03/12] syntax --- tools/wheelmaker.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index a4c34fcd2d..dc3f589ed8 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -600,8 +600,7 @@ def get_new_requirement_line(reqs_text, extra): reqs.append(get_new_requirement_line(reqs_text, extra)) - if reqs: - processed_metadata_lines.append("\n".join(reqs)) + processed_metadata_lines.extend(reqs) metadata = "\n".join(processed_metadata_lines) From e1f5e48647bdeb97071d17c62fd4be5da284c41d Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:27:50 -0400 Subject: [PATCH 04/12] alternative approach --- tools/wheelmaker.py | 48 +++++++++++++++++++++++---------------------- 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index dc3f589ed8..e8296ab23d 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -568,41 +568,43 @@ def get_new_requirement_line(reqs_text, extra): else: return f"Requires-Dist: {req.name}{req.specifier}; {extra}".strip(" ;") - processed_metadata_lines = [] - for meta_line in metadata.splitlines(): if not meta_line.startswith("Requires-Dist: "): - processed_metadata_lines.append(meta_line) + continue - elif not meta_line[len("Requires-Dist: ") :].startswith("@"): + 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. - processed_metadata_lines.append(meta_line) - else: - extra = extra.strip() - processed_metadata_lines.append(get_new_requirement_line(package, extra)) - - # This is a requirement that refers to a file. - else: - file, _, extra = meta_line[len("Requires-Dist: @") :].partition(";") + continue extra = extra.strip() + metadata = metadata.replace( + meta_line, get_new_requirement_line(package, extra) + ) + continue - reqs = [] - for reqs_line in Path(file).read_text(encoding="utf-8").splitlines(): - reqs_text = reqs_line.strip() - if not reqs_text or reqs_text.startswith(("#", "-")): - continue + # This is a requirement that refers to a file. + file, _, extra = meta_line[len("Requires-Dist: @") :].partition(";") + extra = extra.strip() - # Strip any comments - reqs_text, _, _ = reqs_text.partition("#") + reqs = [] + for reqs_line in Path(file).read_text(encoding="utf-8").splitlines(): + reqs_text = reqs_line.strip() + if not reqs_text or reqs_text.startswith(("#", "-")): + continue - reqs.append(get_new_requirement_line(reqs_text, extra)) + # Strip any comments + reqs_text, _, _ = reqs_text.partition("#") - processed_metadata_lines.extend(reqs) + reqs.append(get_new_requirement_line(reqs_text, extra)) - metadata = "\n".join(processed_metadata_lines) + if reqs: + metadata = metadata.replace(meta_line, "\n".join(reqs)) + # File is empty + # So replace the meta_line entirely, including removing newline chars + else: + metadata = re.sub(re.escape(meta_line) + r"(?:\r?\n)?", "", metadata, count=1) maker.add_metadata( metadata=metadata, @@ -631,4 +633,4 @@ def get_new_requirement_line(reqs_text, extra): if __name__ == "__main__": - main() + main() \ No newline at end of file From 0b0d9694d9e1782b73df2b7442087e5949532674 Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:28:07 -0400 Subject: [PATCH 05/12] newl --- tools/wheelmaker.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index e8296ab23d..908b3fe956 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -633,4 +633,4 @@ def get_new_requirement_line(reqs_text, extra): if __name__ == "__main__": - main() \ No newline at end of file + main() From d9432c3b5e4ffb4ed7f1a1d2aa778ffea718371b Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:47:01 -0400 Subject: [PATCH 06/12] unit test --- examples/wheel/BUILD.bazel | 16 ++++++++++++++++ examples/wheel/wheel_test.py | 26 ++++++++++++++++++++++++++ 2 files changed, 42 insertions(+) diff --git a/examples/wheel/BUILD.bazel b/examples/wheel/BUILD.bazel index d9ba800125..01c81ef50e 100644 --- a/examples/wheel/BUILD.bazel +++ b/examples/wheel/BUILD.bazel @@ -294,6 +294,12 @@ starlark # Example comment """.splitlines(), ) +write_file( + name = "empty_requires_file", + out = "empty_requires.txt", + content = "", +) + write_file( name = "extra_requires_file", out = "extra_requires.txt", @@ -324,6 +330,16 @@ py_wheel( deps = [":example_pkg"], ) +py_wheel( + name = "empty_requires_files", + distribution = "empty_requires_files", + python_tag = "py3", + requires_file = ":empty_requires.txt", + version = "0.0.1", + deps = [":example_pkg"], +) + + # Package just a specific py_libraries, without their dependencies py_wheel( name = "minimal_data_files", diff --git a/examples/wheel/wheel_test.py b/examples/wheel/wheel_test.py index a3d6034930..1e44510512 100644 --- a/examples/wheel/wheel_test.py +++ b/examples/wheel/wheel_test.py @@ -495,6 +495,32 @@ def test_requires_file_and_extra_requires_files(self): requires, ) + def test_empty_requires_file(self): + filename = self._get_path("empty_requires_files-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) + + with zipfile.ZipFile(wheel_file, "r") as zf: + metadata = zf.read(metadata_file).decode("utf-8") + + metadata_lines = metadata.splitlines() + + requires = [] + for i, line in enumerate(metadata_lines): + if line.startswith("Name:"): + self.assertTrue(lines[i + 1].startswith("Version:")) + if line.startswith(b"Requires-Dist:"): + requires.append(line.decode("utf-8").strip()) + + print(requires) + self.assertEqual([], requires) + def test_minimal_data_files(self): filename = self._get_path("minimal_data_files-0.0.1-py3-none-any.whl") From caa5d7e69519d5e8ed4d2521fd44e4c38dc56ece Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:48:54 -0400 Subject: [PATCH 07/12] test --- examples/wheel/BUILD.bazel | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/examples/wheel/BUILD.bazel b/examples/wheel/BUILD.bazel index 01c81ef50e..55dd17b20f 100644 --- a/examples/wheel/BUILD.bazel +++ b/examples/wheel/BUILD.bazel @@ -297,7 +297,7 @@ starlark # Example comment write_file( name = "empty_requires_file", out = "empty_requires.txt", - content = "", + content = [""], ) write_file( From d64f8e48f8d043abf82829a8b3637b43b3f0eb6b Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:50:38 -0400 Subject: [PATCH 08/12] fmt --- examples/wheel/BUILD.bazel | 1 - 1 file changed, 1 deletion(-) diff --git a/examples/wheel/BUILD.bazel b/examples/wheel/BUILD.bazel index 55dd17b20f..d59c326f34 100644 --- a/examples/wheel/BUILD.bazel +++ b/examples/wheel/BUILD.bazel @@ -339,7 +339,6 @@ py_wheel( deps = [":example_pkg"], ) - # Package just a specific py_libraries, without their dependencies py_wheel( name = "minimal_data_files", From 7624dcc60756b9eebd4407494349b1966f1d19f9 Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 10:51:54 -0400 Subject: [PATCH 09/12] add to test --- examples/wheel/BUILD.bazel | 1 + 1 file changed, 1 insertion(+) diff --git a/examples/wheel/BUILD.bazel b/examples/wheel/BUILD.bazel index d59c326f34..b434e67405 100644 --- a/examples/wheel/BUILD.bazel +++ b/examples/wheel/BUILD.bazel @@ -382,6 +382,7 @@ py_test( ":custom_package_root_multi_prefix", ":custom_package_root_multi_prefix_reverse_order", ":customized", + ":empty_requires_files", ":extra_requires", ":filename_escaping", ":minimal_data_files", From 065e9d7fd7514e089a0209349fb046d4851b1dc3 Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Sun, 13 Apr 2025 11:00:01 -0400 Subject: [PATCH 10/12] fix test --- examples/wheel/wheel_test.py | 10 ++++------ 1 file changed, 4 insertions(+), 6 deletions(-) diff --git a/examples/wheel/wheel_test.py b/examples/wheel/wheel_test.py index 1e44510512..f8cb8bf476 100644 --- a/examples/wheel/wheel_test.py +++ b/examples/wheel/wheel_test.py @@ -506,17 +506,15 @@ def test_empty_requires_file(self): metadata_file = f self.assertIsNotNone(metadata_file) - with zipfile.ZipFile(wheel_file, "r") as zf: - metadata = zf.read(metadata_file).decode("utf-8") - + metadata = zf.read(metadata_file).decode("utf-8") metadata_lines = metadata.splitlines() requires = [] for i, line in enumerate(metadata_lines): if line.startswith("Name:"): - self.assertTrue(lines[i + 1].startswith("Version:")) - if line.startswith(b"Requires-Dist:"): - requires.append(line.decode("utf-8").strip()) + self.assertTrue(metadata_lines[i + 1].startswith("Version:")) + if line.startswith("Requires-Dist:"): + requires.append(line.strip()) print(requires) self.assertEqual([], requires) From 005bedea4c1f34ade143eb6a7eff8d60f62f09b4 Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Mon, 14 Apr 2025 08:50:07 -0400 Subject: [PATCH 11/12] review feedback --- CHANGELOG.md | 1 + examples/wheel/wheel_test.py | 2 -- python/packaging.bzl | 5 +++++ 3 files changed, 6 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7d9b648bea..8df9d4ec41 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -94,6 +94,7 @@ Unreleased changes template. * (toolchains) Run the check on the Python interpreter in isolated mode, to ensure it's not affected by userland environment variables, such as `PYTHONPATH`. * (toolchains) Ensure temporary `.pyc` and `.pyo` files are also excluded from the interpreters repository files. * (pypi) Run interpreter version call in isolated mode, to ensure it's not affected by userland environment variables, such as `PYTHONPATH`. +* (packaging) An empty `requires_file` is treated as if it were omitted, resulting in a valid `METADATA` file. {#v0-0-0-added} ### Added diff --git a/examples/wheel/wheel_test.py b/examples/wheel/wheel_test.py index f8cb8bf476..9ec150301d 100644 --- a/examples/wheel/wheel_test.py +++ b/examples/wheel/wheel_test.py @@ -483,7 +483,6 @@ def test_requires_file_and_extra_requires_files(self): if line.startswith(b"Requires-Dist:"): requires.append(line.decode("utf-8").strip()) - print(requires) self.assertEqual( [ "Requires-Dist: tomli>=2.0.0", @@ -516,7 +515,6 @@ def test_empty_requires_file(self): if line.startswith("Requires-Dist:"): requires.append(line.strip()) - print(requires) self.assertEqual([], requires) def test_minimal_data_files(self): diff --git a/python/packaging.bzl b/python/packaging.bzl index 629af2d6a4..95f60b6d74 100644 --- a/python/packaging.bzl +++ b/python/packaging.bzl @@ -171,6 +171,11 @@ def py_wheel( These are subject to make var expansion, as with the `args` attribute. Note that you can also pass additional args to the bazel run command as in the example above. **kwargs: other named parameters passed to the underlying [py_wheel rule](#py_wheel_rule) + + :::{versionchanged} VERSION_NEXT_FEATURE + From now on, an empty `requires_file` is treated as if it were omitted, resulting in a valid + `METADATA` file. + ::: """ tags = kwargs.pop("tags", []) manual_tags = depset(tags + ["manual"]).to_list() From a094253184f7ee16cecfb933ed3c6a4578e4c49f Mon Sep 17 00:00:00 2001 From: Frank Portman Date: Mon, 14 Apr 2025 08:57:37 -0400 Subject: [PATCH 12/12] stardoc --- python/packaging.bzl | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/python/packaging.bzl b/python/packaging.bzl index 95f60b6d74..b190635cfe 100644 --- a/python/packaging.bzl +++ b/python/packaging.bzl @@ -101,6 +101,11 @@ def py_wheel( Currently only pure-python wheels are supported. + :::{versionchanged} VERSION_NEXT_FEATURE + From now on, an empty `requires_file` is treated as if it were omitted, resulting in a valid + `METADATA` file. + ::: + Examples: ```python @@ -171,11 +176,6 @@ def py_wheel( These are subject to make var expansion, as with the `args` attribute. Note that you can also pass additional args to the bazel run command as in the example above. **kwargs: other named parameters passed to the underlying [py_wheel rule](#py_wheel_rule) - - :::{versionchanged} VERSION_NEXT_FEATURE - From now on, an empty `requires_file` is treated as if it were omitted, resulting in a valid - `METADATA` file. - ::: """ tags = kwargs.pop("tags", []) manual_tags = depset(tags + ["manual"]).to_list()