python: add --incompatible_python_disallow_native_rules

The `--incompatible_python_disallow_native_rules` flag, when enabled,
will cause an error if a native Python rule is used without going through
the @rule_python rule set.

To aid incremental migration, an allowlist can be specified using
`--python_native_rules_allowlist`. When specified, packages in the allowlist
won't fail when directly using the native rules.

Work towards #17773

PiperOrigin-RevId: 516687379
Change-Id: I154b538a9c7956bc3b2fba9e619fe63207559598

src/main/java/com/google/devtools/build/lib/rules/python/BUILD

@@ -21,33 +21,29 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib:build-request-options",
         "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/actions:artifacts",
-        "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
         "//src/main/java/com/google/devtools/build/lib/analysis:actions/abstract_file_write_action",
         "//src/main/java/com/google/devtools/build/lib/analysis:actions/deterministic_writer",
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_configuration",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options_cache",
-        "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/core_option_converters",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/configuration_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/no_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/patch_transition",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/transitions/transition_factory",
-        "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",
         "//src/main/java/com/google/devtools/build/lib/analysis:file_provider",
         "//src/main/java/com/google/devtools/build/lib/analysis:rule_definition_environment",
-        "//src/main/java/com/google/devtools/build/lib/analysis:test/execution_info",
         "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_collection",
         "//src/main/java/com/google/devtools/build/lib/analysis:transitive_info_provider",
-        "//src/main/java/com/google/devtools/build/lib/analysis/platform",
+        "//src/main/java/com/google/devtools/build/lib/analysis/starlark/annotations",
         "//src/main/java/com/google/devtools/build/lib/cmdline",
         "//src/main/java/com/google/devtools/build/lib/collect/nestedset",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/packages",
-        "//src/main/java/com/google/devtools/build/lib/rules/apple",
         "//src/main/java/com/google/devtools/build/lib/rules/cpp",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec",
         "//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
@@ -63,7 +59,6 @@ java_library(
         "//src/main/java/net/starlark/java/eval",
         "//src/main/java/net/starlark/java/syntax",
         "//src/main/protobuf:extra_actions_base_java_proto",
-        "//src/main/protobuf:failure_details_java_proto",
         "//third_party:error_prone_annotations",
         "//third_party:guava",
         "//third_party:jsr305",

src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java

@@ -119,6 +119,16 @@ public boolean getLegacyExternalRunfiles(StarlarkRuleContext starlarkCtx) throws
     return starlarkCtx.getConfiguration().legacyExternalRunfiles();
   }
 
