improve docs - add repositories.bzl docs and improve docs for bzlmod

Author: aignas

CHANGELOG.md

@@ -25,7 +25,12 @@ A brief description of the categories of changes:
 [x.x.x]: https://github.com/bazelbuild/rules_python/releases/tag/x.x.x
 
 ### Changed
-* (gazelle): Update error messages when unable to resolve a dependency to be more human-friendly.
+* (gazelle): Update error messages when unable to resolve a dependency to be
+  more human-friendly.
+* (toolchain): The toolchain patches now expose the `patch_strip` attribute
+  that one should use when patching toolchains. Please set it if you are
+  patching python interpreter. In the next release the default will be set to
+  `0` which better reflects the defaults used in public `bazel` APIs.
 
 ### Fixed
 * (whl_library): Remove `--no-index` and add `--no-build-isolation` to the

docs/BUILD.bazel

@@ -94,6 +94,8 @@ sphinx_stardocs(
         "//python/private/common:py_library_rule_bazel_bzl",
         "//python/private/common:py_runtime_rule_bzl",
         "//python/private/common:py_test_rule_bazel_bzl",
+        # FIXME @aignas 2024-08-26: should we have this here?
+        "//python:repositories_bzl",
     ] + ([
         # Bazel 6 + Stardoc isn't able to parse something about the python bzlmod extension
         "//python/extensions:python_bzl",

python/private/BUILD.bazel

@@ -298,6 +298,7 @@ bzl_library(
     srcs = ["toolchains_repo.bzl"],
     deps = [
         ":repo_utils_bzl",
+        ":text_util_bzl",
         "//python:versions_bzl",
     ],
 )

python/private/common/py_runtime_rule.bzl

@@ -204,8 +204,8 @@ See @bazel_tools//tools/python:python_bootstrap_template.txt for more variables.
         "coverage_tool": attr.label(
             allow_files = False,
             doc = """
-This is a target to use for collecting code coverage information from `py_binary`
-and `py_test` targets.
+This is a target to use for collecting code coverage information from
+{rule}`py_binary` and {rule}`py_test` targets.
 
 If set, the target must either produce a single file or be an executable target.
 The path to the single file, or the executable if the target is executable,
@@ -214,7 +214,7 @@ runfiles will be added to the runfiles when coverage is enabled.
 
 The entry point for the tool must be loadable by a Python interpreter (e.g. a
 `.py` or `.pyc` file).  It must accept the command line arguments
-of coverage.py (https://coverage.readthedocs.io), at least including
+of [`coverage.py`](https://coverage.readthedocs.io), at least including
 the `run` and `lcov` subcommands.
 """,
         ),
@@ -306,7 +306,7 @@ The template to use when two stage bootstrapping is enabled
             default = DEFAULT_STUB_SHEBANG,
             doc = """
 "Shebang" expression prepended to the bootstrapping Python stub script
-used when executing `py_binary` targets.
+used when executing {rule}`py_binary` targets.
 
 See https://github.com/bazelbuild/bazel/issues/8685 for
 motivation.

python/private/python.bzl

@@ -147,7 +147,7 @@ def _python_impl(module_ctx):
                     base_url = python_tools.base_url,
                     ignore_root_user_error = ignore_root_user_error,
                     tool_versions = python_tools.available_versions,
-                    # TODO @aignas 2024-08-08: allow to modify these values via the bzlmod extension
+                    # TODO @aignas 2024-08-08: allow modifying these values via the bzlmod extension
                     # distutils_content = None,
                 )
                 global_toolchain_versions[toolchain_version] = toolchain_info
@@ -280,11 +280,16 @@ def _process_tag_classes(mod, fail = fail):
 
                 sha256[p] = sha
 
-            available_versions[tag.version] = {
+            version = {
+                "patch_strip": tag.patch_strip,
+                # Use a list if there is only a single item the key is `*`,
+                # otherwise pass the dict down to the toolchain rule.
+                "patches": tag.patches.get("*", tag.patches) if len(tag.patches) == 1 else tag.patches,
                 "sha256": sha256,
                 "strip_prefix": tag.strip_prefix,
                 "url": tag.url,
             }
+            available_versions[tag.version] = version
 
         register_all = False
         for tag in mod.tags.override:
@@ -343,12 +348,6 @@ def _get_bazel_version_specific_kwargs():
 
     return kwargs
 
-_rules_python_private_testing = tag_class(
-    attrs = {
-        "register_all_versions": attr.bool(default = False),
-    },
-)
-
 _toolchain = tag_class(
     doc = """Tag class used to register Python toolchains.
 Use this tag class to register one or more Python toolchains. This class
@@ -413,7 +412,11 @@ can result in spurious build failures.
 )
 
 _override = tag_class(
-    doc = """Tag class used to override defaults and behaviour of the module extension.""",
+    doc = """Tag class used to override defaults and behaviour of the module extension.
+
+:::{versionadded} 0.36.0
+:::
+""",
     attrs = {
         "available_python_versions": attr.string_list(
             mandatory = False,
@@ -426,13 +429,27 @@ _override = tag_class(
         ),
 
         # Internal attributes that are only usable from `rules_python`
-        "register_all_versions": attr.bool(default = False, doc = "rules_python internal use only"),
+        "register_all_versions": attr.bool(default = False, doc = "`rules_python` **internal** use only!"),
     },
 )
 
 _single_version_override = tag_class(
-    doc = """Override single python version settings.""",
+    doc = """Override single python version settings.
+
+:::{versionadded} 0.36.0
+:::
+""",
     attrs = {
+        "patch_strip": attr.int(
+            mandatory = False,
+            doc = "Same as the --strip argument of Unix patch.",
+            # TODO @aignas 2024-08-26: switch to 0 when 0.36.0 is released
+            default = 1,
+        ),
+        "patches": attr.string_list_dict(
+            mandatory = False,
+            doc = "A list of labels pointing to patch files to apply for this module as values with keys as values from the PLATFORMS dict. The patch files must exist in the source tree of the top level project. They are applied in the list order. If patches have a single key '*', then the patches will be applied to all available interpreters for that version.",
+        ),
         "sha256s": attr.string_dict(
             mandatory = False,
             doc = "The python platform to sha256 dict. The platform key must be present in the PLATFORMS dict.",
@@ -459,7 +476,6 @@ python = module_extension(
     implementation = _python_impl,
     tag_classes = {
         "override": _override,
-        "rules_python_private_testing": _rules_python_private_testing,
         "single_version_override": _single_version_override,
         "toolchain": _toolchain,
     },

python/private/python_repositories.bzl

@@ -47,8 +47,10 @@ def http_archive(**kwargs):
 def py_repositories():
     """Runtime dependencies that users must install.
 
-    This function should be loaded and called in the user's WORKSPACE.
-    With bzlmod enabled, this function is not needed since MODULE.bazel handles transitive deps.
+    This function should be loaded and called in the user's `WORKSPACE`.
+
+    With `bzlmod` enabled, this function is not needed since `MODULE.bazel`
+    handles transitive deps.
     """
     maybe(
         internal_config_repo,
@@ -178,8 +180,7 @@ def _python_repository_impl(rctx):
     patches = rctx.attr.patches
     if patches:
         for patch in patches:
-            # Should take the strip as an attr, but this is fine for the moment
-            rctx.patch(patch, strip = 1)
+            rctx.patch(patch, strip = rctx.attr.patch_strip)
 
     # Write distutils.cfg to the Python installation.
     if "windows" in platform:
@@ -450,6 +451,7 @@ py_exec_tools_toolchain(
         "ignore_root_user_error": rctx.attr.ignore_root_user_error,
         "name": rctx.attr.name,
         "netrc": rctx.attr.netrc,
+        "patch_strip": rctx.attr.patch_strip,
         "patches": rctx.attr.patches,
         "platform": platform,
         "python_version": python_version,
@@ -473,27 +475,11 @@ python_repository = repository_rule(
             doc = "Override mapping of hostnames to authorization patterns; mirrors the eponymous attribute from http_archive",
         ),
         "coverage_tool": attr.string(
-            # Mirrors the definition at
-            # https://github.com/bazelbuild/bazel/blob/master/src/main/starlark/builtins_bzl/common/python/py_runtime_rule.bzl
             doc = """
-This is a target to use for collecting code coverage information from `py_binary`
-and `py_test` targets.
-
-If set, the target must either produce a single file or be an executable target.
-The path to the single file, or the executable if the target is executable,
-determines the entry point for the python coverage tool.  The target and its
-runfiles will be added to the runfiles when coverage is enabled.
-
-The entry point for the tool must be loadable by a Python interpreter (e.g. a
-`.py` or `.pyc` file).  It must accept the command line arguments
-of coverage.py (https://coverage.readthedocs.io), at least including
-the `run` and `lcov` subcommands.
+This is a target to use for collecting code coverage information from {rule}`py_binary`
+and {rule}`py_test` targets.
 
-The target is accepted as a string by the python_repository and evaluated within
-the context of the toolchain repository.
-
-For more information see the official bazel docs
-(https://bazel.build/reference/be/python#py_runtime.coverage_tool).
+For more information see {attr}`py_runtime.coverage_tool`.
 """,
         ),
         "distutils": attr.label(
@@ -515,6 +501,12 @@ For more information see the official bazel docs
         "netrc": attr.string(
             doc = ".netrc file to use for authentication; mirrors the eponymous attribute from http_archive",
         ),
+        "patch_strip": attr.int(
+            doc = "Same as the --strip argument of Unix patch.",
+            # TODO @aignas 2024-08-26: switch to 0 when 0.36.0 is released
+            default = 1,
+            mandatory = False,
+        ),
         "patches": attr.label_list(
             doc = "A list of patch files to apply to the unpacked interpreter",
             mandatory = False,
@@ -568,32 +560,40 @@ def python_register_toolchains(
         register_toolchains = True,
         register_coverage_tool = False,
         set_python_version_constraint = False,
-        tool_versions = TOOL_VERSIONS,
+        tool_versions = None,
         **kwargs):
-    """Convenience macro for users which does typical setup.
+    """Convenience macro for users which does typical setup in `WORKSPACE`.
 
     - Create a repository for each built-in platform like "python_linux_amd64" -
       this repository is lazily fetched when Python is needed for that platform.
     - Create a repository exposing toolchains for each platform like
       "python_platforms".
     - Register a toolchain pointing at each platform.
+
     Users can avoid this macro and do these steps themselves, if they want more
     control.
+
+    With `bzlmod` enabled, this function is not needed since `rules_python` is
+    handling everything. In order to override the default behaviour from the
+    root module one can see the docs for the {rule}`python` extension.
+
     Args:
         name: base name for all created repos, like "python38".
         python_version: the Python version.
-        distutils: see the distutils attribute in the python_repository repository rule.
-        distutils_content: see the distutils_content attribute in the python_repository repository rule.
+        distutils: see the {attr}`python_repository.distutils`.
+        distutils_content: see the {attr}`python_repository.distutils_content`.
         register_toolchains: Whether or not to register the downloaded toolchains.
         register_coverage_tool: Whether or not to register the downloaded coverage tool to the toolchains.
             NOTE: Coverage support using the toolchain is only supported in Bazel 6 and higher.
 
         set_python_version_constraint: When set to true, target_compatible_with for the toolchains will include a version constraint.
         tool_versions: a dict containing a mapping of version with SHASUM and platform info. If not supplied, the defaults
             in python/versions.bzl will be used.
-        **kwargs: passed to each python_repositories call.
+        **kwargs: passed to each {rule}`python_repositories` call.
     """
 
+    tool_versions = tool_versions or TOOL_VERSIONS
+
     if BZLMOD_ENABLED:
         # you cannot used native.register_toolchains when using bzlmod.
         register_toolchains = False
@@ -626,7 +626,7 @@ def python_register_toolchains(
             continue
 
         loaded_platforms.append(platform)
-        (release_filename, urls, strip_prefix, patches) = get_release_info(platform, python_version, base_url, tool_versions)
+        (release_filename, urls, strip_prefix, patches, patch_strip) = get_release_info(platform, python_version, base_url, tool_versions)
 
         # allow passing in a tool version
         coverage_tool = None
@@ -651,6 +651,7 @@ def python_register_toolchains(
                 platform = platform,
             ),
             sha256 = sha256,
+            patches = patch_strip,
             patches = patches,
             platform = platform,
             python_version = python_version,
@@ -660,6 +661,10 @@ def python_register_toolchains(
             distutils_content = distutils_content,
             strip_prefix = strip_prefix,
             coverage_tool = coverage_tool,
+            # Will be one of
+            # * auth_patterns
+            # * ignore_root_user_error
+            # * netrc
             **kwargs
         )
         if register_toolchains:
@@ -713,7 +718,7 @@ def python_register_multi_toolchains(
         python_versions: the Python version.
         default_version: the default Python version. If not set, the first version in
             python_versions is used.
-        **kwargs: passed to each python_register_toolchains call.
+        **kwargs: passed to each {rule}`python_register_toolchains` call.
     """
     if len(python_versions) == 0:
         fail("python_versions must not be empty")

python/repositories.bzl

@@ -18,7 +18,6 @@
 load(
     "//python/private:python_repositories.bzl",
     _STANDALONE_INTERPRETER_FILENAME = "STANDALONE_INTERPRETER_FILENAME",
-    _http_archive = "http_archive",
     _is_standalone_interpreter = "is_standalone_interpreter",
     _py_repositories = "py_repositories",
     _python_register_multi_toolchains = "python_register_multi_toolchains",
@@ -33,6 +32,5 @@ python_register_toolchains = _python_register_toolchains
 # These symbols are of questionable public visibility. They were probably
 # not intended to be actually public.
 STANDALONE_INTERPRETER_FILENAME = _STANDALONE_INTERPRETER_FILENAME
-http_archive = _http_archive
 is_standalone_interpreter = _is_standalone_interpreter
 python_repository = _python_repository

python/versions.bzl

@@ -636,7 +636,7 @@ def get_release_info(platform, python_version, base_url = DEFAULT_RELEASE_BASE_U
         tool_versions: A dict listing the interpreter versions, their SHAs and URL
 
     Returns:
-        A tuple of (filename, url, and archive strip prefix)
+        A tuple of (filename, url, archive strip prefix, patches, patch_strip)
     """
 
     url = tool_versions[python_version]["url"]
@@ -669,12 +669,13 @@ def get_release_info(platform, python_version, base_url = DEFAULT_RELEASE_BASE_U
 
     patches = tool_versions[python_version].get("patches", [])
     if type(patches) == type({}):
-        if platform in patches.keys():
+        if platform in patches:
             patches = patches[platform]
         else:
             patches = []
+    patch_strip = tool_versions[python_version].get("patch_strip", None)
 
-    return (release_filename, rendered_urls, strip_prefix, patches)
+    return (release_filename, rendered_urls, strip_prefix, patches, patch_strip)
 
 def print_toolchains_checksums(name):
     native.genrule(