make the changes fully backwards compatible and ensure that the WORKSPACE code works

Author: aignas

CHANGELOG.md

@@ -33,12 +33,12 @@ A brief description of the categories of changes:
 
 ### Added
 
-* (pip) Allow specifying the requirements by (os, arch) and add extra validations when
-  parsing the inputs. This should be a non-breaking change for most users
-  unless they have been passing `requirements_linux` together with
-  `extra_pip_args = ["--platform=manylinux_2_4_x86_64"]`, in which case they
-  would have to change their code to use `requirements_lock` attribute which
-  itself is a trivial change.
+* (pip) Allow specifying the requirements by (os, arch) and add extra
+  validations when parsing the inputs. This is a non-breaking change for most
+  users unless they have been passing multiple `requirements_*` files together
+  with `extra_pip_args = ["--platform=manylinux_2_4_x86_64"]`, that was an
+  invalid usage previously but we were not failing the build. From now on this
+  is explicitly disallowed.
 
 ## [0.32.2] - 2024-05-14
 

docs/sphinx/pip.md

@@ -19,11 +19,41 @@ load("@pip_deps//:requirements.bzl", "install_deps")
 install_deps()
 ```
 
+For `bzlmod` an equivalent `MODULE.bazel` would look like:
+```starlark
+pip = use_extension("//python/extensions:pip.bzl", "pip")
+pip.parse(
+    hub_name = "pip_deps",
+    requirements_lock = ":requirements.txt",
+)
+use_repo(pip, "pip_deps")
+```
+
 You can then reference installed dependencies from a `BUILD` file with:
 
 ```starlark
 load("@pip_deps//:requirements.bzl", "requirement")
 
+py_library(
+    name = "bar",
+    ...
+    deps = [
+        "//my/other:dep",
+        "@pip_deps//requests",
+        "@pip_deps//numpy",
+    ],
+)
+```
+
+The rules also provide a convenience macro for translating the entries in the
+`requirements.txt` file (e.g. `opencv-python`) to the right bazel label (e.g.
+`@pip_deps//opencv_python`). The convention of bazel labels is lowercase
+`snake_case`, but you can use the helper to avoid depending on this convention
+as follows:
+
+```starlark
+load("@pip_deps//:requirements.bzl", "requirement")
+
 py_library(
     name = "bar",
     ...
@@ -35,33 +65,35 @@ py_library(
 )
 ```
 
-In addition to the `requirement` macro, which is used to access the generated `py_library`
-target generated from a package's wheel, The generated `requirements.bzl` file contains
-functionality for exposing [entry points][whl_ep] as `py_binary` targets as well.
+If you would like to access [entry points][whl_ep], see the `py_console_script_binary` rule documentation.
 
 [whl_ep]: https://packaging.python.org/specifications/entry-points/
 
-```starlark
-load("@pip_deps//:requirements.bzl", "entry_point")
-
-alias(
-    name = "pip-compile",
-    actual = entry_point(
-        pkg = "pip-tools",
-        script = "pip-compile",
-    ),
-)
-```
+(per-os-arch-requirements)=
+## Requirements for a specific OS/Architecture
 
-Note that for packages whose name and script are the same, only the name of the package
-is needed when calling the `entry_point` macro.
+IN some cases you may need to use different requirements files for different OS, Arch combinations. This is enabled via the `requirements_by_platform` attribute in `pip.parse` extension and the `pip_parse` repository rule. The keys of the dictionary are labels to the file and the values are a list of comma separated target (os, arch) tuples.
 
+For example:
 ```starlark
-load("@pip_deps//:requirements.bzl", "entry_point")
+    # ...
+    requirements_by_platform = {
+        "requirements_linux_x86_64.txt": "linux_x86_64",
+        "requirements_osx.txt": "osx_x86_64,osx_aarch64",
+        "requirements_linux_exotic.txt": "linux_exotic",
+    },
+    # For the list of standard platforms that the rules_python has toolchains for, default to
+    # the following requirements file.
+    "requirements_lock" = "requirements_lock.txt",
+```
 
