From e5240a676828846435051981f35ddde919f6cd90 Mon Sep 17 00:00:00 2001
From: Richard Levasseur <rlevasseur@google.com>
Date: Tue, 12 Dec 2023 13:45:03 -0800
Subject: [PATCH] python: make disallow native rules to work with external
 repos with empty package

This basically makes it usable with the downloaded runtimes rules_python
makes available. 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 https://github.com/bazelbuild/bazel/issues/17773
---
 .../builtins_bzl/common/python/common.bzl     |  5 +-
 src/test/shell/bazel/python_version_test.sh   | 60 +++++++++++++++++++
 2 files changed, 63 insertions(+), 2 deletions(-)

diff --git a/src/main/starlark/builtins_bzl/common/python/common.bzl b/src/main/starlark/builtins_bzl/common/python/common.bzl
index 97ed3e3ee6e0f7..8a4ad4b717b6cd 100644
--- 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
index 785c966903c8a3..0715f2cbdab075 100755
--- a/src/test/shell/bazel/python_version_test.sh
+++ b/src/test/shell/bazel/python_version_test.sh
@@ -449,4 +449,64 @@ EOF
   fi
 }
 
+function test_incompatible_python_disallow_native_rules_external_repos() {
+
+  mkdir external_repo
+  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")
+py_binary(
+    name = "bin",
+    srcs = ["bin.py"],
+    deps = [
+        "@external_repo//:root",
+        "@external_repo//pkg:pkg",
+    ],
+)
+package_group(
+    name = "allowed",
+    packages = [
+        "//__EXTERNAL_REPOS__/external_repo/...",
+        "//__EXTERNAL_REPOS__/external_repo~override/...",
+        "//__EXTERNAL_REPOS__/bazel_tools/...",
+    ],
+)
+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",
+    path = "$PWD/external_repo",
+)
+EOF
+
+  bazel build \
+    --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"