Skip to content

Commit

Permalink
refactor: don't load repo-phase objects from build-phase (bazelbuild#…
Browse files Browse the repository at this point in the history
…2056)

As a general practice, the repo-phase and build-phase shouldn't load
code from one
another because they can't use each other's objects. It can also result
in confusing
behavior because the "starlark environment" is slightly different
between the two phases.

Additionally, Google's version of Bazel essentially disables repo-phase
objects, so
loading e.g. http_archive results in errors. This makes it more
difficult to import
rules_python into Google, as we have to maintain patches to cut out the
code (and thus
we spend more time trying to import the code than working on it).
  • Loading branch information
rickeylev authored Jul 11, 2024
1 parent 04f5798 commit 03854a2
Show file tree
Hide file tree
Showing 4 changed files with 22 additions and 28 deletions.
5 changes: 1 addition & 4 deletions python/private/pypi/deps.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -123,10 +123,7 @@ py_library(
"""

# Collate all the repository names so they can be easily consumed
all_requirements = [name for (name, _, _) in _RULE_DEPS]

def requirement(pkg):
return Label("@pypi__" + pkg + "//:lib")
all_repo_names = [name for (name, _, _) in _RULE_DEPS]

def pypi_deps():
"""
Expand Down
27 changes: 13 additions & 14 deletions python/private/pypi/pip_compile.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,6 @@ make it possible to have multiple tools inside the `pypi` directory
"""

load("//python:defs.bzl", _py_binary = "py_binary", _py_test = "py_test")
load(":deps.bzl", "requirement")

def pip_compile(
name,
Expand Down Expand Up @@ -115,19 +114,19 @@ def pip_compile(
args.extend(extra_args)

deps = [
requirement("build"),
requirement("click"),
requirement("colorama"),
requirement("importlib_metadata"),
requirement("more_itertools"),
requirement("packaging"),
requirement("pep517"),
requirement("pip"),
requirement("pip_tools"),
requirement("pyproject_hooks"),
requirement("setuptools"),
requirement("tomli"),
requirement("zipp"),
Label("@pypi__build//:lib"),
Label("@pypi__click//:lib"),
Label("@pypi__colorama//:lib"),
Label("@pypi__importlib_metadata//:lib"),
Label("@pypi__more_itertools//:lib"),
Label("@pypi__packaging//:lib"),
Label("@pypi__pep517//:lib"),
Label("@pypi__pip//:lib"),
Label("@pypi__pip_tools//:lib"),
Label("@pypi__pyproject_hooks//:lib"),
Label("@pypi__setuptools//:lib"),
Label("@pypi__tomli//:lib"),
Label("@pypi__zipp//:lib"),
Label("//python/runfiles:runfiles"),
] + extra_deps

Expand Down
14 changes: 6 additions & 8 deletions python/private/pypi/whl_installer/BUILD.bazel
Original file line number Diff line number Diff line change
@@ -1,5 +1,4 @@
load("//python:defs.bzl", "py_binary", "py_library")
load("//python/private/pypi:deps.bzl", "requirement")

py_library(
name = "lib",
Expand All @@ -10,14 +9,13 @@ py_library(
"wheel_installer.py",
],
visibility = [
"//tests:__subpackages__",
"//third_party/rules_pycross/pycross/private:__subpackages__",
"//:__subpackages__",
],
deps = [
requirement("installer"),
requirement("pip"),
requirement("packaging"),
requirement("setuptools"),
"@pypi__installer//:lib",
"@pypi__packaging//:lib",
"@pypi__pip//:lib",
"@pypi__setuptools//:lib",
],
)

Expand All @@ -32,5 +30,5 @@ py_binary(
filegroup(
name = "distribution",
srcs = glob(["*"]),
visibility = ["//python/private/pypi:__subpackages__"],
visibility = ["//:__subpackages__"],
)
4 changes: 2 additions & 2 deletions python/private/pypi/whl_library.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ load("//python/private:envsubst.bzl", "envsubst")
load("//python/private:repo_utils.bzl", "REPO_DEBUG_ENV_VAR", "repo_utils")
load("//python/private:toolchains_repo.bzl", "get_host_os_arch")
load(":attrs.bzl", "ATTRS", "use_isolated")
load(":deps.bzl", "all_requirements")
load(":deps.bzl", "all_repo_names")
load(":generate_whl_library_build_bazel.bzl", "generate_whl_library_build_bazel")
load(":parse_whl_name.bzl", "parse_whl_name")
load(":patch_whl.bzl", "patch_whl")
Expand Down Expand Up @@ -490,7 +490,7 @@ attr makes `extra_pip_args` and `download_only` ignored.""",
] + [
# Includes all the external dependencies from repositories.bzl
Label("@" + repo + "//:BUILD.bazel")
for repo in all_requirements
for repo in all_repo_names
],
),
}, **ATTRS)
Expand Down

0 comments on commit 03854a2

Please sign in to comment.