fix(rules): remove rules_python --incompatible_python_disallow_native_rules checking (#2327)

rickeylev · rickeylev · commit 1eebc2ab2c3d · 2024-10-22T12:21:50.000-07:00
When --incompatible_python_disallow_native_rules is enabled, all the core rules fail with an error that rules_python should be used. This is incorrect, since the rules_python rules are being used. What's happening is #2257 removed the magic migration tag when pystar is enabled, but the code to check the tag was present wasn't removed. This went unnoticed because our CI doesn't set the migration flag. To fix, remove the validation logic entirely. If we're in the rules_python implementation, then there is not need to perform this validation. It was just something copy/pasted from the original code from Bazel itself. Also update the bazelrc to always set --incompatible_python_disallow_native_rules. Fixes #2326 Fixes #1645
diff --git a/.bazelci/presubmit.yml b/.bazelci/presubmit.yml
@@ -94,6 +94,8 @@ buildifier:
   bazel: "7.x"
   environment:
     RULES_PYTHON_ENABLE_PYSTAR: "1"
+  build_flags:
+    - "--config=bazel7.x"
   test_flags:
     # The doc check tests fail because the Starlark implementation makes the
     # PyInfo and PyRuntimeInfo symbols become documented.
diff --git a/.bazelrc b/.bazelrc
@@ -33,4 +33,6 @@ build:rtd --stamp
 # Some bzl files contain repos only available under bzlmod
 build:rtd --enable_bzlmod
 
+common:bazel7.x --incompatible_python_disallow_native_rules
+
 build --lockfile_mode=update
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -26,7 +26,9 @@ A brief description of the categories of changes:
 - Nothing yet
 
 ### Fixed
-- Nothing yet
+* (rules) Setting `--incompatible_python_disallow_native_rules` no longer
+  causes rules_python rules to fail.
+  ([#2326](https://github.com/bazelbuild/rules_python/issues/2326).
 
 ### Added
 - Nothing yet
diff --git a/examples/build_file_generation/.bazelrc b/examples/build_file_generation/.bazelrc
@@ -6,3 +6,5 @@ build --enable_runfiles
 # The bzlmod version of this example is in examples/bzlmod_build_file_generation
 # Once WORKSPACE support is dropped, this example can be entirely deleted.
 build --experimental_enable_bzlmod=false
+
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/examples/bzlmod/.bazelrc b/examples/bzlmod/.bazelrc
@@ -7,3 +7,4 @@ test --test_output=errors --enable_runfiles
 
 # Windows requires these for multi-python support:
 build --enable_runfiles
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/examples/bzlmod_build_file_generation/.bazelrc b/examples/bzlmod_build_file_generation/.bazelrc
@@ -6,3 +6,4 @@ build --enable_runfiles
 common --experimental_enable_bzlmod
 
 coverage --java_runtime_version=remotejdk_11
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/examples/multi_python_versions/.bazelrc b/examples/multi_python_versions/.bazelrc
@@ -4,3 +4,4 @@ test --test_output=errors
 build --enable_runfiles
 
 coverage --java_runtime_version=remotejdk_11
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/examples/pip_parse/.bazelrc b/examples/pip_parse/.bazelrc
@@ -1,2 +1,3 @@
 # https://docs.bazel.build/versions/main/best-practices.html#using-the-bazelrc-file
 try-import %workspace%/user.bazelrc
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/examples/pip_parse_vendored/.bazelrc b/examples/pip_parse_vendored/.bazelrc
@@ -6,3 +6,4 @@ build --enable_runfiles
 # Vendoring requirements.bzl files isn't necessary under bzlmod
 # When workspace support is dropped, this example can be removed.
 build --noexperimental_enable_bzlmod
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/examples/pip_repository_annotations/.bazelrc b/examples/pip_repository_annotations/.bazelrc
@@ -4,3 +4,4 @@ try-import %workspace%/user.bazelrc
 # This example is WORKSPACE specific. The equivalent functionality
 # is in examples/bzlmod as the `whl_mods` feature.
 build --experimental_enable_bzlmod=false
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/examples/py_proto_library/.bazelrc b/examples/py_proto_library/.bazelrc
@@ -1,2 +1,3 @@
 # The equivalent bzlmod behavior is covered by examples/bzlmod/py_proto_library
 common --noenable_bzlmod
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/gazelle/.bazelrc b/gazelle/.bazelrc
@@ -11,10 +11,4 @@ build --incompatible_default_to_explicit_init_py
 # Windows makes use of runfiles for some rules
 build --enable_runfiles
 
-# Do NOT implicitly create empty __init__.py files in the runfiles tree.
-# By default, these are created in every directory containing Python source code
-# or shared libraries, and every parent directory of those directories,
-# excluding the repo root directory. With this flag set, we are responsible for
-# creating (possibly empty) __init__.py files and adding them to the srcs of
-# Python targets as required.
-build --incompatible_default_to_explicit_init_py
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/python/private/common.bzl b/python/private/common.bzl
@@ -17,24 +17,15 @@ load(":cc_helper.bzl", "cc_helper")
 load(":py_info.bzl", "PyInfo", "PyInfoBuilder")
 load(":py_internal.bzl", "py_internal")
 load(":reexports.bzl", "BuiltinPyInfo")
-load(
-    ":semantics.bzl",
-    "NATIVE_RULES_MIGRATION_FIX_CMD",
-    "NATIVE_RULES_MIGRATION_HELP_URL",
-)
 
 _testing = testing
 _platform_common = platform_common
 _coverage_common = coverage_common
-_py_builtins = py_internal
 PackageSpecificationInfo = getattr(py_internal, "PackageSpecificationInfo", None)
 
 # Extensions without the dot
 _PYTHON_SOURCE_EXTENSIONS = ["py"]
 
-# NOTE: Must stay in sync with the value used in rules_python
-_MIGRATION_TAG = "__PYTHON_RULES_MIGRATION_DO_NOT_USE_WILL_BREAK__"
-
 def create_binary_semantics_struct(
         *,
         create_executable,
@@ -477,65 +468,3 @@ def target_platform_has_any_constraint(ctx, constraints):
         if ctx.target_platform_has_constraint(constraint_value):
             return True
     return False
-
-def check_native_allowed(ctx):
-    """Check if the usage of the native rule is allowed.
-
-    Args:
-        ctx: rule context to check
-    """
-    if not ctx.fragments.py.disallow_native_rules:
-        return
-
-    if _MIGRATION_TAG in ctx.attr.tags:
-        return
-
-    # NOTE: The main repo name is empty in *labels*, but not in
-    # ctx.workspace_name
-    is_main_repo = not bool(ctx.label.workspace_name)
-    if is_main_repo:
-        check_label = ctx.label
-    else:
-        # 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(
-            workspace = ctx.label.workspace_name,
-            package = ctx.label.package,
-        ))
-    allowlist = ctx.attr._native_rules_allowlist
-    if allowlist:
-        allowed = ctx.attr._native_rules_allowlist[PackageSpecificationInfo].contains(check_label)
-        allowlist_help = str(allowlist.label).replace("@//", "//")
-    else:
-        allowed = False
-        allowlist_help = ("no allowlist specified; all disallowed; specify one " +
-                          "with --python_native_rules_allowlist")
-    if not allowed:
-        if ctx.attr.generator_function:
-            generator = "{generator_function}(name={generator_name}) in {generator_location}".format(
-                generator_function = ctx.attr.generator_function,
-                generator_name = ctx.attr.generator_name,
-                generator_location = ctx.attr.generator_location,
-            )
-        else:
-            generator = "No generator (called directly in BUILD file)"
-
-        msg = (
-            "{target} not allowed to use native.{rule}\n" +
-            "Generated by: {generator}\n" +
-            "Allowlist: {allowlist}\n" +
-            "Migrate to using @rules_python, see {help_url}\n" +
-            "FIXCMD: {fix_cmd} --target={target} --rule={rule} " +
-            "--generator_name={generator_name} --location={generator_location}"
-        )
-        fail(msg.format(
-            target = str(ctx.label).replace("@//", "//"),
-            rule = _py_builtins.get_rule_name(ctx),
-            generator = generator,
-            allowlist = allowlist_help,
-            generator_name = ctx.attr.generator_name,
-            generator_location = ctx.attr.generator_location,
-            help_url = NATIVE_RULES_MIGRATION_HELP_URL,
-            fix_cmd = NATIVE_RULES_MIGRATION_FIX_CMD,
-        ))
diff --git a/python/private/py_executable.bzl b/python/private/py_executable.bzl
@@ -33,7 +33,6 @@ load(":builders.bzl", "builders")
 load(":cc_helper.bzl", "cc_helper")
 load(
     ":common.bzl",
-    "check_native_allowed",
     "collect_imports",
     "collect_runfiles",
     "create_instrumented_files_info",
@@ -269,7 +268,6 @@ def _get_build_info(ctx, cc_toolchain):
 def _validate_executable(ctx):
     if ctx.attr.python_version != "PY3":
         fail("It is not allowed to use Python 2")
-    check_native_allowed(ctx)
 
 def _declare_executable_file(ctx):
     if target_platform_has_any_constraint(ctx, ctx.attr._windows_constraints):
diff --git a/python/private/py_library.bzl b/python/private/py_library.bzl
@@ -28,7 +28,6 @@ load(
 load(":builders.bzl", "builders")
 load(
     ":common.bzl",
-    "check_native_allowed",
     "collect_imports",
     "collect_runfiles",
     "create_instrumented_files_info",
@@ -70,7 +69,6 @@ def py_library_impl(ctx, *, semantics):
     Returns:
         A list of modern providers to propagate.
     """
-    check_native_allowed(ctx)
     direct_sources = filter_to_py_srcs(ctx.files.srcs)
 
     precompile_result = semantics.maybe_precompile(ctx, direct_sources)
diff --git a/tests/integration/compile_pip_requirements/.bazelrc b/tests/integration/compile_pip_requirements/.bazelrc
@@ -2,3 +2,4 @@ test --test_output=errors
 
 # Windows requires these for multi-python support:
 build --enable_runfiles
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/tests/integration/compile_pip_requirements_test_from_external_repo/.bazelrc b/tests/integration/compile_pip_requirements_test_from_external_repo/.bazelrc
@@ -1 +1,2 @@
 test --test_output=errors
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/tests/integration/ignore_root_user_error/.bazelrc b/tests/integration/ignore_root_user_error/.bazelrc
@@ -4,3 +4,4 @@ test --test_output=errors
 
 # Windows requires these for multi-python support:
 build --enable_runfiles
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/tests/integration/local_toolchains/.bazelrc b/tests/integration/local_toolchains/.bazelrc
@@ -3,3 +3,4 @@ common --lockfile_mode=off
 test --test_output=errors
 # Windows requires these for multi-python support:
 build --enable_runfiles
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/tests/integration/pip_parse/.bazelrc b/tests/integration/pip_parse/.bazelrc
@@ -5,3 +5,4 @@ build --enable_runfiles
 # https://docs.bazel.build/versions/main/best-practices.html#using-the-bazelrc-file
 try-import %workspace%/user.bazelrc
 
+common:bazel7.x --incompatible_python_disallow_native_rules
diff --git a/tests/integration/py_cc_toolchain_registered/.bazelrc b/tests/integration/py_cc_toolchain_registered/.bazelrc
@@ -1,2 +1,3 @@
 # This aids debugging on failure
 build --toolchain_resolution_debug=python
+common:bazel7.x --incompatible_python_disallow_native_rules

Original file line number	Diff line number	Diff line change
`@@ -7,3 +7,4 @@ test --test_output=errors --enable_runfiles`
`7`	`7`
`8`	`8`	`# Windows requires these for multi-python support:`
`9`	`9`	`build --enable_runfiles`
	`10`	`+common:bazel7.x --incompatible_python_disallow_native_rules`
Original file line number	Diff line number	Diff line change
`@@ -6,3 +6,4 @@ build --enable_runfiles`
`6`	`6`	`common --experimental_enable_bzlmod`
`7`	`7`
`8`	`8`	`coverage --java_runtime_version=remotejdk_11`
	`9`	`+common:bazel7.x --incompatible_python_disallow_native_rules`
Original file line number	Diff line number	Diff line change
`@@ -4,3 +4,4 @@ test --test_output=errors`
`4`	`4`	`build --enable_runfiles`
`5`	`5`
`6`	`6`	`coverage --java_runtime_version=remotejdk_11`
	`7`	`+common:bazel7.x --incompatible_python_disallow_native_rules`
Original file line number	Diff line number	Diff line change
`@@ -1,2 +1,3 @@`
`1`	`1`	`# https://docs.bazel.build/versions/main/best-practices.html#using-the-bazelrc-file`
`2`	`2`	`try-import %workspace%/user.bazelrc`
	`3`	`+common:bazel7.x --incompatible_python_disallow_native_rules`