Allow the name_scheme argument to python_wheel to be a list inste…

…ad of a single string. (#171) * Allow the `name_scheme` argument to `python_wheel` to be a list instead of a single string. * Update the comment and tidy a bit * Simplify the build def further, update the comment, and add a test * Add missing newline at eof * Add a comment on the packages we are using for testing
please-build · Aug 21, 2024 · 2bb0d87 · 2bb0d87
1 parent 8e78b49
commit 2bb0d87
Show file tree

Hide file tree

Showing 7 changed files with 71 additions and 35 deletions.
diff --git a/.plzconfig b/.plzconfig
@@ -1,5 +1,5 @@
 [Please]
-Version = >=17.10.1
+Version = >=17.10.3
 
 [Build]
 hashcheckers = sha256

diff --git a/ChangeLog b/ChangeLog
@@ -1,3 +1,8 @@
+Version 1.7.2
+-------------
+    * Allow the `name_scheme` argument to `python_wheel` to be a list instead of a single string.
+    * Update required Please version to 17.10.3 to avoid a memory consumption bug.
+
 Version 1.7.1
 -------------
     * Bug: change so that every URL is passed with its individual --urls flag in python_wheel (#154)

diff --git a/VERSION b/VERSION
@@ -1 +1 @@
-1.7.1
+1.7.2
diff --git a/build_defs/python.build_defs b/build_defs/python.build_defs
@@ -504,22 +504,35 @@ def pip_library(name:str, version:str, labels:list=[], hashes:list=None, package
         visibility = visibility,
     )
 
+# A reasonable set of possible wheel naming schemes.
+# Look for an arch-specific wheel first; in some cases there can be both (e.g. protobuf
+# has optional arch-specific bits) and we prefer the one with the cool stuff.
+_DEFAULT_WHEEL_NAME_SCHEMES = [
+    '{url_base}/{package_name}-{version}-${{OS}}-${{ARCH}}.whl',
+    '{url_base}/{package_name}-{version}-${{OS}}_${{ARCH}}.whl',
+    '{url_base}/{package_name}-{version}.whl',
+]
+
 def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, package_name:str=None,
                  outs:list=None, post_install_commands:list=None, patch:str|list=None, licences:list=None,
                  test_only:bool&testonly=False, repo:str=None, zip_safe:bool=True, visibility:list=None,
-                 deps:list=[], name_scheme:str=None, strip:list=['*.pyc', 'tests'], binary = False,
-                 entry_points={}, tool:str=CONFIG.PYTHON.WHEEL_TOOL, prereleases:bool=CONFIG.PYTHON.PRERELEASES,
+                 deps:list=[], name_scheme:str|list=CONFIG.PYTHON.WHEEL_NAME_SCHEME,
+                 strip:list=['*.pyc', 'tests'], binary = False, entry_points={},
+                 tool:str=CONFIG.PYTHON.WHEEL_TOOL, prereleases:bool=CONFIG.PYTHON.PRERELEASES,
                  tool_verbosity:str=CONFIG.PYTHON.VERBOSITY):
     """Downloads a Python wheel and extracts it.
 
     This is a lightweight pip-free alternative to pip_library which supports cross-compiling.
     Rather than leaning on pip which is difficult to achieve reproducible builds with and
     support on different platforms, this rule is a simple wrapper around curl and unzip.
-    Unless otherwise specified, the wheels are expected to adhere to common naming schemes,
-    such as:
-      <package_name>-<version>[-<os>-<arch>].whl
-      <package_name>-<version>[-<os>_<arch>].whl
-      <package_name>-<version>.whl
+
+    Wheel URLs are resolved in one or both of two ways:
+      * If `repo` is passed or `wheel_repo` is set in the .plzconfig, URLs are constructed using
+        `name_scheme` if passed, else `wheel_name_scheme` if set in the .plzconfig, else using some
+        reasonable defaults defined in _DEFAULT_WHEEL_NAME_SCHEMES.
+      * If `tool` is passed or `wheel_tool` is set in the .plzconfig, the tool is used to look up
+        URLs. If `repo` is also passed or `wheel_repo` is also set in the .plzconfig, the URLs
+        generated from the naming schemes are passed to the resolver tool.
 
     Args:
       name (str): Name of the rule. Also doubles as the name of the package if package_name
@@ -538,8 +551,8 @@ def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, packag
       zip_safe (bool): Flag to indicate whether a pex including this rule will be zip-safe.
       visibility (list): Visibility declaration.
       deps (list): Dependencies of this rule.
-      name_scheme (str): The templatized wheel naming scheme (available template variables
-                         are `url_base`, `package_name`, `initial`, and `version`).
+      name_scheme (str | list): The templatized wheel naming scheme(s) (available template variables
+                                are `url_base`, `package_name`, `initial`, and `version`).
       strip (list): Files to strip after install. Note that these are done at any level.
       binary (bool): Whether this wheel should be executable. This assumes that the wheel will contain a __main__ module
                      with a main() function. If this is not the case, then entry_points should be used.
@@ -558,30 +571,15 @@ def python_wheel(name:str, version:str, labels:list=[], hashes:list=None, packag
         fail('python.wheel_repo is not set in the config, must pass repo explicitly to python_wheel')
     urls = []
     if url_base:
-        if name_scheme:
-            urls += [name_scheme.format(url_base=url_base,
-                                        package_name=package_name,
-                                        initial=initial,
-                                        version=version)]
-        elif CONFIG.PYTHON.WHEEL_NAME_SCHEME:
-            urls += [scheme.format(url_base=url_base, package_name=package_name, initial=initial, version=version)
-                     for scheme in CONFIG.PYTHON.WHEEL_NAME_SCHEME]
-        else:
-            # Populate urls using a reasonable set of possible wheel naming schemes.
-            # Look for an arch-specific wheel first; in some cases there can be both (e.g. protobuf
-            # has optional arch-specific bits) and we prefer the one with the cool stuff.
-            urls += ['{url_base}/{package_name}-{version}-${{OS}}-${{ARCH}}.whl'.format(url_base=url_base,
-                                                                                        package_name=package_name,
-                                                                                        initial=initial,
-                                                                                        version=version),
-                     '{url_base}/{package_name}-{version}-${{OS}}_${{ARCH}}.whl'.format(url_base=url_base,
-                                                                                        package_name=package_name,
-                                                                                        initial=initial,
-                                                                                        version=version),
-                     '{url_base}/{package_name}-{version}.whl'.format(url_base=url_base,
-                                                                      package_name=package_name,
-                                                                      initial=initial,
-                                                                      version=version)]
+        if not name_scheme:
+            name_scheme = _DEFAULT_WHEEL_NAME_SCHEMES
+        elif isinstance(name_scheme, str):
+            name_scheme = [name_scheme]
+
+        urls += [
+            scheme.format(url_base=url_base, package_name=package_name, initial=initial, version=version)
+            for scheme in name_scheme
+        ]
 
     file_rule = None
     if tool:

diff --git a/test/BUILD b/test/BUILD
@@ -182,3 +182,17 @@ plz_e2e_test(
     labels = ["py3"],
     deps = ["//third_party/python:grpcio"],
 )
+
+# Test that python_wheel targets can have name_scheme as a list or a string
+
+python_test(
+    name = "name_scheme_test",
+    srcs = ["name_scheme_test.py"],
+    labels = [
+        "py3",
+    ],
+    deps = [
+        "//third_party/python:click",
+        "//third_party/python:click-log",
+    ],
+)
diff --git a/test/name_scheme_test.py b/test/name_scheme_test.py
@@ -0,0 +1,15 @@
+import unittest
+
+
+class NameSchemeTest(unittest.TestCase):
+
+    # The click python_wheel has no name_scheme argument, and is thus using the default value which
+    # is a list.
+    def test_click_is_importable(self):
+        import click
+        self.assertIsNotNone(click.command)
+
+    # The click-log python_wheel has an explicit name_scheme argument which is a string.
+    def test_click_log_is_importable(self):
+        import click_log
+        self.assertIsNotNone(click_log.basic_config)
diff --git a/third_party/python/BUILD b/third_party/python/BUILD
@@ -574,6 +574,8 @@ filegroup(
     ],
 )
 
+# Note: this package is used for //test:name_scheme_test as an example of something using a list of
+# name schemes
 python_wheel(
     name = "click",
     hashes = [],
@@ -582,6 +584,8 @@ python_wheel(
     deps = [],
 )
 
+# Note: this package is used for //test:name_scheme_test as an example of something using a single
+# string name scheme.
 python_wheel(
     name = "click-log",
     package_name = "click_log",