-alias(
-    name = "flake8",
-    actual = entry_point("flake8"),
+An alternative way is to use per-OS requirement attributes.
+```starlark
+    # ...
+    requirements_darwin = "requirements_darwin.txt",
+    # For the list of standard platforms that the rules_python has toolchains for, default to
+    # the following requirements file.
+    "requirements_lock" = "requirements_lock.txt",
 )
 ```
 

python/pip_install/pip_repository.bzl

@@ -24,7 +24,7 @@ load("//python/pip_install/private:srcs.bzl", "PIP_INSTALL_PY_SRCS")
 load("//python/private:auth.bzl", "AUTH_ATTRS", "get_auth")
 load("//python/private:envsubst.bzl", "envsubst")
 load("//python/private:normalize_name.bzl", "normalize_name")
-load("//python/private:parse_requirements.bzl", "parse_requirements", "select_requirement")
+load("//python/private:parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
 load("//python/private:parse_whl_name.bzl", "parse_whl_name")
 load("//python/private:patch_whl.bzl", "patch_whl")
 load("//python/private:render_pkg_aliases.bzl", "render_pkg_aliases", "whl_alias")
@@ -284,11 +284,11 @@ def _pip_repository_impl(rctx):
     )
     selected_requirements = {}
     options = None
+    repository_platform = host_platform(rctx.os)
     for name, requirements in requirements_by_platform.items():
         r = select_requirement(
             requirements,
-            host_os = rctx.os.name,
-            host_cpu = rctx.os.arch,
+            platform = repository_platform,
         )
         if not r:
             continue

python/private/bzlmod/pip.bzl

@@ -24,7 +24,7 @@ load(
 )
 load("//python/private:auth.bzl", "AUTH_ATTRS")
 load("//python/private:normalize_name.bzl", "normalize_name")
-load("//python/private:parse_requirements.bzl", "parse_requirements", "select_requirement")
+load("//python/private:parse_requirements.bzl", "host_platform", "parse_requirements", "select_requirement")
 load("//python/private:parse_whl_name.bzl", "parse_whl_name")
 load("//python/private:pypi_index.bzl", "simpleapi_download")
 load("//python/private:render_pkg_aliases.bzl", "whl_alias")
@@ -162,7 +162,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
 
     requirements_by_platform = parse_requirements(
         module_ctx,
-        requirements_by_platform = pip_attr.experimental_requirements_by_platform,
+        requirements_by_platform = pip_attr.requirements_by_platform,
         requirements_linux = pip_attr.requirements_linux,
         requirements_lock = pip_attr.requirements_lock,
         requirements_osx = pip_attr.requirements_darwin,
@@ -195,11 +195,11 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
             parallel_download = pip_attr.parallel_download,
         )
 
+    repository_platform = host_platform(module_ctx.os)
     for whl_name, requirements in requirements_by_platform.items():
         requirement = select_requirement(
             requirements,
-            host_os = module_ctx.os.name,
-            host_cpu = module_ctx.os.arch,
+            platform = repository_platform,
         )
         if not requirement:
             # Sometimes the package is not present for host platform if there
@@ -284,8 +284,7 @@ def _create_whl_repos(module_ctx, pip_attr, whl_map, whl_overrides, group_map, s
                     # Older python versions have wheels for the `*m` ABI.
                     "cp" + major_minor.replace(".", "") + "m",
                 ],
-                want_os = module_ctx.os.name,
-                want_cpu = module_ctx.os.arch,
+                want_platform = repository_platform,
             ) or sdist
 
             if distribution:

python/private/normalize_platform.bzl

@@ -11,57 +11,3 @@
 # 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.
-
-"""
-A small function to normalize the platform name.
-
-This includes the vendored _translate_cpu and _translate_os from
-@platforms//host:extension.bzl at version 0.0.9 so that we don't
-force the users to depend on it.
-"""
-
-def _translate_cpu(arch):
-    if arch in ["i386", "i486", "i586", "i686", "i786", "x86"]:
-        return "x86_32"
-    if arch in ["amd64", "x86_64", "x64"]:
-        return "x86_64"
-    if arch in ["ppc", "ppc64", "ppc64le"]:
-        return "ppc"
-    if arch in ["arm", "armv7l"]:
-        return "arm"
-    if arch in ["aarch64"]:
-        return "aarch64"
-    if arch in ["s390x", "s390"]:
-        return "s390x"
-    if arch in ["mips64el", "mips64"]:
-        return "mips64"
-    if arch in ["riscv64"]:
-        return "riscv64"
-    return None
-
-def _translate_os(os):
-    if os.startswith("mac os"):
-        return "osx"
-    if os.startswith("freebsd"):
-        return "freebsd"
-    if os.startswith("openbsd"):
-        return "openbsd"
-    if os.startswith("linux"):
-        return "linux"
-    if os.startswith("windows"):
-        return "windows"
-    return None
-
-def normalize_platform(*, os, cpu):
-    """Normalize the platform.
-
-    Args:
-        os: The operating system name.
-        cpu: The CPU name.
-
-    Returns: A normalized string
-    """
-    return "{}_{}".format(
-        _translate_os(os),
-        _translate_cpu(cpu),
-    )

python/private/parse_requirements.bzl

@@ -28,10 +28,45 @@ behavior.
 
 load("//python/pip_install:requirements_parser.bzl", "parse")
 load(":normalize_name.bzl", "normalize_name")
-load(":normalize_platform.bzl", "normalize_platform")
 load(":pypi_index_sources.bzl", "get_simpleapi_sources")
 load(":whl_target_platforms.bzl", "whl_target_platforms")
 
