python: make incompatible_python_disallow_native_rules work for top-level external repo targets

rickeylev · copybara-github · commit 1533cd123fa4 · 2023-12-14T14:33:08.000-08:00
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 #17773 Closes #20516 PiperOrigin-RevId: 591053422 Change-Id: I7790df2db10278844ae2b36cfe671de03164972f
diff --git a/src/main/starlark/builtins_bzl/common/python/common.bzl b/src/main/starlark/builtins_bzl/common/python/common.bzl
@@ -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:
diff --git a/src/test/shell/bazel/python_version_test.sh b/src/test/shell/bazel/python_version_test.sh
@@ -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"
