fix: Don't require default Python version for pip hubs (#1344)

rickeylev · web-flow · commit afc40f018b93 · 2023-07-31T20:33:48.000Z
This fixes the issue where a sub-module was required to always have a pip.parse() call configured for the default Python version if it used any pip.parse() call. Such a requirement puts sub-modules in an impossible situation: * If they don't have the default version, they'll get an error. * If they register the default version, but also register a specific version, they'll potentially cause an error if a root module changes the default to match their specific version (because two pip.parse() calls for the same version are made, which is an error). The requirement to have the default version registered for a pip hub was only present to satisfy the `whl_library_alias` repository rule, which needed a Python version to map `//conditions:default` to. To fix, the `whl_library_alias` rule's `default_version` arg is made optional. When None is passed, the `//conditions:default` condition is replaced with a `no_match_error` setting. This prevents the pip hub from being used with the version-unaware rules, but that makes sense: no wheels were setup for that version, so it's not like there is something that can be used anyways. Fixes #1320
diff --git a/.bazelrc b/.bazelrc
@@ -3,8 +3,8 @@
 # This lets us glob() up all the files inside the examples to make them inputs to tests
 # (Note, we cannot use `common --deleted_packages` because the bazel version command doesn't support it)
 # To update these lines, run tools/bazel_integration_test/update_deleted_packages.sh
-build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod/entry_point,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/whl_mods,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,tests/compile_pip_requirements,tests/compile_pip_requirements_test_from_external_workspace,tests/ignore_root_user_error,tests/pip_repository_entry_points
-query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod/entry_point,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/whl_mods,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,tests/compile_pip_requirements,tests/compile_pip_requirements_test_from_external_workspace,tests/ignore_root_user_error,tests/pip_repository_entry_points
+build --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_point,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,tests/compile_pip_requirements,tests/compile_pip_requirements_test_from_external_workspace,tests/ignore_root_user_error,tests/pip_repository_entry_points
+query --deleted_packages=examples/build_file_generation,examples/build_file_generation/random_number_generator,examples/bzlmod,examples/bzlmod_build_file_generation,examples/bzlmod_build_file_generation/other_module/other_module/pkg,examples/bzlmod_build_file_generation/runfiles,examples/bzlmod/entry_point,examples/bzlmod/libs/my_lib,examples/bzlmod/other_module,examples/bzlmod/other_module/other_module/pkg,examples/bzlmod/runfiles,examples/bzlmod/tests,examples/bzlmod/tests/other_module,examples/bzlmod/whl_mods,examples/multi_python_versions/libs/my_lib,examples/multi_python_versions/requirements,examples/multi_python_versions/tests,examples/pip_install,examples/pip_parse,examples/pip_parse_vendored,examples/pip_repository_annotations,examples/py_proto_library,tests/compile_pip_requirements,tests/compile_pip_requirements_test_from_external_workspace,tests/ignore_root_user_error,tests/pip_repository_entry_points
 
 test --test_output=errors
 
