From 6c9aaca5cac12cd7d0e6dcf0f8498663da7c3a33 Mon Sep 17 00:00:00 2001 From: Sahin Yort Date: Tue, 2 Apr 2024 15:15:17 -0700 Subject: [PATCH] refactor: split toolchains and repositories (#323) ### Type of change Currently, rules_py_dependencies and rules_py_toolchains are exported from the same bzl file which makes it impossible to depend on another rule and load from at the same since both macros are in the same file. This changes this to allow adding a dependency and loading from it. - Refactor (a code change that neither fixes a bug or adds a new feature) **For changes visible to end-users** - Breaking change (this change will force users to change their own code or config) - Relevant documentation has been updated ### Test plan - Covered by existing test cases --- WORKSPACE | 4 +++ e2e/smoke/WORKSPACE.bazel | 4 +++ e2e/use_release/WORKSPACE.bazel | 6 +++-- py/extensions.bzl | 2 +- py/repositories.bzl | 43 +++------------------------------ py/toolchains.bzl | 34 ++++++++++++++++++++++++++ 6 files changed, 50 insertions(+), 43 deletions(-) create mode 100644 py/toolchains.bzl diff --git a/WORKSPACE b/WORKSPACE index 7947d1d2..6f93e64e 100644 --- a/WORKSPACE +++ b/WORKSPACE @@ -15,6 +15,10 @@ load("//py:repositories.bzl", "rules_py_dependencies") # Fetch dependencies which users need as well rules_py_dependencies() +load("//py:toolchains.bzl", "rules_py_toolchains") + +rules_py_toolchains() + # Load the Python toolchain for rules_docker register_toolchains("//:container_py_toolchain") diff --git a/e2e/smoke/WORKSPACE.bazel b/e2e/smoke/WORKSPACE.bazel index 70e3a8f7..0a1c26b7 100644 --- a/e2e/smoke/WORKSPACE.bazel +++ b/e2e/smoke/WORKSPACE.bazel @@ -56,6 +56,10 @@ load("@aspect_rules_py//py:repositories.bzl", "rules_py_dependencies") rules_py_dependencies() +load("@aspect_rules_py//py:toolchains.bzl", "rules_py_toolchains") + +rules_py_toolchains() + # "Installation" for rules_python load("@rules_python//python:repositories.bzl", "py_repositories", "python_register_toolchains") diff --git a/e2e/use_release/WORKSPACE.bazel b/e2e/use_release/WORKSPACE.bazel index 99bf0460..cd3c4484 100644 --- a/e2e/use_release/WORKSPACE.bazel +++ b/e2e/use_release/WORKSPACE.bazel @@ -9,9 +9,11 @@ local_repository( # you should fetch it *before* calling this. # Alternatively, you can skip calling this function, so long as you've # already fetched all the dependencies. -load("@aspect_rules_py//py:repositories.bzl", "rules_py_dependencies", "rules_py_toolchains") +load("@aspect_rules_py//py:repositories.bzl", "rules_py_dependencies") -rules_py_dependencies(register_toolchains = False) +rules_py_dependencies() + +load("@aspect_rules_py//py:toolchains.bzl", "rules_py_toolchains") # Force use of pre-built release binaries regardless of the content of /tools/version.bzl rules_py_toolchains(is_prerelease = False) diff --git a/py/extensions.bzl b/py/extensions.bzl index 0aca5b81..6ac14836 100644 --- a/py/extensions.bzl +++ b/py/extensions.bzl @@ -1,6 +1,6 @@ "Module Extensions used from MODULE.bazel" -load(":repositories.bzl", "DEFAULT_TOOLS_REPOSITORY", "rules_py_toolchains") +load(":toolchains.bzl", "DEFAULT_TOOLS_REPOSITORY", "rules_py_toolchains") load("//tools:version.bzl", "IS_PRERELEASE") py_toolchain = tag_class(attrs = { diff --git a/py/repositories.bzl b/py/repositories.bzl index 0ca0f651..17d321f4 100644 --- a/py/repositories.bzl +++ b/py/repositories.bzl @@ -6,17 +6,10 @@ See https://docs.bazel.build/versions/main/skylark/deploying.html#dependencies load("@bazel_tools//tools/build_defs/repo:http.bzl", _http_archive = "http_archive") load("@bazel_tools//tools/build_defs/repo:utils.bzl", "maybe") -load("//py/private/toolchain:autodetecting.bzl", _register_autodetecting_python_toolchain = "register_autodetecting_python_toolchain") -load("//py/private/toolchain:tools.bzl", "TOOLCHAIN_PLATFORMS", "prebuilt_tool_repo") -load("//py/private/toolchain:repo.bzl", "prerelease_toolchains_repo", "toolchains_repo") -load("//tools:version.bzl", "IS_PRERELEASE") def http_archive(name, **kwargs): maybe(_http_archive, name = name, **kwargs) -register_autodetecting_python_toolchain = _register_autodetecting_python_toolchain - -DEFAULT_TOOLS_REPOSITORY = "rules_py_tools" # WARNING: any changes in this function may be BREAKING CHANGES for users # because we'll fetch a dependency which may be different from one that @@ -26,12 +19,8 @@ DEFAULT_TOOLS_REPOSITORY = "rules_py_tools" # and released only in semver majors. # buildifier: disable=unnamed-macro -def rules_py_dependencies(register_toolchains = True): - """Fetch rules_py's dependencies - - Args: - register_toolchains: whether to also do default toolchain registration - """ +def rules_py_dependencies(): + """Fetch rules_py's dependencies""" # The minimal version of bazel_skylib we require http_archive( @@ -53,30 +42,4 @@ def rules_py_dependencies(register_toolchains = True): sha256 = "c68bdc4fbec25de5b5493b8819cfc877c4ea299c0dcb15c244c5a00208cde311", strip_prefix = "rules_python-0.31.0", url = "https://github.com/bazelbuild/rules_python/releases/download/0.31.0/rules_python-0.31.0.tar.gz", - ) - - if register_toolchains: - rules_py_toolchains() - -def rules_py_toolchains(name = DEFAULT_TOOLS_REPOSITORY, register = True, is_prerelease = IS_PRERELEASE): - """Create a downloaded toolchain for every tool under every supported platform. - - Args: - name: prefix used in created repositories - register: whether to call the register_toolchains, should be True for WORKSPACE and False for bzlmod. - is_prerelease: True iff there are no pre-built tool binaries for this version of rules_py - """ - if is_prerelease: - prerelease_toolchains_repo(name = name) - if register: - native.register_toolchains( - "@aspect_rules_py//py/private/toolchain/venv/...", - "@aspect_rules_py//py/private/toolchain/unpack/...", - ) - else: - for platform in TOOLCHAIN_PLATFORMS.keys(): - prebuilt_tool_repo(name = ".".join([name, platform]), platform = platform) - toolchains_repo(name = name, user_repository_name = name) - - if register: - native.register_toolchains("@{}//:all".format(name)) + ) \ No newline at end of file diff --git a/py/toolchains.bzl b/py/toolchains.bzl new file mode 100644 index 00000000..b68a400a --- /dev/null +++ b/py/toolchains.bzl @@ -0,0 +1,34 @@ +"""Declare toolchains""" + +load("//py/private/toolchain:autodetecting.bzl", _register_autodetecting_python_toolchain = "register_autodetecting_python_toolchain") +load("//py/private/toolchain:repo.bzl", "prerelease_toolchains_repo", "toolchains_repo") +load("//py/private/toolchain:tools.bzl", "TOOLCHAIN_PLATFORMS", "prebuilt_tool_repo") +load("//tools:version.bzl", "IS_PRERELEASE") + + +register_autodetecting_python_toolchain = _register_autodetecting_python_toolchain + +DEFAULT_TOOLS_REPOSITORY = "rules_py_tools" + +def rules_py_toolchains(name = DEFAULT_TOOLS_REPOSITORY, register = True, is_prerelease = IS_PRERELEASE): + """Create a downloaded toolchain for every tool under every supported platform. + + Args: + name: prefix used in created repositories + register: whether to call the register_toolchains, should be True for WORKSPACE and False for bzlmod. + is_prerelease: True iff there are no pre-built tool binaries for this version of rules_py + """ + if is_prerelease: + prerelease_toolchains_repo(name = name) + if register: + native.register_toolchains( + "@aspect_rules_py//py/private/toolchain/venv/...", + "@aspect_rules_py//py/private/toolchain/unpack/...", + ) + else: + for platform in TOOLCHAIN_PLATFORMS.keys(): + prebuilt_tool_repo(name = ".".join([name, platform]), platform = platform) + toolchains_repo(name = name, user_repository_name = name) + + if register: + native.register_toolchains("@{}//:all".format(name))