+  @StarlarkMethod(
+      name = "get_rule_name",
+      doc = "Get the name of the rule for the given ctx",
+      parameters = {
+        @Param(name = "ctx", positional = true, named = true, defaultValue = "unbound")
+      })
+  public String getRuleName(StarlarkRuleContext starlarkCtx) throws EvalException {
+    return starlarkCtx.getRuleContext().getRule().getRuleClass();
+  }
+
   @StarlarkMethod(
       name = "get_current_os_name",
       doc = "Get the name of the OS Bazel itself is running on.",

src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java

@@ -20,6 +20,8 @@
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.Fragment;
 import com.google.devtools.build.lib.analysis.config.RequiresOptions;
+import com.google.devtools.build.lib.analysis.starlark.annotations.StarlarkConfigurationField;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.util.OS;
 import com.google.devtools.common.options.TriState;
@@ -53,6 +55,8 @@ public class PythonConfiguration extends Fragment implements StarlarkValue {
 
   private final boolean defaultToExplicitInitPy;
   private final boolean disablePy2;
+  @Nullable private final Label nativeRulesAllowlist;
+  private final boolean disallowNativeRules;
 
   public PythonConfiguration(BuildOptions buildOptions) {
     PythonOptions pythonOptions = buildOptions.get(PythonOptions.class);
@@ -66,6 +70,8 @@ public PythonConfiguration(BuildOptions buildOptions) {
     this.useToolchains = pythonOptions.incompatibleUsePythonToolchains;
     this.defaultToExplicitInitPy = pythonOptions.incompatibleDefaultToExplicitInitPy;
     this.disablePy2 = pythonOptions.disablePy2;
+    this.nativeRulesAllowlist = pythonOptions.nativeRulesAllowlist;
+    this.disallowNativeRules = pythonOptions.disallowNativeRules;
   }
 
   @Override
@@ -182,4 +188,20 @@ public boolean defaultToExplicitInitPy() {
   public boolean getDisablePy2() {
     return disablePy2;
   }
+
+  @StarlarkMethod(
+      name = "disallow_native_rules",
+      structField = true,
+      doc = "The value of the --incompatible_python_disallow_native_rules flag.")
+  public boolean getDisallowNativeRules() {
+    return disallowNativeRules;
+  }
+
+  @StarlarkConfigurationField(
+      name = "native_rules_allowlist",
+      doc = "The value of --python_native_rules_allowlist; may be None if not specified")
+  @Nullable
+  public Label getNativeRulesAllowlist() {
+    return nativeRulesAllowlist;
+  }
 }

src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java

@@ -17,7 +17,9 @@
 import com.google.common.base.Ascii;
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.analysis.config.CoreOptionConverters.LabelConverter;
 import com.google.devtools.build.lib.analysis.config.FragmentOptions;
+import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.common.options.Converter;
 import com.google.devtools.common.options.Option;
 import com.google.devtools.common.options.OptionDefinition;
@@ -260,6 +262,29 @@ public String getTypeDescription() {
               + "https://github.com/bazelbuild/bazel/issues/10076.")
   public boolean incompatibleDefaultToExplicitInitPy;
 
+  @Option(
+      name = "python_native_rules_allowlist",
+      documentationCategory = OptionDocumentationCategory.INPUT_STRICTNESS,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      defaultValue = "null",
+      converter = LabelConverter.class,
+      help =
+          "An allowlist (package_group target) to use when enforcing "
+              + "--incompatible_python_disallow_native_rules.")
+  public Label nativeRulesAllowlist;
+
+  @Option(
+      name = "incompatible_python_disallow_native_rules",
+      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      defaultValue = "false",
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help =
+          "When true, an error occurs when using the builtin py_* rules; instead the rule_python"
+              + " rules should be used. See https://github.com/bazelbuild/bazel/issues/17773 for"
+              + " more information and migration instructions.")
+  public boolean disallowNativeRules;
+
   // Helper field to store hostForcePython in exec configuration
   private PythonVersion defaultPythonVersion = null;
 
@@ -276,12 +301,12 @@ public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
     restrictions.put(
         FORCE_PYTHON_DEFINITION,
         new SelectRestriction(
-            /*visibleWithinToolsPackage=*/ true,
+            /* visibleWithinToolsPackage= */ true,
             "Use @bazel_tools//python/tools:python_version instead."));
     restrictions.put(
         HOST_FORCE_PYTHON_DEFINITION,
         new SelectRestriction(
-            /*visibleWithinToolsPackage=*/ false,
+            /* visibleWithinToolsPackage= */ false,
             "Use @bazel_tools//python/tools:python_version instead."));
     return restrictions.buildOrThrow();
   }
@@ -355,6 +380,8 @@ public FragmentOptions getExec() {
     hostPythonOptions.incompatibleDisallowLegacyPyProvider = incompatibleDisallowLegacyPyProvider;
     hostPythonOptions.incompatibleRemoveOldPythonVersionApi = incompatibleRemoveOldPythonVersionApi;
     hostPythonOptions.disablePy2 = disablePy2;
+    hostPythonOptions.nativeRulesAllowlist = nativeRulesAllowlist;
+    hostPythonOptions.disallowNativeRules = disallowNativeRules;
 
     // Save host options in case of a further exec->host transition.
     hostPythonOptions.hostForcePython = hostForcePython;

src/main/starlark/builtins_bzl/common/python/attributes.bzl

@@ -81,9 +81,20 @@ DATA_ATTRS = {
     ),
 }
 
+NATIVE_RULES_ALLOWLIST_ATTRS = {
+    "_native_rules_allowlist": attr.label(
+        default = configuration_field(
+            fragment = "py",
+            name = "native_rules_allowlist",
+        ),
+        providers = ["PackageSpecificationProvider"],
+    ),
+}
+
 # Attributes common to all rules.
 COMMON_ATTRS = union_attrs(
     DATA_ATTRS,
+    NATIVE_RULES_ALLOWLIST_ATTRS,
     {
         # TODO(b/148103851): This attribute is deprecated and slated for
         # removal.

src/main/starlark/builtins_bzl/common/python/common.bzl

@@ -18,17 +18,26 @@ load(
     ":common/python/providers.bzl",
     "PyInfo",
 )
-load(":common/python/semantics.bzl", "TOOLS_REPO")
+load(
+    ":common/python/semantics.bzl",
+    "NATIVE_RULES_MIGRATION_FIX_CMD",
+    "NATIVE_RULES_MIGRATION_HELP_URL",
+    "TOOLS_REPO",
+)
 
 _testing = _builtins.toplevel.testing
 _platform_common = _builtins.toplevel.platform_common
 _coverage_common = _builtins.toplevel.coverage_common
+_py_builtins = _builtins.internal.py_builtins
 
 TOOLCHAIN_TYPE = "@" + TOOLS_REPO + "//tools/python:toolchain_type"
 
 # 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,
@@ -454,3 +463,56 @@ 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.isAvailableFor(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:
+        msg = (
+            "{target} not allowed to use native.{rule}\n" +
+            "Generated by: {generator_function}(name={generator_name}) in {generator_location}\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),
+            allowlist = allowlist_help,
+            generator_function = ctx.attr.generator_function,
+            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,
+        ))

src/main/starlark/builtins_bzl/common/python/py_executable.bzl

@@ -17,6 +17,7 @@ load(":common/cc/cc_helper.bzl", "cc_helper")
 load(
     ":common/python/common.bzl",
     "TOOLCHAIN_TYPE",
+    "check_native_allowed",
     "collect_imports",
     "collect_runfiles",
     "create_instrumented_files_info",
@@ -184,6 +185,7 @@ def py_executable_base_impl(ctx, *, semantics, is_test, inherited_environment =
 def _validate_executable(ctx):
     if ctx.attr.python_version != "PY3":
         fail("It is not allowed to use Python 2")
+    check_native_allowed(ctx)
 
 def _compute_outputs(ctx, output_sources):
     # TODO: This should use the configuration instead of the Bazel OS.

src/main/starlark/builtins_bzl/common/python/py_internal.bzl

@@ -20,6 +20,9 @@ These may change at any time and are closely coupled to the rule implementation.
 
 _py_builtins = _builtins.internal.py_builtins
 
+def _is_available_for(package_group_target, label):
+    return package_group_target.isAvailableFor(label)
+
 # This replaces the Java-defined name using exports.bzl toplevels mapping.
 py_internal = struct(
     add_py_extra_pseudo_action = _py_builtins.add_py_extra_pseudo_action,
@@ -31,8 +34,10 @@ py_internal = struct(
     get_current_os_name = _py_builtins.get_current_os_name,
     get_label_repo_runfiles_path = _py_builtins.get_label_repo_runfiles_path,
     get_legacy_exernal_runfiles = _py_builtins.get_legacy_external_runfiles,
+    get_rule_name = _py_builtins.get_rule_name,
+    is_available_for = _is_available_for,
     is_singleton_depset = _py_builtins.is_singleton_depset,
+    make_runfiles_respect_legacy_external_runfiles = _py_builtins.make_runfiles_respect_legacy_external_runfiles,
     merge_runfiles_with_generated_inits_empty_files_supplier = _py_builtins.merge_runfiles_with_generated_inits_empty_files_supplier,
     new_py_cc_link_params_provider = _py_builtins.new_py_cc_link_params_provider,
-    make_runfiles_respect_legacy_external_runfiles = _py_builtins.make_runfiles_respect_legacy_external_runfiles,
 )

src/main/starlark/builtins_bzl/common/python/py_library.bzl

@@ -23,6 +23,7 @@ load(
 )
 load(
     ":common/python/common.bzl",
+    "check_native_allowed",
     "collect_imports",
     "collect_runfiles",
     "create_instrumented_files_info",
@@ -51,6 +52,7 @@ 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)
     output_sources = depset(semantics.maybe_precompile(ctx, direct_sources))
     runfiles = collect_runfiles(ctx = ctx, files = output_sources)

src/main/starlark/builtins_bzl/common/python/py_runtime_rule.bzl

@@ -15,10 +15,13 @@
 
 load(":common/paths.bzl", "paths")
 load(":common/python/providers.bzl", "DEFAULT_BOOTSTRAP_TEMPLATE", "DEFAULT_STUB_SHEBANG", _PyRuntimeInfo = "PyRuntimeInfo")
+load(":common/python/common.bzl", "check_native_allowed")
+load(":common/python/attributes.bzl", "NATIVE_RULES_ALLOWLIST_ATTRS")
 
 _py_builtins = _builtins.internal.py_builtins
 
 def _py_runtime_impl(ctx):
+    check_native_allowed(ctx)
     interpreter_path = ctx.attr.interpreter_path or None  # Convert empty string to None
     interpreter = ctx.file.interpreter
     if (interpreter_path and interpreter) or (not interpreter_path and not interpreter):
@@ -124,7 +127,7 @@ py_runtime(
 ```
 """,
     fragments = ["py"],
-    attrs = {
+    attrs = NATIVE_RULES_ALLOWLIST_ATTRS | {
         "files": attr.label_list(
             allow_files = True,
             doc = """

src/main/starlark/builtins_bzl/common/python/semantics.bzl

@@ -27,3 +27,6 @@ PY_RUNTIME_ATTR_NAME = "_py_interpreter"
 BUILD_DATA_SYMLINK_PATH = None
 
 IS_BAZEL = True
+
+NATIVE_RULES_MIGRATION_HELP_URL = "https://github.com/bazelbuild/bazel/issues/17773"
+NATIVE_RULES_MIGRATION_FIX_CMD = "add_python_loads"