diff --git a/docs/pip.md b/docs/pip.md
diff --git a/examples/bzlmod/other_module/BUILD.bazel b/examples/bzlmod/other_module/BUILD.bazel
@@ -0,0 +1,9 @@
+load("@python_versions//3.11:defs.bzl", compile_pip_requirements_311 = "compile_pip_requirements")
+
+# NOTE: To update the requirements, you need to uncomment the rules_python
+# override in the MODULE.bazel.
+compile_pip_requirements_311(
+    name = "requirements",
+    requirements_in = "requirements.in",
+    requirements_txt = "requirements_lock_3_11.txt",
+)
diff --git a/examples/bzlmod/other_module/MODULE.bazel b/examples/bzlmod/other_module/MODULE.bazel
@@ -6,10 +6,20 @@ module(
 # that the parent module uses.
 bazel_dep(name = "rules_python", version = "")
 
-# It is not best practice to use a python.toolchian in
-# a submodule.  This code only exists to test that
-# we support doing this. This code is only for rules_python
-# testing purposes.
+# The story behind this commented out override:
+# This override is necessary to generate/update the requirements file
+# for this module. This is because running it via the outer
+# module doesn't work -- the `requirements.update` target can't find
+# the correct file to update.
+# Running in the submodule itself works, but submodules using overrides
+# is considered an error until Bazel 6.3, which prevents the outer module
+# from depending on this module.
+# So until 6.3 and higher is the minimum, we leave this commented out.
+# local_path_override(
+#     module_name = "rules_python",
+#     path = "../../..",
+# )
+
 PYTHON_NAME_39 = "python_3_9"
 
 PYTHON_NAME_311 = "python_3_11"
@@ -29,6 +39,20 @@ python.toolchain(
 # created by the above python.toolchain calls.
 use_repo(
     python,
+    "python_versions",
     PYTHON_NAME_39,
     PYTHON_NAME_311,
 )
+
+pip = use_extension("@rules_python//python/extensions:pip.bzl", "pip")
+pip.parse(
+    hub_name = "other_module_pip",
+    # NOTE: This version must be different than the root module's
+    # default python version.
+    # This is testing that a sub-module can use pip.parse() and only specify
+    # Python versions that DON'T include whatever the root-module's default
+    # Python version is.
+    python_version = "3.11",
+    requirements_lock = ":requirements_lock_3_11.txt",
+)
+use_repo(pip, "other_module_pip")
diff --git a/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel b/examples/bzlmod/other_module/other_module/pkg/BUILD.bazel
@@ -1,4 +1,7 @@
-load("@python_3_11//:defs.bzl", py_binary_311 = "py_binary")
+load(
+    "@python_3_11//:defs.bzl",
+    py_binary_311 = "py_binary",
+)
 load("@rules_python//python:defs.bzl", "py_library")
 
 py_library(
@@ -13,11 +16,16 @@ py_library(
 # used only when you need to support multiple versions of Python
 # in the same project.
 py_binary_311(
-    name = "lib_311",
-    srcs = ["lib.py"],
+    name = "bin",
+    srcs = ["bin.py"],
     data = ["data/data.txt"],
+    main = "bin.py",
     visibility = ["//visibility:public"],
-    deps = ["@rules_python//python/runfiles"],
+    deps = [
+        ":lib",
+        "@other_module_pip//absl_py",
+        "@rules_python//python/runfiles",
+    ],
 )
 
 exports_files(["data/data.txt"])
diff --git a/examples/bzlmod/other_module/other_module/pkg/bin.py b/examples/bzlmod/other_module/other_module/pkg/bin.py
@@ -0,0 +1,6 @@
+import sys
+
+import absl
+
+print("Python version:", sys.version)
+print("Module 'absl':", absl)
diff --git a/examples/bzlmod/other_module/requirements.in b/examples/bzlmod/other_module/requirements.in
@@ -0,0 +1 @@
+absl-py
diff --git a/examples/bzlmod/other_module/requirements_lock_3_11.txt b/examples/bzlmod/other_module/requirements_lock_3_11.txt
@@ -0,0 +1,10 @@
+#
+# This file is autogenerated by pip-compile with Python 3.11
+# by the following command:
+#
+#    bazel run //other_module/pkg:requirements.update
+#
+absl-py==1.4.0 \
+    --hash=sha256:0d3fe606adfa4f7db64792dd4c7aee4ee0c38ab75dfd353b7a83ed3e957fcb47 \
+    --hash=sha256:d2c244d01048ba476e7c080bd2c6df5e141d211de80223460d5b3b8a2a58433d
+    # via -r other_module/pkg/requirements.in
diff --git a/examples/bzlmod/tests/other_module/BUILD.bazel b/examples/bzlmod/tests/other_module/BUILD.bazel
@@ -0,0 +1,14 @@
+# Tests to verify the root module can interact with the "other_module"
+# submodule.
+#
+# Note that other_module is seen as "our_other_module" due to repo-remapping
+# in the root module.
+
+load("@bazel_skylib//rules:build_test.bzl", "build_test")
+
+build_test(
+    name = "other_module_bin_build_test",
+    targets = [
+        "@our_other_module//other_module/pkg:bin",
+    ],
+)
diff --git a/python/extensions/pip.bzl b/python/extensions/pip.bzl
@@ -296,23 +296,18 @@ def _pip_impl(module_ctx):
 
     for hub_name, whl_map in hub_whl_map.items():
         for whl_name, version_map in whl_map.items():
-            if DEFAULT_PYTHON_VERSION not in version_map:
-                fail((
-                    "Default python version '{version}' missing in pip " +
-                    "hub '{hub}': update your pip.parse() calls so that " +
-                    'includes `python_version = "{version}"`'
-                ).format(
-                    version = DEFAULT_PYTHON_VERSION,
-                    hub = hub_name,
-                ))
+            if DEFAULT_PYTHON_VERSION in version_map:
+                whl_default_version = DEFAULT_PYTHON_VERSION
+            else:
+                whl_default_version = None
 
             # Create the alias repositories which contains different select
             # statements  These select statements point to the different pip
             # whls that are based on a specific version of Python.
             whl_library_alias(
                 name = hub_name + "_" + whl_name,
                 wheel_name = whl_name,
-                default_version = DEFAULT_PYTHON_VERSION,
+                default_version = whl_default_version,
                 version_map = version_map,
             )
 
@@ -362,7 +357,7 @@ configured.
             mandatory = False,
             doc = """\
 A dict of labels to wheel names that is typically generated by the whl_modifications.
-The labels are JSON config files describing the modifications.           
+The labels are JSON config files describing the modifications.
 """,
         ),
     }, **pip_repository_attrs)
@@ -395,7 +390,7 @@ executable.""",
         ),
         "copy_files": attr.string_dict(
             doc = """\
-(dict, optional): A mapping of `src` and `out` files for 
+(dict, optional): A mapping of `src` and `out` files for
 [@bazel_skylib//rules:copy_file.bzl][cf]""",
         ),
         "data": attr.string_list(
@@ -456,10 +451,10 @@ the BUILD files for wheels.
             attrs = _pip_parse_ext_attrs(),
             doc = """\
 This tag class is used to create a pip hub and all of the spokes that are part of that hub.
-This tag class reuses most of the pip attributes that are found in 
+This tag class reuses most of the pip attributes that are found in
 @rules_python//python/pip_install:pip_repository.bzl.
-The exceptions are it does not use the args 'repo_prefix', 
-and 'incompatible_generate_aliases'.  We set the repository prefix 
+The exceptions are it does not use the args 'repo_prefix',
+and 'incompatible_generate_aliases'.  We set the repository prefix
 for the user and the alias arg is always True in bzlmod.
 """,
         ),
@@ -483,7 +478,7 @@ def _whl_mods_repo_impl(rctx):
 
 _whl_mods_repo = repository_rule(
     doc = """\
-This rule creates json files based on the whl_mods attribute. 
+This rule creates json files based on the whl_mods attribute.
 """,
     implementation = _whl_mods_repo_impl,
     attrs = {
diff --git a/python/pip.bzl b/python/pip.bzl
@@ -22,6 +22,25 @@ load(":versions.bzl", "MINOR_MAPPING")
 compile_pip_requirements = _compile_pip_requirements
 package_annotation = _package_annotation
 
+_NO_MATCH_ERROR_MESSAGE_TEMPLATE = """\
+No matching wheel for current configuration's Python version.
+
+The current build configuration's Python version doesn't match any of the Python
+versions available for this wheel. This wheel supports the following Python versions:
+    {supported_versions}
+
+As matched by the `@{rules_python}//python/config_settings:is_python_<version>`
+configuration settings.
+
+To determine the current configuration's Python version, run:
+    `bazel config <config id>` (shown further below)
+and look for
+    {rules_python}//python/config_settings:python_version
+
+If the value is missing, then the "default" Python version is being used,
+which has a "null" version value and will not match version constraints.
+"""
+
 def pip_install(requirements = None, name = "pip", **kwargs):
     """Accepts a locked/compiled requirements file and installs the dependencies listed within.
 
@@ -260,13 +279,10 @@ _multi_pip_parse = repository_rule(
 
 def _whl_library_alias_impl(rctx):
     rules_python = rctx.attr._rules_python_workspace.workspace_name
-    if rctx.attr.default_version not in rctx.attr.version_map:
-        fail(
-            """
-Unable to find '{}' in your version map, you may need to update your requirement files.
-        """.format(rctx.attr.version_map),
-        )
-    default_repo_prefix = rctx.attr.version_map[rctx.attr.default_version]
+    if rctx.attr.default_version:
+        default_repo_prefix = rctx.attr.version_map[rctx.attr.default_version]
+    else:
+        default_repo_prefix = None
     version_map = rctx.attr.version_map.items()
     build_content = ["# Generated by python/pip.bzl"]
     for alias_name in ["pkg", "whl", "data", "dist_info"]:
@@ -289,6 +305,7 @@ def _whl_library_render_alias_target(
     # is canonical, so we have to add a second @.
     if BZLMOD_ENABLED:
         rules_python = "@" + rules_python
+
     alias = ["""\
 alias(
     name = "{alias_name}",
@@ -304,23 +321,42 @@ alias(
             ),
             rules_python = rules_python,
         ))
-    alias.append("""\
-        "//conditions:default": "{default_actual}",
-    }}),
-    visibility = ["//visibility:public"],
-)""".format(
+    if default_repo_prefix:
         default_actual = "@{repo_prefix}{wheel_name}//:{alias_name}".format(
             repo_prefix = default_repo_prefix,
             wheel_name = wheel_name,
             alias_name = alias_name,
-        ),
-    ))
+        )
+        alias.append('        "//conditions:default": "{default_actual}",'.format(
+            default_actual = default_actual,
+        ))
+
+    alias.append("    },")  # Close select expression condition dict
+    if not default_repo_prefix:
+        supported_versions = sorted([python_version for python_version, _ in version_map])
+        alias.append('    no_match_error="""{}""",'.format(
+            _NO_MATCH_ERROR_MESSAGE_TEMPLATE.format(
+                supported_versions = ", ".join(supported_versions),
+                rules_python = rules_python,
+            ),
+        ))
+    alias.append("    ),")  # Close the select expression
+    alias.append('    visibility = ["//visibility:public"],')
+    alias.append(")")  # Close the alias() expression
     return "\n".join(alias)
 
 whl_library_alias = repository_rule(
     _whl_library_alias_impl,
     attrs = {
-        "default_version": attr.string(mandatory = True),
+        "default_version": attr.string(
+            mandatory = False,
+            doc = "Optional Python version in major.minor format, e.g. '3.10'." +
+                  "The Python version of the wheel to use when the versions " +
+                  "from `version_map` don't match. This allows the default " +
+                  "(version unaware) rules to match and select a wheel. If " +
+                  "not specified, then the default rules won't be able to " +
+                  "resolve a wheel and an error will occur.",
+        ),
         "version_map": attr.string_dict(mandatory = True),
         "wheel_name": attr.string(mandatory = True),
         "_rules_python_workspace": attr.label(default = Label("//:WORKSPACE")),
