From 6864decb77fc1a59272673db25b7e4642b06600d Mon Sep 17 00:00:00 2001 From: Douglas Thor Date: Tue, 24 Jun 2025 20:12:53 -0700 Subject: [PATCH 1/4] Update py_wheel docs --- python/private/py_wheel.bzl | 10 +++++++++- 1 file changed, 9 insertions(+), 1 deletion(-) diff --git a/python/private/py_wheel.bzl b/python/private/py_wheel.bzl index cfd4efdcda..e6352efcea 100644 --- a/python/private/py_wheel.bzl +++ b/python/private/py_wheel.bzl @@ -217,7 +217,15 @@ _other_attrs = { ), "strip_path_prefixes": attr.string_list( default = [], - doc = "path prefixes to strip from files added to the generated package", + doc = """\ +Path prefixes to strip from files added to the generated package. +Prefixes are checked **in order** and only the **first match** will be used. + +For example: ++ `["foo", "foo/bar/baz"]` will strip `"foo/bar/baz/file.py"` to `"bar/baz/file.py"` ++ `["foo/bar/baz", "foo"]` will strip `"foo/bar/baz/file.py"` to `"file.py"` and + `"foo/file2.py"` to `"file2.py"` +""", ), "summary": attr.string( doc = "A one-line summary of what the distribution does", From 30343d481ecdeb893c740064937f432e64170daa Mon Sep 17 00:00:00 2001 From: Douglas Thor Date: Tue, 24 Jun 2025 21:49:32 -0700 Subject: [PATCH 2/4] Light refactor; add test --- tests/tools/BUILD.bazel | 23 +++++++++++++++++++++++ tests/tools/wheelmaker_test.py | 27 +++++++++++++++++++++++++++ tools/wheelmaker.py | 25 +++++++++++++------------ 3 files changed, 63 insertions(+), 12 deletions(-) create mode 100644 tests/tools/BUILD.bazel create mode 100644 tests/tools/wheelmaker_test.py diff --git a/tests/tools/BUILD.bazel b/tests/tools/BUILD.bazel new file mode 100644 index 0000000000..4d163f19f1 --- /dev/null +++ b/tests/tools/BUILD.bazel @@ -0,0 +1,23 @@ +# Copyright 2025 The Bazel Authors. All rights reserved. +# +# Licensed under the Apache License, Version 2.0 (the "License"); +# you may not use this file except in compliance with the License. +# You may obtain a copy of the License at +# +# http://www.apache.org/licenses/LICENSE-2.0 +# +# Unless required by applicable law or agreed to in writing, software +# distributed under the License is distributed on an "AS IS" BASIS, +# WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. +# See the License for the specific language governing permissions and +# limitations under the License. +load("//python:py_test.bzl", "py_test") + +licenses(["notice"]) + +py_test( + name = "wheelmaker_test", + size = "small", + srcs = ["wheelmaker_test.py"], + deps = ["//tools:wheelmaker"], +) diff --git a/tests/tools/wheelmaker_test.py b/tests/tools/wheelmaker_test.py new file mode 100644 index 0000000000..99a2df6e54 --- /dev/null +++ b/tests/tools/wheelmaker_test.py @@ -0,0 +1,27 @@ +import unittest + +import tools.wheelmaker as wheelmaker + + +class ArcNameFromTest(unittest.TestCase): + + def test_arcname_from(self) -> None: + + # (name, distribution_prefix, strip_path_prefixes, want) tuples + checks = [ + ("foo/bar/baz/file.py", "", [], "foo/bar/baz/file.py"), + ("foo/bar/baz/file.py", "", ["foo"], "/bar/baz/file.py"), + ("foo/bar/baz/file.py", "", ["foo/"], "bar/baz/file.py"), + ("foo/bar/baz/file.py", "", ["foo/bar"], "/baz/file.py"), + ("foo/bar/baz/file.py", "", ["foo/bar", "baz"], "/baz/file.py"), + ("foo/bar/baz/file.py", "", ["foo", "bar"], "/bar/baz/file.py"), + ("foo/bar/baz/file.py", "", ["baz", "foo/bar"], "/baz/file.py"), + ] + for name, prefix, strip, want in checks: + with self.subTest(name=name, distribution_prefix=prefix, strip_path_prefixes=strip, want=want): + got = wheelmaker.arcname_from(name=name, distribution_prefix=prefix, strip_path_prefixes=strip) + self.assertEqual(got, want) + + +if __name__ == "__main__": + unittest.main() diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index 8b775e1541..c3015e9bec 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -98,6 +98,19 @@ def normalize_pep440(version): return str(packaging.version.Version(f"0+{sanitized}")) +def arcname_from(name: str, distribution_prefix: str, strip_path_prefixes:list[str] | None = None): + # Always use unix path separators. + normalized_arcname = name.replace(os.path.sep, "/") + # Don't manipulate names filenames in the .distinfo or .data directories. + if distribution_prefix and normalized_arcname.startswith(distribution_prefix): + return normalized_arcname + for prefix in strip_path_prefixes: + if normalized_arcname.startswith(prefix): + return normalized_arcname[len(prefix) :] + + return normalized_arcname + + class _WhlFile(zipfile.ZipFile): def __init__( self, @@ -126,18 +139,6 @@ def data_path(self, basename): def add_file(self, package_filename, real_filename): """Add given file to the distribution.""" - def arcname_from(name): - # Always use unix path separators. - normalized_arcname = name.replace(os.path.sep, "/") - # Don't manipulate names filenames in the .distinfo or .data directories. - if normalized_arcname.startswith(self._distribution_prefix): - return normalized_arcname - for prefix in self._strip_path_prefixes: - if normalized_arcname.startswith(prefix): - return normalized_arcname[len(prefix) :] - - return normalized_arcname - if os.path.isdir(real_filename): directory_contents = os.listdir(real_filename) for file_ in directory_contents: From cb955055c35604e7a0868eb03fc96fdfd28f809a Mon Sep 17 00:00:00 2001 From: Douglas Thor Date: Tue, 24 Jun 2025 22:00:56 -0700 Subject: [PATCH 3/4] lol forgot to update the call. And run black --- tests/tools/wheelmaker_test.py | 11 +++++++++-- tools/wheelmaker.py | 14 +++++++++++--- 2 files changed, 20 insertions(+), 5 deletions(-) diff --git a/tests/tools/wheelmaker_test.py b/tests/tools/wheelmaker_test.py index 99a2df6e54..5bc30a0074 100644 --- a/tests/tools/wheelmaker_test.py +++ b/tests/tools/wheelmaker_test.py @@ -18,8 +18,15 @@ def test_arcname_from(self) -> None: ("foo/bar/baz/file.py", "", ["baz", "foo/bar"], "/baz/file.py"), ] for name, prefix, strip, want in checks: - with self.subTest(name=name, distribution_prefix=prefix, strip_path_prefixes=strip, want=want): - got = wheelmaker.arcname_from(name=name, distribution_prefix=prefix, strip_path_prefixes=strip) + with self.subTest( + name=name, + distribution_prefix=prefix, + strip_path_prefixes=strip, + want=want, + ): + got = wheelmaker.arcname_from( + name=name, distribution_prefix=prefix, strip_path_prefixes=strip + ) self.assertEqual(got, want) diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index c3015e9bec..29afbabac1 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -98,7 +98,9 @@ def normalize_pep440(version): return str(packaging.version.Version(f"0+{sanitized}")) -def arcname_from(name: str, distribution_prefix: str, strip_path_prefixes:list[str] | None = None): +def arcname_from( + name: str, distribution_prefix: str, strip_path_prefixes: list[str] | None = None +): # Always use unix path separators. normalized_arcname = name.replace(os.path.sep, "/") # Don't manipulate names filenames in the .distinfo or .data directories. @@ -148,7 +150,11 @@ def add_file(self, package_filename, real_filename): ) return - arcname = arcname_from(package_filename) + arcname = arcname_from( + package_filename, + distribution_prefix=self._distribution_prefix, + strip_path_prefixes=self._strip_path_prefixes, + ) zinfo = self._zipinfo(arcname) # Write file to the zip archive while computing the hash and length @@ -570,7 +576,9 @@ def get_new_requirement_line(reqs_text, extra): else: return f"Requires-Dist: {req.name}{req_extra_deps}{req.specifier}; {req.marker}" else: - return f"Requires-Dist: {req.name}{req_extra_deps}{req.specifier}; {extra}".strip(" ;") + return f"Requires-Dist: {req.name}{req_extra_deps}{req.specifier}; {extra}".strip( + " ;" + ) for meta_line in metadata.splitlines(): if not meta_line.startswith("Requires-Dist: "): From 89c81f85ed933e194338a75048dad1bffdbba4fc Mon Sep 17 00:00:00 2001 From: Douglas Thor Date: Wed, 25 Jun 2025 20:58:12 -0700 Subject: [PATCH 4/4] Review comments: + test cases + docstring + formatting + types --- tests/tools/wheelmaker_test.py | 22 +++++++++++++--------- tools/wheelmaker.py | 14 ++++++++++++-- 2 files changed, 25 insertions(+), 11 deletions(-) diff --git a/tests/tools/wheelmaker_test.py b/tests/tools/wheelmaker_test.py index 5bc30a0074..0efe1c9fbc 100644 --- a/tests/tools/wheelmaker_test.py +++ b/tests/tools/wheelmaker_test.py @@ -4,18 +4,22 @@ class ArcNameFromTest(unittest.TestCase): - def test_arcname_from(self) -> None: - # (name, distribution_prefix, strip_path_prefixes, want) tuples checks = [ - ("foo/bar/baz/file.py", "", [], "foo/bar/baz/file.py"), - ("foo/bar/baz/file.py", "", ["foo"], "/bar/baz/file.py"), - ("foo/bar/baz/file.py", "", ["foo/"], "bar/baz/file.py"), - ("foo/bar/baz/file.py", "", ["foo/bar"], "/baz/file.py"), - ("foo/bar/baz/file.py", "", ["foo/bar", "baz"], "/baz/file.py"), - ("foo/bar/baz/file.py", "", ["foo", "bar"], "/bar/baz/file.py"), - ("foo/bar/baz/file.py", "", ["baz", "foo/bar"], "/baz/file.py"), + ("a/b/c/file.py", "", [], "a/b/c/file.py"), + ("a/b/c/file.py", "", ["a"], "/b/c/file.py"), + ("a/b/c/file.py", "", ["a/b/"], "c/file.py"), + # only first found is used and it's not cumulative. + ("a/b/c/file.py", "", ["a/", "b/"], "b/c/file.py"), + # Examples from docs + ("foo/bar/baz/file.py", "", ["foo", "foo/bar/baz"], "/bar/baz/file.py"), + ("foo/bar/baz/file.py", "", ["foo/bar/baz", "foo"], "/file.py"), + ("foo/file2.py", "", ["foo/bar/baz", "foo"], "/file2.py"), + # Files under the distribution prefix (eg mylib-1.0.0-dist-info) + # are unmodified + ("mylib-0.0.1-dist-info/WHEEL", "mylib", [], "mylib-0.0.1-dist-info/WHEEL"), + ("mylib/a/b/c/WHEEL", "mylib", ["mylib"], "mylib/a/b/c/WHEEL"), ] for name, prefix, strip, want in checks: with self.subTest( diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index 29afbabac1..3401c749ed 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -24,6 +24,7 @@ import stat import sys import zipfile +from collections.abc import Iterable from pathlib import Path _ZIP_EPOCH = (1980, 1, 1, 0, 0, 0) @@ -99,8 +100,17 @@ def normalize_pep440(version): def arcname_from( - name: str, distribution_prefix: str, strip_path_prefixes: list[str] | None = None -): + name: str, distribution_prefix: str, strip_path_prefixes: Sequence[str] = () +) -> str: + """Return the within-archive name for a given file path name. + + Prefixes to strip are checked in order and only the first match will be used. + + Args: + name: The file path eg 'mylib/a/b/c/file.py' + distribution_prefix: The + strip_path_prefixes: Remove these prefixes from names. + """ # Always use unix path separators. normalized_arcname = name.replace(os.path.sep, "/") # Don't manipulate names filenames in the .distinfo or .data directories.