From 50668412fb2a5ce6380e65ec1b25f3076a102eab Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Stradomski?= Date: Sat, 1 Oct 2022 18:59:52 +0200 Subject: [PATCH 1/4] Move construction of the METADATA file (except for appending description) from wheelmaker to .bzl to avoid passing possibly unicode data on the commandline, which causes issues on windows an RBE due to UTF-16 vs UTF-8 confusion. Also fix some wrong attribute descriptions. --- python/packaging.bzl | 58 ++++++++++++++---------- tools/wheelmaker.py | 103 +++++++++---------------------------------- 2 files changed, 56 insertions(+), 105 deletions(-) diff --git a/python/packaging.bzl b/python/packaging.bzl index 22d1f5bc4d..c9e5ebc4b6 100644 --- a/python/packaging.bzl +++ b/python/packaging.bzl @@ -153,7 +153,6 @@ def _py_wheel_impl(ctx): args.add("--name", ctx.attr.distribution) args.add("--version", version) args.add("--python_tag", ctx.attr.python_tag) - args.add("--python_requires", ctx.attr.python_requires) args.add("--abi", ctx.attr.abi) args.add("--platform", ctx.attr.platform) args.add("--out", outfile) @@ -168,28 +167,42 @@ def _py_wheel_impl(ctx): args.add("--input_file_list", packageinputfile) - extra_headers = [] + # Note: Description file is not embedded into metadata.txt yet, + # it will be done later by wheelmaker script. + metadata_file = ctx.actions.declare_file(ctx.attr.name + ".metadata.txt") + metadata_contents = ["Metadata-Version: 2.1"] + metadata_contents.append("Name: %s" % ctx.attr.distribution) + metadata_contents.append("Version: %s" % version) + if ctx.attr.author: - extra_headers.append("Author: %s" % ctx.attr.author) + metadata_contents.append("Author: %s" % ctx.attr.author) if ctx.attr.author_email: - extra_headers.append("Author-email: %s" % ctx.attr.author_email) + metadata_contents.append("Author-email: %s" % ctx.attr.author_email) if ctx.attr.homepage: - extra_headers.append("Home-page: %s" % ctx.attr.homepage) + metadata_contents.append("Home-page: %s" % ctx.attr.homepage) if ctx.attr.license: - extra_headers.append("License: %s" % ctx.attr.license) - - for h in extra_headers: - args.add("--header", h) + metadata_contents.append("License: %s" % ctx.attr.license) for c in ctx.attr.classifiers: - args.add("--classifier", c) - - for r in ctx.attr.requires: - args.add("--requires", r) - - for option, requirements in ctx.attr.extra_requires.items(): - for r in requirements: - args.add("--extra_requires", r + ";" + option) + metadata_contents.append("Classifier: %s" % c) + + if ctx.attr.python_requires: + metadata_contents.append("Requires-Python: %s" % ctx.attr.python_requires) + for requirement in ctx.attr.requires: + metadata_contents.append("Requires-Dist: %s" % requirement) + + for option, option_requirements in sorted(ctx.attr.extra_requires.items()): + metadata_contents.append("Provides-Extra: %s" % option) + for requirement in option_requirements: + metadata_contents.append( + "Requires-Dist: %s; extra == '%s'" % (requirement, option) + ) + ctx.actions.write( + output = metadata_file, + content = "\n".join(metadata_contents) + "\n", + ) + other_inputs.append(metadata_file) + args.add("--metadata_file", metadata_file) # Merge console_scripts into entry_points. entrypoints = dict(ctx.attr.entry_points) # Copy so we can mutate it @@ -334,7 +347,9 @@ _requirement_attrs = { doc = "List of optional requirements for this package", ), "requires": attr.string_list( - doc = "List of requirements for this package", + doc = ("List of requirements for this package. See the section on " + + "[Declaring required dependency](https://setuptools.readthedocs.io/en/latest/userguide/dependency_management.html#declaring-dependencies) " + + "for details and examples of the format of this argument."), ), } @@ -366,7 +381,7 @@ _other_attrs = { doc = "A list of strings describing the categories for the package. For valid classifiers see https://pypi.org/classifiers", ), "description_file": attr.label( - doc = "A file containing text describing the package in a single line.", + doc = "A file containing text describing the package.", allow_single_file = True, ), "extra_distinfo_files": attr.label_keyed_string_dict( @@ -383,10 +398,7 @@ _other_attrs = { ), "python_requires": attr.string( doc = ( - "A string specifying what other distributions need to be installed " + - "when this one is. See the section on " + - "[Declaring required dependency](https://setuptools.readthedocs.io/en/latest/userguide/dependency_management.html#declaring-dependencies) " + - "for details and examples of the format of this argument." + "Python versions required by this distribution, e.g. '>=3.5,<3.7'" ), default = "", ), diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index 85b97bb44f..12fdae36b7 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -167,39 +167,11 @@ def add_wheelfile(self): wheel_contents += "Tag: %s\n" % tag self.add_string(self.distinfo_path("WHEEL"), wheel_contents) - def add_metadata( - self, - extra_headers, - description, - classifiers, - python_requires, - requires, - extra_requires, - ): + def add_metadata(self, metadata, description): """Write METADATA file to the distribution.""" # https://www.python.org/dev/peps/pep-0566/ # https://packaging.python.org/specifications/core-metadata/ - metadata = [] - metadata.append("Metadata-Version: 2.1") - metadata.append("Name: %s" % self._name) - metadata.append("Version: %s" % self._version) - metadata.extend(extra_headers) - for classifier in classifiers: - metadata.append("Classifier: %s" % classifier) - if python_requires: - metadata.append("Requires-Python: %s" % python_requires) - for requirement in requires: - metadata.append("Requires-Dist: %s" % requirement) - - extra_requires = sorted(extra_requires.items()) - for option, option_requires in extra_requires: - metadata.append("Provides-Extra: %s" % option) - for requirement in option_requires: - metadata.append( - "Requires-Dist: %s; extra == '%s'" % (requirement, option) - ) - - metadata = "\n".join(metadata) + "\n\n" + metadata += "\n" # setuptools seems to insert UNKNOWN as description when none is # provided. metadata += description if description else "UNKNOWN" @@ -303,25 +275,15 @@ def parse_args() -> argparse.Namespace: action="append", default=[], help="Path prefix to be stripped from input package files' path. " - "Can be supplied multiple times. " - "Evaluated in order.", + "Can be supplied multiple times. Evaluated in order.", ) wheel_group = parser.add_argument_group("Wheel metadata") wheel_group.add_argument( - "--header", - action="append", - help="Additional headers to be embedded in the package metadata. " - "Can be supplied multiple times.", - ) - wheel_group.add_argument( - "--classifier", - action="append", - help="Classifiers to embed in package metadata. " - "Can be supplied multiple times", - ) - wheel_group.add_argument( - "--python_requires", help="Version of python that the wheel will work with" + "--metadata_file", + type=Path, + help="Contents of the METADATA file (before appending contents of " + "--description_file)", ) wheel_group.add_argument( "--description_file", help="Path to the file with package description" @@ -352,21 +314,6 @@ def parse_args() -> argparse.Namespace: "dist-info directory. Can be supplied multiple times.", ) - requirements_group = parser.add_argument_group("Package requirements") - requirements_group.add_argument( - "--requires", - type=str, - action="append", - help="List of package requirements. Can be supplied multiple times.", - ) - requirements_group.add_argument( - "--extra_requires", - type=str, - action="append", - help="List of optional requirements in a 'requirement;option name'. " - "Can be supplied multiple times.", - ) - build_group = parser.add_argument_group("Building requirements") build_group.add_argument( "--volatile_status_file", @@ -435,32 +382,24 @@ def main() -> None: description = None if arguments.description_file: if sys.version_info[0] == 2: - with open(arguments.description_file, "rt") as description_file: + with open(arguments.description_file, + "rt") as description_file: description = description_file.read() else: - with open( - arguments.description_file, "rt", encoding="utf-8" - ) as description_file: + with open(arguments.description_file, "rt", + encoding="utf-8") as description_file: description = description_file.read() - extra_requires = collections.defaultdict(list) - if arguments.extra_requires: - for extra in arguments.extra_requires: - req, option = extra.rsplit(";", 1) - extra_requires[option].append(req) - classifiers = arguments.classifier or [] - python_requires = arguments.python_requires or "" - requires = arguments.requires or [] - extra_headers = arguments.header or [] - - maker.add_metadata( - extra_headers=extra_headers, - description=description, - classifiers=classifiers, - python_requires=python_requires, - requires=requires, - extra_requires=extra_requires, - ) + metadata = None + if sys.version_info[0] == 2: + with open(arguments.metadata_file, "rt") as metadata_file: + metadata = metadata_file.read() + else: + with open(arguments.metadata_file, "rt", + encoding="utf-8") as metadata_file: + metadata = metadata_file.read() + + maker.add_metadata(metadata=metadata, description=description) if arguments.entry_points_file: maker.add_file( From 8c4d9228f1bb972bdcf0316032f5b328fc17a88e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Stradomski?= Date: Sat, 1 Oct 2022 19:08:42 +0200 Subject: [PATCH 2/4] Regenerated docs. Buildifier fixes. --- docs/packaging.md | 6 +++--- python/packaging.bzl | 2 +- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/docs/packaging.md b/docs/packaging.md index 6d063edbff..d263574bff 100755 --- a/docs/packaging.md +++ b/docs/packaging.md @@ -91,7 +91,7 @@ py_wheel( | classifiers | A list of strings describing the categories for the package. For valid classifiers see https://pypi.org/classifiers | List of strings | optional | [] | | console_scripts | Deprecated console_script entry points, e.g. {'main': 'examples.wheel.main:main'}.

