From 5b164a29c996887be0ffe4adc623b9098f516124 Mon Sep 17 00:00:00 2001 From: UebelAndre Date: Mon, 22 Jan 2024 20:46:01 -0800 Subject: [PATCH] feat(py_wheel): Added `requires_file` and `extra_requires_files` attrs (#1710) The `compile_pip_requirements` rule promotes having `requirements.in` files describe python dependencies. This change aims to allow these files to be the source of truth for constraints by allowing the `py_wheel` rule to use them for adding requirements to a wheel. This reduces overhead in needing to maintain two lists of equal information (one as he `.in` and the other as starlark data). --- CHANGELOG.md | 8 +++++ examples/wheel/BUILD.bazel | 43 ++++++++++++++++++++++++++ examples/wheel/wheel_test.py | 28 +++++++++++++++++ python/private/py_wheel.bzl | 58 +++++++++++++++++++++++++++++++++--- tools/wheelmaker.py | 33 ++++++++++++++++++-- 5 files changed, 163 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 58a872d3fb..8660f6455c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,6 +21,14 @@ A brief description of the categories of changes: [0.XX.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.XX.0 +### Changed + +### Fixed + +### Added + +* (py_wheel) Added `requires_file` and `extra_requires_files` attributes. + ## 0.29.0 - 2024-01-22 [0.29.0]: https://github.com/bazelbuild/rules_python/releases/tag/0.29.0 diff --git a/examples/wheel/BUILD.bazel b/examples/wheel/BUILD.bazel index 5c138a227f..b11ec6903a 100644 --- a/examples/wheel/BUILD.bazel +++ b/examples/wheel/BUILD.bazel @@ -13,6 +13,7 @@ # limitations under the License. load("@bazel_skylib//rules:build_test.bzl", "build_test") +load("@bazel_skylib//rules:write_file.bzl", "write_file") load("//examples/wheel/private:wheel_utils.bzl", "directory_writer", "make_variable_tags") load("//python:defs.bzl", "py_library", "py_test") load("//python:packaging.bzl", "py_package", "py_wheel") @@ -269,6 +270,47 @@ py_wheel( deps = [":example_pkg"], ) +write_file( + name = "requires_file", + out = "requires.txt", + content = """\ +# Requirements file +--index-url https://pypi.com + +tomli>=2.0.0 +""".splitlines(), +) + +write_file( + name = "extra_requires_file", + out = "extra_requires.txt", + content = """\ +# Extras Requirements file +--index-url https://pypi.com + +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" +""".splitlines(), +) + +# 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. This target shows +# how to provide this file. +py_wheel( + name = "requires_files", + distribution = "requires_files", + extra_requires_files = {":extra_requires.txt": "example"}, + 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_file = ":requires.txt", + version = "0.0.1", + deps = [":example_pkg"], +) + py_test( name = "wheel_test", srcs = ["wheel_test.py"], @@ -283,6 +325,7 @@ py_test( ":minimal_with_py_package", ":python_abi3_binary_wheel", ":python_requires_in_a_package", + ":requires_files", ":use_rule_with_dir_in_outs", ], deps = [ diff --git a/examples/wheel/wheel_test.py b/examples/wheel/wheel_test.py index 03a94084f0..a33e435803 100644 --- a/examples/wheel/wheel_test.py +++ b/examples/wheel/wheel_test.py @@ -438,6 +438,34 @@ def test_rule_expands_workspace_status_keys_in_wheel_metadata(self): self.assertNotIn("{BUILD_TIMESTAMP}", version) self.assertNotIn("{BUILD_USER}", name) + def test_requires_file_and_extra_requires_files(self): + filename = self._get_path("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) + + 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: 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() diff --git a/python/private/py_wheel.bzl b/python/private/py_wheel.bzl index bca8615bd6..5919abea05 100644 --- a/python/private/py_wheel.bzl +++ b/python/private/py_wheel.bzl @@ -122,12 +122,26 @@ _feature_flags = {} _requirement_attrs = { "extra_requires": attr.string_list_dict( - doc = "List of optional requirements for this package", + doc = ("A mapping of [extras](https://peps.python.org/pep-0508/#extras) options to lists of requirements (similar to `requires`). This attribute " + + "is mutually exclusive with `extra_requires_file`."), + ), + "extra_requires_files": attr.label_keyed_string_dict( + doc = ("A mapping of requirements files (similar to `requires_file`) to the name of an [extras](https://peps.python.org/pep-0508/#extras) option " + + "This attribute is mutually exclusive with `extra_requires`."), + allow_files = True, ), "requires": attr.string_list( 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."), + "for details and examples of the format of this argument. This " + + "attribute is mutually exclusive with `requires_file`."), + ), + "requires_file": attr.label( + doc = ("A file containing a 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. This " + + "attribute is mutually exclusive with `requires`."), + allow_single_file = True, ), } @@ -365,15 +379,50 @@ def _py_wheel_impl(ctx): 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) + if ctx.attr.requires and ctx.attr.requires_file: + fail("`requires` and `requires_file` are mutually exclusive. Please update {}".format(ctx.label)) + + for requires in ctx.attr.requires: + metadata_contents.append("Requires-Dist: %s" % requires) + if ctx.attr.requires_file: + # The @ prefixed paths will be resolved by the PyWheel action. + # Expanding each line containing a constraint in place of this + # directive. + metadata_contents.append("Requires-Dist: @%s" % ctx.file.requires_file.path) + other_inputs.append(ctx.file.requires_file) + + if ctx.attr.extra_requires and ctx.attr.extra_requires_files: + fail("`extra_requires` and `extra_requires_files` are mutually exclusive. Please update {}".format(ctx.label)) 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), ) + extra_requires_files = {} + for option_requires_target, option in ctx.attr.extra_requires_files.items(): + if option in extra_requires_files: + fail("Duplicate `extra_requires_files` option '{}' found on target {}".format(option, ctx.label)) + option_requires_files = option_requires_target[DefaultInfo].files.to_list() + if len(option_requires_files) != 1: + fail("Labels in `extra_requires_files` must result in a single file, but {label} provides {files} from {owner}".format( + label = ctx.label, + files = option_requires_files, + owner = option_requires_target.label, + )) + extra_requires_files.update({option: option_requires_files[0]}) + + for option, option_requires_file in sorted(extra_requires_files.items()): + metadata_contents.append("Provides-Extra: %s" % option) + metadata_contents.append( + # The @ prefixed paths will be resolved by the PyWheel action. + # Expanding each line containing a constraint in place of this + # directive and appending the extra option. + "Requires-Dist: @%s; extra == '%s'" % (option_requires_file.path, option), + ) + other_inputs.append(option_requires_file) + ctx.actions.write( output = metadata_file, content = "\n".join(metadata_contents) + "\n", @@ -425,6 +474,7 @@ def _py_wheel_impl(ctx): ) ctx.actions.run( + mnemonic = "PyWheel", inputs = depset(direct = other_inputs, transitive = [inputs_to_package]), outputs = [outfile, name_file], arguments = [args], diff --git a/tools/wheelmaker.py b/tools/wheelmaker.py index e2d0121b93..2f9a8cb622 100644 --- a/tools/wheelmaker.py +++ b/tools/wheelmaker.py @@ -510,9 +510,36 @@ def main() -> None: ) as description_file: description = description_file.read() - metadata = None - with open(arguments.metadata_file, "rt", encoding="utf-8") as metadata_file: - metadata = metadata_file.read() + metadata = arguments.metadata_file.read_text(encoding="utf-8") + + # This is not imported at the top of the file due to the reliance + # on this file in the `whl_library` repository rule which does not + # provide `packaging` but does import symbols defined here. + from packaging.requirements import Requirement + + # Search for any `Requires-Dist` entries that refer to other files and + # expand them. + for meta_line in metadata.splitlines(): + if not meta_line.startswith("Requires-Dist: @"): + continue + 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 + + req = Requirement(reqs_text) + if req.marker: + reqs.append( + f"Requires-Dist: {req.name}{req.specifier}; ({req.marker}) and {extra}" + ) + else: + reqs.append(f"Requires-Dist: {req.name}{req.specifier}; {extra}") + + metadata = metadata.replace(meta_line, "\n".join(reqs)) maker.add_metadata( metadata=metadata,