+# This includes the vendored _translate_cpu and _translate_os from
+# @platforms//host:extension.bzl at version 0.0.9 so that we don't
+# force the users to depend on it.
+
+def _translate_cpu(arch):
+    if arch in ["i386", "i486", "i586", "i686", "i786", "x86"]:
+        return "x86_32"
+    if arch in ["amd64", "x86_64", "x64"]:
+        return "x86_64"
+    if arch in ["ppc", "ppc64", "ppc64le"]:
+        return "ppc"
+    if arch in ["arm", "armv7l"]:
+        return "arm"
+    if arch in ["aarch64"]:
+        return "aarch64"
+    if arch in ["s390x", "s390"]:
+        return "s390x"
+    if arch in ["mips64el", "mips64"]:
+        return "mips64"
+    if arch in ["riscv64"]:
+        return "riscv64"
+    return arch
+
+def _translate_os(os):
+    if os.startswith("mac os"):
+        return "osx"
+    if os.startswith("freebsd"):
+        return "freebsd"
+    if os.startswith("openbsd"):
+        return "openbsd"
+    if os.startswith("linux"):
+        return "linux"
+    if os.startswith("windows"):
+        return "windows"
+    return os
+
 # TODO @aignas 2024-05-13: consider using the same platform tags as are used in
 # the //python:versions.bzl
 DEFAULT_PLATFORMS = [
@@ -104,7 +139,7 @@ def parse_requirements(
         requirements_windows (label): The requirements file for windows OS.
         extra_pip_args (string list): Extra pip arguments to perform extra validations and to
             be joined with args fined in files.
-        fail_fn (fn): A failure function used in testing failure cases.
+        fail_fn (Callable[[str], None]): A failure function used in testing failure cases.
 
     Returns:
         A tuple where the first element a dict of dicts where the first key is
@@ -125,29 +160,36 @@ def parse_requirements(
         requirements_windows or
         requirements_by_platform
     ):
-        fail_fn("""\
-A requirements_lock attribute must be specified, or a platform-specific lockfile using one of the requirements_* or requirements_by_platform attributes.
-""")
+        fail_fn(
+            "A 'requirements_lock' attribute must be specified, a platform-specific lockfiles " +
+            "via 'requirements_by_platform' or an os-specific lockfiles must be specified " +
+            "via 'requirements_*' attributes",
+        )
         return None
 
     platforms = _platforms_from_args(extra_pip_args)
 
-    if platforms and (
-        not requirements_lock or
-        requirements_linux or
-        requirements_osx or
-        requirements_windows or
-        requirements_by_platform
-    ):
-        # If the --platform argument is used, check that we are using
-        # `requirements_lock` file instead of the OS specific ones as that is
-        # the only correct way to use the API.
-        fail_fn("only 'requirements_lock' can be used when using '--platform' pip argument")
-        return None
-
     if platforms:
+        lock_files = [
+            f
+            for f in [
+                requirements_lock,
+                requirements_linux,
+                requirements_osx,
+                requirements_windows,
+            ] + list(requirements_by_platform.keys())
+            if f
+        ]
+
+        if len(lock_files) > 1:
+            # If the --platform argument is used, check that we are using
+            # a single `requirements_lock` file instead of the OS specific ones as that is
+            # the only correct way to use the API.
+            fail_fn("only a single 'requirements_lock' file can be used when using '--platform' pip argument, consider specifying it via 'requirements_lock' attribute")
+            return None
+
         files_by_platform = [
-            (requirements_lock, platforms),
+            (lock_files[0], platforms),
         ]
     else:
         files_by_platform = {
@@ -257,21 +299,20 @@ A requirements_lock attribute must be specified, or a platform-specific lockfile
         for whl_name, reqs in requirements_by_platform.items()
     }
 
-def select_requirement(requirements, *, host_os, host_cpu):
+def select_requirement(requirements, *, platform):
     """A simple function to get a requirement for a particular platform.
 
     Args:
         requirements (list[struct]): The list of requirements as returned by
             the `parse_requirements` function above.
-        host_os (str): The host OS.
-        host_cpu (str): The host CPU.
+        platform (str): The host platform. Usually an output of the
+        `host_platform` function.
 
     Returns:
         None if not found or a struct returned as one of the values in the
         parse_requirements function. The requirement that should be downloaded
         by the host platform will be returned.
     """
-    platform = normalize_platform(os = host_os, cpu = host_cpu)
     maybe_requirement = [
         req
         for req in requirements
@@ -287,3 +328,19 @@ def select_requirement(requirements, *, host_os, host_cpu):
         return None
 
     return maybe_requirement[0]
+
+def host_platform(repository_os):
+    """Return a string representation of the repository OS.
+
+    Args:
+        repository_os (struct): The `module_ctx.os` or `repository_ctx.os` attribute.
+            See https://bazel.build/rules/lib/builtins/repository_os.html
+
+    Returns:
+        The string representation of the platform that we can later used in the `pip`
+        machinery.
+    """
+    return "{}_{}".format(
+        _translate_os(repository_os.name.lower()),
+        _translate_cpu(repository_os.arch.lower()),
+    )