Skip to content

Commit

Permalink
python: make incompatible_python_disallow_native_rules work for top-l…
Browse files Browse the repository at this point in the history
…evel external repo targets

This basically makes it usable with the downloaded runtimes rules_python makes
available, which reference their runtimes as e.g. `@python_3_11//:python`.

The issue was the code to construct the label was leaving a trailing "/" when
the the target being checked at the root of the workspace. To fix, just omit
the trailing slash when the package name is empty to prevent the trailing
slash.

Work towards bazelbuild#17773

Closes bazelbuild#20516

PiperOrigin-RevId: 591053422
Change-Id: I7790df2db10278844ae2b36cfe671de03164972f
  • Loading branch information
rickeylev authored and bazel-io committed Jan 17, 2024
1 parent 86a5f6b commit a6f9619
Show file tree
Hide file tree
Showing 2 changed files with 87 additions and 2 deletions.
5 changes: 3 additions & 2 deletions src/main/starlark/builtins_bzl/common/python/common.bzl
Original file line number Diff line number Diff line change
Expand Up @@ -486,9 +486,10 @@ def check_native_allowed(ctx):
# package_group doesn't allow @repo syntax, so we work around that
# by prefixing external repos with a fake package path. This also
# makes it easy to enable or disable all external repos.
check_label = Label("@//__EXTERNAL_REPOS__/{workspace}/{package}".format(
check_label = Label("@//__EXTERNAL_REPOS__/{workspace}{package}".format(
workspace = ctx.label.workspace_name,
package = ctx.label.package,
# Prevent a label with trailing slash, which is malformed.
package = "/" + ctx.label.package if ctx.label.package else "",
))
allowlist = ctx.attr._native_rules_allowlist
if allowlist:
Expand Down
84 changes: 84 additions & 0 deletions src/test/shell/bazel/python_version_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -449,4 +449,88 @@ EOF
fi
}

function test_incompatible_python_disallow_native_rules_external_repos() {

mkdir ../external_repo
external_repo=$(cd ../external_repo && pwd)

touch $external_repo/WORKSPACE
touch $external_repo/WORKSPACE.bzlmod
cat > $external_repo/MODULE.bazel <<EOF
module(name="external_repo")
EOF

# There's special logic to handle targets at the root.
cat > $external_repo/BUILD <<EOF
py_library(
name = "root",
visibility = ["//visibility:public"],
)
EOF
mkdir $external_repo/pkg
cat > $external_repo/pkg/BUILD <<EOF
py_library(
name = "pkg",
visibility = ["//visibility:public"],
)
EOF

touch bin.py
cat > BUILD <<EOF
load("@rules_python//python:py_binary.bzl", "py_binary")
load("@rules_python//python:py_runtime.bzl", "py_runtime")
load("@rules_python//python:py_runtime_pair.bzl", "py_runtime_pair")
py_binary(
name = "bin",
srcs = ["bin.py"],
deps = [
"@external_repo//:root",
"@external_repo//pkg:pkg",
],
)
py_runtime(
name = "runtime",
interpreter_path = "/fakepython",
python_version = "PY3",
)
py_runtime_pair(
name = "pair",
py3_runtime = ":runtime",
)
# A custom toolchain is used so this test is independent of
# custom python toolchain setup the integration test performs
toolchain(
name = "py_toolchain",
toolchain = ":pair",
toolchain_type = "@rules_python//python:toolchain_type",
)
package_group(
name = "allowed",
packages = [
"//__EXTERNAL_REPOS__/external_repo/...",
"//__EXTERNAL_REPOS__/external_repo~override/...",
"//__EXTERNAL_REPOS__/bazel_tools/...",
##"//tools/python/windows...",
],
)
EOF
cat > MODULE.bazel <<EOF
module(name="python_version_test")
bazel_dep(name = "external_repo", version="0.0.0")
local_path_override(
module_name = "external_repo",
# A relative path must be used to keep Windows CI happy.
path = "../external_repo",
)
EOF

bazel build \
--extra_toolchains=//:py_toolchain \
--incompatible_python_disallow_native_rules \
--python_native_rules_allowlist=//:allowed \
//:bin &> $TEST_log || fail "build failed"

}

run_suite "Tests for how the Python rules handle Python 2 vs Python 3"

0 comments on commit a6f9619

Please sign in to comment.