Deprecated: prefer the entry_points attribute, which supports console_scripts as well as other entry points. | Dictionary: String -> String | optional | {} | | deps | Targets to be included in the distribution.

The targets to package are usually py_library rules or filesets (for packaging data files).

Note it's usually better to package py_library targets and use entry_points attribute to specify console_scripts than to package py_binary rules. py_binary targets would wrap a executable script that tries to locate .runfiles directory which is not packaged in the wheel. | List of labels | optional | [] | -| description_file | A file containing text describing the package in a single line. | Label | optional | None | +| description_file | A file containing text describing the package. | Label | optional | None | | distribution | Name of the distribution.

This should match the project name onm PyPI. It's also the name that is used to refer to the package in other packages' dependencies. | String | required | | | entry_points | entry_points, e.g. {'console_scripts': ['main = examples.wheel.main:main']}. | Dictionary: String -> List of strings | optional | {} | | extra_distinfo_files | Extra files to add to distinfo directory in the archive. | Dictionary: Label -> String | optional | {} | @@ -99,9 +99,9 @@ py_wheel( | homepage | A string specifying the URL for the package homepage. | String | optional | "" | | license | A string specifying the license of the package. | String | optional | "" | | platform | Supported platform. Use 'any' for pure-Python wheel.

If you have included platform-specific data, such as a .pyd or .so extension module, you will need to specify the platform in standard pip format. If you support multiple platforms, you can define platform constraints, then use a select() to specify the appropriate specifier, eg:

platform = select({ "//platforms:windows_x86_64": "win_amd64", "//platforms:macos_x86_64": "macosx_10_7_x86_64", "//platforms:linux_x86_64": "manylinux2014_x86_64", }) | String | optional | "any" | -| python_requires | A string specifying what other distributions need to be installed when this one is. See the section on [Declaring required dependency](https://setuptools.readthedocs.io/en/latest/userguide/dependency_management.html#declaring-dependencies) for details and examples of the format of this argument. | String | optional | "" | +| python_requires | Python versions required by this distribution, e.g. '>=3.5,<3.7' | String | optional | "" | | python_tag | Supported Python version(s), eg py3, cp35.cp36, etc | String | optional | "py3" | -| requires | List of requirements for this package | List of strings | optional | [] | +| requires | List of requirements for this package. See the section on [Declaring required dependency] | stamp | Whether to encode build information into the wheel. Possible values:

- stamp = 1: Always stamp the build information into the wheel, even in [--nostamp](https://docs.bazel.build/versions/main/user-manual.html#flag--stamp) builds. This setting should be avoided, since it potentially kills remote caching for the target and any downstream actions that depend on it.

- stamp = 0: Always replace build information by constant values. This gives good build result caching.

- stamp = -1: Embedding of build information is controlled by the [--[no]stamp](https://docs.bazel.build/versions/main/user-manual.html#flag--stamp) flag.

Stamped targets are not rebuilt unless their dependencies change. | Integer | optional | -1 | | strip_path_prefixes | path prefixes to strip from files added to the generated package | List of strings | optional | [] | | version | Version number of the package. Note that this attribute supports stamp format strings (eg. 1.2.3-{BUILD_TIMESTAMP}) as well as 'make variables' (e.g. 1.2.3-$(VERSION)). | String | required | | diff --git a/python/packaging.bzl b/python/packaging.bzl index c9e5ebc4b6..6d7a901f53 100644 --- a/python/packaging.bzl +++ b/python/packaging.bzl @@ -195,7 +195,7 @@ def _py_wheel_impl(ctx): metadata_contents.append("Provides-Extra: %s" % option) for requirement in option_requirements: metadata_contents.append( - "Requires-Dist: %s; extra == '%s'" % (requirement, option) + "Requires-Dist: %s; extra == '%s'" % (requirement, option), ) ctx.actions.write( output = metadata_file, From 4d117d8f260749be599e082f6dfa338f3daae095 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Stradomski?= Date: Sat, 1 Oct 2022 19:16:05 +0200 Subject: [PATCH 3/4] Another missing docs update. --- docs/packaging.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/docs/packaging.md b/docs/packaging.md index d263574bff..af822b07eb 100755 --- a/docs/packaging.md +++ b/docs/packaging.md @@ -101,7 +101,7 @@ py_wheel( | platform | Supported platform. Use 'any' for pure-Python wheel.

If you have included platform-specific data, such as a .pyd or .so extension module, you will need to specify the platform in standard pip format. If you support multiple platforms, you can define platform constraints, then use a select() to specify the appropriate specifier, eg:

platform = select({ "//platforms:windows_x86_64": "win_amd64", "//platforms:macos_x86_64": "macosx_10_7_x86_64", "//platforms:linux_x86_64": "manylinux2014_x86_64", }) | String | optional | "any" | | python_requires | Python versions required by this distribution, e.g. '>=3.5,<3.7' | String | optional | "" | | python_tag | Supported Python version(s), eg py3, cp35.cp36, etc | String | optional | "py3" | -| requires | List of requirements for this package. See the section on [Declaring required dependency] +| requires | List of requirements for this package. See the section on [Declaring required dependency](https://setuptools.readthedocs.io/en/latest/userguide/dependency_management.html#declaring-dependencies) for details and examples of the format of this argument. | List of strings | optional | [] | | stamp | Whether to encode build information into the wheel. Possible values:

- stamp = 1: Always stamp the build information into the wheel, even in [--nostamp](https://docs.bazel.build/versions/main/user-manual.html#flag--stamp) builds. This setting should be avoided, since it potentially kills remote caching for the target and any downstream actions that depend on it.

- stamp = 0: Always replace build information by constant values. This gives good build result caching.

- stamp = -1: Embedding of build information is controlled by the [--[no]stamp](https://docs.bazel.build/versions/main/user-manual.html#flag--stamp) flag.

Stamped targets are not rebuilt unless their dependencies change. | Integer | optional | -1 | | strip_path_prefixes | path prefixes to strip from files added to the generated package | List of strings | optional | [] | | version | Version number of the package. Note that this attribute supports stamp format strings (eg. 1.2.3-{BUILD_TIMESTAMP}) as well as 'make variables' (e.g. 1.2.3-$(VERSION)). | String | required | | From bffa7de98d62bcac4d4f6f88bf2ed0410d2a6196 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Pawe=C5=82=20Stradomski?= Date: Sat, 1 Oct 2022 19:26:19 +0200 Subject: [PATCH 4/4] Fix tests: all platforms should produce identical wheels. --- examples/wheel/wheel_test.py | 99 ++++-------------------------------- 1 file changed, 10 insertions(+), 89 deletions(-) diff --git a/examples/wheel/wheel_test.py b/examples/wheel/wheel_test.py index 0e495eb236..cbca0927c3 100644 --- a/examples/wheel/wheel_test.py +++ b/examples/wheel/wheel_test.py @@ -94,40 +94,11 @@ def test_customized_wheel(self): entry_point_contents = zf.read( "example_customized-0.0.1.dist-info/entry_points.txt" ) - # The entries are guaranteed to be sorted. - if platform.system() == "Windows": - self.assertEqual( - record_contents, - b"""\ -example_customized-0.0.1.dist-info/METADATA,sha256=pzE96o3Sp63TDzxAZgl0F42EFevm8x15vpDLqDVp_EQ,378 -example_customized-0.0.1.dist-info/NOTICE,sha256=Xpdw-FXET1IRgZ_wTkx1YQfo1-alET0FVf6V1LXO4js,76 -example_customized-0.0.1.dist-info/README,sha256=WmOFwZ3Jga1bHG3JiGRsUheb4UbLffUxyTdHczS27-o,40 -example_customized-0.0.1.dist-info/RECORD,, -example_customized-0.0.1.dist-info/WHEEL,sha256=sobxWSyDDkdg_rinUth-jxhXHqoNqlmNMJY3aTZn2Us,91 -example_customized-0.0.1.dist-info/entry_points.txt,sha256=pqzpbQ8MMorrJ3Jp0ntmpZcuvfByyqzMXXi2UujuXD0,137 -examples/wheel/lib/data.txt,sha256=9vJKEdfLu8bZRArKLroPZJh1XKkK3qFMXiM79MBL2Sg,12 -examples/wheel/lib/module_with_data.py,sha256=8s0Khhcqz3yVsBKv2IB5u4l4TMKh7-c_V6p65WVHPms,637 -examples/wheel/lib/simple_module.py,sha256=z2hwciab_XPNIBNH8B1Q5fYgnJvQTeYf0ZQJpY8yLLY,637 -examples/wheel/main.py,sha256=sgg5iWN_9inYBjm6_Zw27hYdmo-l24fA-2rfphT-IlY,909 -""", - ) - else: - # TODO: The non-ascii characters in the METADATA file are interpreted differently on the - # ubuntu16_rbe hosts in comparison to other unix platforms. This should not be the case - # and the code should be updated to account for this. - rbe_expected_contents = b"""\ -example_customized-0.0.1.dist-info/METADATA,sha256=pzE96o3Sp63TDzxAZgl0F42EFevm8x15vpDLqDVp_EQ,378 -example_customized-0.0.1.dist-info/NOTICE,sha256=Xpdw-FXET1IRgZ_wTkx1YQfo1-alET0FVf6V1LXO4js,76 -example_customized-0.0.1.dist-info/README,sha256=WmOFwZ3Jga1bHG3JiGRsUheb4UbLffUxyTdHczS27-o,40 -example_customized-0.0.1.dist-info/RECORD,, -example_customized-0.0.1.dist-info/WHEEL,sha256=sobxWSyDDkdg_rinUth-jxhXHqoNqlmNMJY3aTZn2Us,91 -example_customized-0.0.1.dist-info/entry_points.txt,sha256=pqzpbQ8MMorrJ3Jp0ntmpZcuvfByyqzMXXi2UujuXD0,137 -examples/wheel/lib/data.txt,sha256=9vJKEdfLu8bZRArKLroPZJh1XKkK3qFMXiM79MBL2Sg,12 -examples/wheel/lib/module_with_data.py,sha256=8s0Khhcqz3yVsBKv2IB5u4l4TMKh7-c_V6p65WVHPms,637 -examples/wheel/lib/simple_module.py,sha256=z2hwciab_XPNIBNH8B1Q5fYgnJvQTeYf0ZQJpY8yLLY,637 -examples/wheel/main.py,sha256=sgg5iWN_9inYBjm6_Zw27hYdmo-l24fA-2rfphT-IlY,909 -""" - unix_expected_contents = b"""\ + + self.assertEqual( + record_contents, + # The entries are guaranteed to be sorted. + b"""\ example_customized-0.0.1.dist-info/METADATA,sha256=TeeEmokHE2NWjkaMcVJuSAq4_AXUoIad2-SLuquRmbg,372 example_customized-0.0.1.dist-info/NOTICE,sha256=Xpdw-FXET1IRgZ_wTkx1YQfo1-alET0FVf6V1LXO4js,76 example_customized-0.0.1.dist-info/README,sha256=WmOFwZ3Jga1bHG3JiGRsUheb4UbLffUxyTdHczS27-o,40 @@ -138,15 +109,7 @@ def test_customized_wheel(self): examples/wheel/lib/module_with_data.py,sha256=8s0Khhcqz3yVsBKv2IB5u4l4TMKh7-c_V6p65WVHPms,637 examples/wheel/lib/simple_module.py,sha256=z2hwciab_XPNIBNH8B1Q5fYgnJvQTeYf0ZQJpY8yLLY,637 examples/wheel/main.py,sha256=sgg5iWN_9inYBjm6_Zw27hYdmo-l24fA-2rfphT-IlY,909 -""" - self.assertIn( - record_contents, - [ - rbe_expected_contents, - unix_expected_contents, - ], - ) - +""") self.assertEqual( wheel_contents, b"""\ @@ -156,44 +119,9 @@ def test_customized_wheel(self): Tag: py3-none-any """, ) - if platform.system() == "Windows": - self.assertEqual( - metadata_contents, - b"""\ -Metadata-Version: 2.1 -Name: example_customized -Version: 0.0.1 -Author: Example Author with non-ascii characters: \xc3\x85\xc2\xbc\xc3\x83\xc2\xb3\xc3\x85\xc2\x82w -Author-email: example@example.com -Home-page: www.example.com -License: Apache 2.0 -Classifier: License :: OSI Approved :: Apache Software License -Classifier: Intended Audience :: Developers -Requires-Dist: pytest - -This is a sample description of a wheel. -""", - ) - else: - # TODO: The non-ascii characters in the METADATA file are interpreted differently on the - # ubuntu16_rbe hosts in comparison to other unix platforms. This should not be the case - # and the code should be updated to account for this. - rbe_expected_contents = b"""\ -Metadata-Version: 2.1 -Name: example_customized -Version: 0.0.1 -Author: Example Author with non-ascii characters: \xc3\x85\xc2\xbc\xc3\x83\xc2\xb3\xc3\x85\xc2\x82w -Author-email: example@example.com -Home-page: www.example.com -License: Apache 2.0 -Classifier: License :: OSI Approved :: Apache Software License -Classifier: Intended Audience :: Developers -Requires-Dist: pytest - -This is a sample description of a wheel. -""" - - unix_expected_contents = b"""\ + self.assertEqual( + metadata_contents, + b"""\ Metadata-Version: 2.1 Name: example_customized Version: 0.0.1 @@ -206,14 +134,7 @@ def test_customized_wheel(self): Requires-Dist: pytest This is a sample description of a wheel. -""" - self.assertIn( - metadata_contents, - [ - rbe_expected_contents, - unix_expected_contents, - ], - ) +""") self.assertEqual( entry_point_contents, b"""\