feat(wheel): Add support for add_path_prefix #3679
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -34,39 +34,69 @@ def test_quote_all_false_leaves_simple_filenames_unquoted(self) -> None: | |
| def test_quote_all_quotes_filenames_with_commas(self) -> None: | ||
| """Filenames with commas are always quoted, regardless of quote_all_filenames.""" | ||
| whl = self._make_whl_file(quote_all=True) | ||
| self.assertEqual( | ||
| whl._quote_filename("foo,bar/baz.py"), '"foo,bar/baz.py"' | ||
| ) | ||
|
|
||
| whl = self._make_whl_file(quote_all=False) | ||
| self.assertEqual( | ||
| whl._quote_filename("foo,bar/baz.py"), '"foo,bar/baz.py"' | ||
| ) | ||
|
|
||
|
|
||
| class ArcNameFromTest(unittest.TestCase): | ||
| def test_arcname_from(self) -> None: | ||
| # (name, distribution_prefix, strip_path_prefixes, add_path_prefix, want) tuples | ||
| checks = [ | ||
| ("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"), | ||
| # Check that prefixes are added | ||
| ("a/b/c/file.py", "", [], "namespace/", "namespace/a/b/c/file.py"), | ||
| ("a/b/c/file.py", "", ["a"], "namespace", "namespace/b/c/file.py"), | ||
|
Collaborator
There was a problem hiding this comment. I know this may be a big ask, but it would be nice to change this to a list of dicts or namedtuples so that it is easier to understand the parameters in the test. |
||
| ( | ||
| "a/b/c/file.py", | ||
| "", | ||
| ["a/b/"], | ||
| "namespace_", | ||
|
Collaborator
There was a problem hiding this comment. I wonder if supporting this is actually error prone. Maybe it would be best to tell users to use: This way we can keep the logic simpler. |
||
| "namespace_c/file.py", | ||
| ), | ||
| ] | ||
| for name, prefix, strip, add, want in checks: | ||
| with self.subTest( | ||
| name=name, | ||
| distribution_prefix=prefix, | ||
| strip_path_prefixes=strip, | ||
| add_path_prefix=add, | ||
| want=want, | ||
| ): | ||
| got = wheelmaker.arcname_from( | ||
| name=name, | ||
| distribution_prefix=prefix, | ||
| strip_path_prefixes=strip, | ||
| add_path_prefix=add, | ||
| ) | ||
| self.assertEqual(got, want) | ||
|
|
||
|
|
@@ -77,7 +107,9 @@ def test_requirement(self): | |
| self.assertEqual(result, "Requires-Dist: requests>=2.0") | ||
|
|
||
| def test_requirement_and_extra(self): | ||
| result = wheelmaker.get_new_requirement_line( | ||
| "requests>=2.0", "extra=='dev'" | ||
| ) | ||
| self.assertEqual(result, "Requires-Dist: requests>=2.0; extra=='dev'") | ||
|
|
||
| def test_requirement_with_url(self): | ||
|
|
||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -94,13 +94,18 @@ def normalize_pep440(version): | |
| substituted = re.sub(r"\{\w+\}", "0", version) | ||
| delimiter = "." if "+" in substituted else "+" | ||
| try: | ||
| return str( | ||
| packaging.version.Version(f"{substituted}{delimiter}{sanitized}") | ||
| ) | ||
| except packaging.version.InvalidVersion: | ||
| return str(packaging.version.Version(f"0+{sanitized}")) | ||
|
|
||
|
|
||
| def arcname_from( | ||
| name: str, | ||
| distribution_prefix: str, | ||
| strip_path_prefixes: Sequence[str] = (), | ||
| add_path_prefix: str = "", | ||
| ) -> str: | ||
| """Return the within-archive name for a given file path name. | ||
|
|
||
|
|
@@ -110,17 +115,20 @@ def arcname_from( | |
| name: The file path eg 'mylib/a/b/c/file.py' | ||
| distribution_prefix: The | ||
| strip_path_prefixes: Remove these prefixes from names. | ||
| add_path_prefix: Add prefix after stripping the path 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. | ||
| if distribution_prefix and normalized_arcname.startswith( | ||
| distribution_prefix | ||
| ): | ||
| return normalized_arcname | ||
| for prefix in strip_path_prefixes: | ||
| if normalized_arcname.startswith(prefix): | ||
| return add_path_prefix + normalized_arcname[len(prefix) :] | ||
|
|
||
| return add_path_prefix + normalized_arcname | ||
|
Comment on lines
+129
to
+131
Contributor
There was a problem hiding this comment. The implementation uses raw string concatenation for As seen in the tests and examples, stripping a directory name without a trailing slash (e.g., Consider using a more robust path joining method or normalizing slashes to ensure a single
Author
There was a problem hiding this comment. This is intentional so that you can actually prepend without a slash, e.g. |
||
|
|
||
|
|
||
| class _WhlFile(zipfile.ZipFile): | ||
|
|
@@ -131,13 +139,15 @@ def __init__( | |
| mode, | ||
| distribution_prefix: str, | ||
| strip_path_prefixes=None, | ||
| add_path_prefix=None, | ||
| compression=zipfile.ZIP_DEFLATED, | ||
| quote_all_filenames: bool = False, | ||
| **kwargs, | ||
| ): | ||
| self._distribution_prefix = distribution_prefix | ||
|
|
||
| self._strip_path_prefixes = strip_path_prefixes or [] | ||
| self._add_path_prefix = add_path_prefix or "" | ||
| # Entries for the RECORD file as (filename, digest, size) tuples. | ||
| self._record: list[tuple[str, str, str]] = [] | ||
| # Whether to quote filenames in the RECORD file (for compatibility with | ||
|
|
@@ -168,6 +178,7 @@ def add_file(self, package_filename, real_filename): | |
| package_filename, | ||
| distribution_prefix=self._distribution_prefix, | ||
| strip_path_prefixes=self._strip_path_prefixes, | ||
| add_path_prefix=self._add_path_prefix, | ||
| ) | ||
| zinfo = self._zipinfo(arcname) | ||
|
|
||
|
|
@@ -194,7 +205,9 @@ def add_string(self, filename, contents): | |
| self.writestr(zinfo, contents) | ||
| hash = hashlib.sha256() | ||
| hash.update(contents) | ||
| self._add_to_record( | ||
| filename, self._serialize_digest(hash), len(contents) | ||
| ) | ||
|
|
||
| def _serialize_digest(self, hash) -> str: | ||
| # https://www.python.org/dev/peps/pep-0376/#record | ||
|
|
@@ -231,7 +244,9 @@ def _quote_filename(self, filename: str) -> str: | |
| filename = filename.lstrip("/") | ||
| # Some RECORDs like torch have *all* filenames quoted and we must minimize diff. | ||
| # Otherwise, we quote only when necessary (e.g. for filenames with commas). | ||
| quoting = ( | ||
| csv.QUOTE_ALL if self.quote_all_filenames else csv.QUOTE_MINIMAL | ||
| ) | ||
| with io.StringIO() as buf: | ||
| csv.writer(buf, quoting=quoting).writerow([filename]) | ||
| return buf.getvalue().strip() | ||
|
|
@@ -261,6 +276,7 @@ def __init__( | |
| compress, | ||
| outfile=None, | ||
| strip_path_prefixes=None, | ||
| add_path_prefix=None, | ||
| ): | ||
| self._name = name | ||
| self._version = normalize_pep440(version) | ||
|
|
@@ -270,9 +286,10 @@ def __init__( | |
| self._platform = platform | ||
| self._outfile = outfile | ||
| self._strip_path_prefixes = strip_path_prefixes | ||
| self._add_path_prefix = add_path_prefix | ||
| self._compress = compress | ||
| self._wheelname_fragment_distribution_name = ( | ||
| escape_filename_distribution_name(self._name) | ||
| ) | ||
|
|
||
| self._distribution_prefix = ( | ||
|
|
@@ -287,7 +304,10 @@ def __enter__(self): | |
| mode="w", | ||
| distribution_prefix=self._distribution_prefix, | ||
| strip_path_prefixes=self._strip_path_prefixes, | ||
| add_path_prefix=self._add_path_prefix, | ||
| compression=( | ||
| zipfile.ZIP_DEFLATED if self._compress else zipfile.ZIP_STORED | ||
| ), | ||
| ) | ||
| return self | ||
|
|
||
|
|
@@ -330,7 +350,9 @@ def add_wheelfile(self): | |
| Wheel-Version: 1.0 | ||
| Generator: bazel-wheelmaker 1.0 | ||
| Root-Is-Purelib: {} | ||
| """.format( | ||
| "true" if self._platform == "any" else "false" | ||
| ) | ||
| for tag in self.disttags(): | ||
| wheel_contents += "Tag: %s\n" % tag | ||
| self._whlfile.add_string(self.distinfo_path("WHEEL"), wheel_contents) | ||
|
|
@@ -339,7 +361,9 @@ def add_metadata(self, metadata, name, description): | |
| """Write METADATA file to the distribution.""" | ||
| # https://www.python.org/dev/peps/pep-0566/ | ||
| # https://packaging.python.org/specifications/core-metadata/ | ||
| metadata = re.sub( | ||
| "^Name: .*$", "Name: %s" % name, metadata, flags=re.MULTILINE | ||
| ) | ||
| metadata += "Version: %s\n\n" % self._version | ||
| # setuptools seems to insert UNKNOWN as description when none is | ||
| # provided. | ||
|
|
@@ -418,7 +442,9 @@ def resolve_argument_stamp( | |
|
|
||
| def parse_args() -> argparse.Namespace: | ||
| parser = argparse.ArgumentParser(description="Builds a python wheel") | ||
| metadata_group = parser.add_argument_group( | ||
| "Wheel name, version and platform" | ||
| ) | ||
| metadata_group.add_argument( | ||
| "--name", required=True, type=str, help="Name of the distribution" | ||
| ) | ||
|
|
@@ -465,6 +491,13 @@ def parse_args() -> argparse.Namespace: | |
| help="Path prefix to be stripped from input package files' path. " | ||
| "Can be supplied multiple times. Evaluated in order.", | ||
| ) | ||
| output_group.add_argument( | ||
| "--path_prefix", | ||
| type=str, | ||
| default="", | ||
| help="Path prefix to be prepended to input package files' path. " | ||
| "It is prepended after stripping any specified path prefixes first.", | ||
| ) | ||
|
|
||
| wheel_group = parser.add_argument_group("Wheel metadata") | ||
| wheel_group.add_argument( | ||
|
|
@@ -477,7 +510,8 @@ def parse_args() -> argparse.Namespace: | |
| "--description_file", help="Path to the file with package description" | ||
| ) | ||
| wheel_group.add_argument( | ||
| "--description_content_type", | ||
| help="Content type of the package description", | ||
| ) | ||
| wheel_group.add_argument( | ||
| "--entry_points_file", | ||
|
|
@@ -579,6 +613,7 @@ def main() -> None: | |
| platform=arguments.platform, | ||
| outfile=arguments.out, | ||
| strip_path_prefixes=strip_prefixes, | ||
| add_path_prefix=arguments.path_prefix, | ||
| compress=not arguments.no_compress, | ||
| ) as maker: | ||
| for package_filename, real_filename in all_files: | ||
|
|
@@ -608,7 +643,9 @@ def main() -> None: | |
|
|
||
| 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 | ||
|
|
@@ -623,7 +660,9 @@ def main() -> None: | |
| 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 | ||
|
|
@@ -650,7 +689,8 @@ def main() -> None: | |
|
|
||
| if arguments.entry_points_file: | ||
| maker.add_file( | ||
| maker.distinfo_path("entry_points.txt"), | ||
| arguments.entry_points_file, | ||
| ) | ||
|
|
||
| # Sort the files for reproducible order in the archive. | ||
|
|
||
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The
add_path_prefixvalue here is set to"custom_prefix"(without a trailing slash), which differs from the examples in thepy_wheeldocumentation (e.g.,"foo/").This works in this specific case because
strip_path_prefixes = ["examples"]leaves a leading slash in the path (e.g.,/wheel/main.py), resulting incustom_prefix/wheel/main.py. However, if no stripping occurred or if the prefix instrip_path_prefixesincluded a trailing slash, the result would be an invalid path likecustom_prefixwheel/main.py.To ensure robustness and clarity, it is recommended to make the examples consistent or explicitly handle the path separator in the implementation.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is intentional so that you can actually prepend without a slash, e.g.