Add flag to support PyInfo provider migration

This adds --experimental_disallow_legacy_py_provider, which 1) makes native Python rules no longer emit a "py" struct provider, and 2) makes these rules fail-fast if any direct dependency returns a "py" struct provider. Rules should upgrade to PyInfo instead.

Once we add suitable documentation / tracking bugs, we'll rename to --incompatible_[...].

Work toward #7010.

RELNOTES: None
PiperOrigin-RevId: 231839371

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

@@ -186,6 +186,7 @@ public PyCommon(RuleContext ruleContext, PythonSemantics semantics) {
     validateTargetPythonVersionAttr(DEFAULT_PYTHON_VERSION_ATTRIBUTE);
     validateTargetPythonVersionAttr(PYTHON_VERSION_ATTRIBUTE);
     validateOldVersionAttrNotUsedIfDisabled();
+    validateLegacyProviderNotUsedIfDisabled();
   }
 
   /** Returns the parsed value of the "srcs_version" attribute. */
@@ -233,7 +234,8 @@ private static void collectTransitivePythonSourcesFromDeps(
         builder.addTransitive(PyProviderUtils.getTransitiveSources(dep));
       } catch (EvalException e) {
         // Either the provider type or field type is bad.
-        ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+        ruleContext.attributeError(
+            "deps", String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
       }
     }
   }
@@ -285,7 +287,8 @@ private static NestedSet<String> initImports(RuleContext ruleContext, PythonSema
           builder.addTransitive(imports);
         }
       } catch (EvalException e) {
-        ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+        ruleContext.attributeError(
+            "deps", String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
       }
     }
     return builder.build();
@@ -307,7 +310,8 @@ private static boolean initHasPy2OnlySources(
           return true;
         }
       } catch (EvalException e) {
-        ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+        ruleContext.attributeError(
+            "deps", String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
       }
     }
     return false;
@@ -328,7 +332,8 @@ private static boolean initHasPy3OnlySources(
           return true;
         }
       } catch (EvalException e) {
-        ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+        ruleContext.attributeError(
+            "deps", String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
       }
     }
     return false;
@@ -437,6 +442,27 @@ private void validateOldVersionAttrNotUsedIfDisabled() {
     }
   }
 
+  /**
+   * Reports an attribute error if a target in {@code deps} passes the legacy "py" provider but this
+   * is disallowed by the configuration.
+   */
+  private void validateLegacyProviderNotUsedIfDisabled() {
+    if (!ruleContext.getFragment(PythonConfiguration.class).disallowLegacyPyProvider()) {
+      return;
+    }
+    for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
+      if (PyProviderUtils.hasLegacyProvider(dep)) {
+        ruleContext.attributeError(
+            "deps",
+            String.format(
+                "In dep '%s': The legacy 'py' provider is disallowed. Migrate to the PyInfo "
+                    + "provider instead. You can temporarily disable this failure with "
+                    + "--incompatible_disallow_legacy_py_provider=false.",
+                dep.getLabel()));
+      }
+    }
+  }
+
   /**
    * Under the new version semantics ({@code --experimental_allow_python_version_transitions=true}),
    * if the Python version (as determined by the configuration) is inconsistent with {@link
@@ -562,7 +588,9 @@ public void addCommonTransitiveInfoProviders(
       NestedSet<Artifact> filesToBuild,
       NestedSet<String> imports) {
 
-    PyProviderUtils.builder()
+    boolean createLegacyPyProvider =
+        !ruleContext.getFragment(PythonConfiguration.class).disallowLegacyPyProvider();
+    PyProviderUtils.builder(createLegacyPyProvider)
         .setTransitiveSources(transitivePythonSources)
         .setUsesSharedLibraries(usesSharedLibraries)
         .setImports(imports)

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

@@ -170,17 +170,27 @@ public static boolean getHasPy3OnlySources(TransitiveInfoCollection target) thro
     }
   }
 
-  public static Builder builder() {
-    return new Builder();
+  /**
+   * Returns a builder to construct the legacy and/or modern Python providers and add them to a
+   * configured target.
+   *
+   * <p>If {@code createLegacy} is false, only the modern {@code PyInfo} provider is produced.
+   * Otherwise both {@code PyInfo} and the "py" provider are produced.
+   */
+  public static Builder builder(boolean createLegacy) {
+    return new Builder(createLegacy);
   }
 
   /** A builder to add both the legacy and modern providers to a configured target. */
   public static class Builder {
     private final PyInfo.Builder modernBuilder = PyInfo.builder();
     private final PyStructUtils.Builder legacyBuilder = PyStructUtils.builder();
+    private final boolean createLegacy;
 
     // Use the static builder() method instead.
-    private Builder() {}
+    private Builder(boolean createLegacy) {
+      this.createLegacy = createLegacy;
+    }
 
     public Builder setTransitiveSources(NestedSet<Artifact> transitiveSources) {
       modernBuilder.setTransitiveSources(transitiveSources);
@@ -214,8 +224,10 @@ public Builder setHasPy3OnlySources(boolean hasPy3OnlySources) {
 
     public RuleConfiguredTargetBuilder buildAndAddToTarget(
         RuleConfiguredTargetBuilder targetBuilder) {
-      targetBuilder.addSkylarkTransitiveInfo(PyStructUtils.PROVIDER_NAME, legacyBuilder.build());
       targetBuilder.addNativeDeclaredProvider(modernBuilder.build());
+      if (createLegacy) {
+        targetBuilder.addSkylarkTransitiveInfo(PyStructUtils.PROVIDER_NAME, legacyBuilder.build());
+      }
       return targetBuilder;
     }
   }

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

@@ -42,21 +42,26 @@ public class PythonConfiguration extends BuildConfiguration.Fragment {
   private final TriState buildPythonZip;
   private final boolean buildTransitiveRunfilesTrees;
 
-  // TODO(brandjon): Remove these once migration to the new API is complete (#6583).
+  // TODO(brandjon): Remove these once migration to the new version API is complete (#6583).
   private final boolean oldPyVersionApiAllowed;
   private final boolean useNewPyVersionSemantics;
 
+  // TODO(brandjon): Remove this once migration to the new provider is complete (#7010).
+  private final boolean disallowLegacyPyProvider;
+
   PythonConfiguration(
       PythonVersion version,
       TriState buildPythonZip,
       boolean buildTransitiveRunfilesTrees,
       boolean oldPyVersionApiAllowed,
-      boolean useNewPyVersionSemantics) {
+      boolean useNewPyVersionSemantics,
+      boolean disallowLegacyPyProvider) {
     this.version = version;
     this.buildPythonZip = buildPythonZip;
     this.buildTransitiveRunfilesTrees = buildTransitiveRunfilesTrees;
     this.oldPyVersionApiAllowed = oldPyVersionApiAllowed;
     this.useNewPyVersionSemantics = useNewPyVersionSemantics;
+    this.disallowLegacyPyProvider = disallowLegacyPyProvider;
   }
 
   /**
@@ -132,4 +137,14 @@ public boolean oldPyVersionApiAllowed() {
   public boolean useNewPyVersionSemantics() {
     return useNewPyVersionSemantics;
   }
+
+  /**
+   * Returns true if Python rules should omit the legacy "py" provider and fail-fast when given this
+   * provider from their {@code deps}.
+   *
+   * <p>Any rules that pass this provider should be updated to pass {@code PyInfo} instead.
+   */
+  public boolean disallowLegacyPyProvider() {
+    return disallowLegacyPyProvider;
+  }
 }

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

@@ -26,7 +26,7 @@
 public class PythonConfigurationLoader implements ConfigurationFragmentFactory {
   @Override
   public ImmutableSet<Class<? extends FragmentOptions>> requiredOptions() {
-    return ImmutableSet.<Class<? extends FragmentOptions>>of(PythonOptions.class);
+    return ImmutableSet.of(PythonOptions.class);
   }
 
   @Override
@@ -39,7 +39,8 @@ public PythonConfiguration create(BuildOptions buildOptions)
         pythonOptions.buildPythonZip,
         pythonOptions.buildTransitiveRunfilesTrees,
         /*oldPyVersionApiAllowed=*/ !pythonOptions.experimentalRemoveOldPythonVersionApi,
-        /*useNewPyVersionSemantics=*/ pythonOptions.experimentalAllowPythonVersionTransitions);
+        /*useNewPyVersionSemantics=*/ pythonOptions.experimentalAllowPythonVersionTransitions,
+        /*disallowLegacyPyProvider=*/ pythonOptions.experimentalDisallowLegacyPyProvider);
   }
 
   @Override

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

@@ -169,6 +169,19 @@ public String getTypeDescription() {
   private static final OptionDefinition HOST_FORCE_PYTHON_DEFINITION =
       OptionsParser.getOptionDefinitionByName(PythonOptions.class, "host_force_python");
 
+  // TODO(#7010): Change the option name to "incompatible_..." and enable the appropriate metadata
+  // tags.
+  @Option(
+      name = "experimental_disallow_legacy_py_provider",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      help =
+          "If set to true, native Python rules will neither produce nor consume the legacy \"py\" "
+              + "provider. Use PyInfo instead. Under this flag, passing the legacy provider to a "
+              + "Python target will be an error.")
+  public boolean experimentalDisallowLegacyPyProvider;
+
   @Override
   public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
     // TODO(brandjon): Add an error string that references documentation explaining to use
@@ -271,6 +284,7 @@ public FragmentOptions getHost() {
         (hostForcePython != null) ? hostForcePython : PythonVersion.DEFAULT_TARGET_VALUE;
     hostPythonOptions.setPythonVersion(hostVersion);
     hostPythonOptions.buildPythonZip = buildPythonZip;
+    hostPythonOptions.experimentalDisallowLegacyPyProvider = experimentalDisallowLegacyPyProvider;
     return hostPythonOptions;
   }
 

src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java

@@ -141,7 +141,8 @@ public void srcsPackageNameCannotHaveHyphen() throws Exception {
   }
 
   @Test
-  public void producesBothModernAndLegacyProviders() throws Exception {
+  public void producesBothModernAndLegacyProviders_WithoutIncompatibleFlag() throws Exception {
+    useConfiguration("--experimental_disallow_legacy_py_provider=false");
     scratch.file(
         "pkg/BUILD", //
         ruleName + "(",
@@ -153,7 +154,21 @@ public void producesBothModernAndLegacyProviders() throws Exception {
   }
 
   @Test
-  public void consumesLegacyProvider() throws Exception {
+  public void producesOnlyModernProvider_WithIncompatibleFlag() throws Exception {
+    useConfiguration("--experimental_disallow_legacy_py_provider=true");
+    scratch.file(
+        "pkg/BUILD", //
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = ['foo.py'])");
+    ConfiguredTarget target = getConfiguredTarget("//pkg:foo");
+    assertThat(target.get(PyInfo.PROVIDER)).isNotNull();
+    assertThat(target.get(PyStructUtils.PROVIDER_NAME)).isNull();
+  }
+
+  @Test
+  public void consumesLegacyProvider_WithoutIncompatibleFlag() throws Exception {
+    useConfiguration("--experimental_disallow_legacy_py_provider=false");
     scratch.file(
         "pkg/rules.bzl",
         "def _myrule_impl(ctx):",
@@ -177,6 +192,33 @@ public void consumesLegacyProvider() throws Exception {
     assertNoEvents();
   }
 
+  @Test
+  public void rejectsLegacyProvider_WithIncompatibleFlag() throws Exception {
+    useConfiguration("--experimental_disallow_legacy_py_provider=true");
+    scratch.file(
+        "pkg/rules.bzl",
+        "def _myrule_impl(ctx):",
+        "    return struct(py=struct(transitive_sources=depset([])))",
+        "myrule = rule(",
+        "    implementation = _myrule_impl,",
+        ")");
+    checkError(
+        "pkg",
+        "foo",
+        // error:
+        "In dep '//pkg:dep': The legacy 'py' provider is disallowed.",
+        // build file:
+        "load(':rules.bzl', 'myrule')",
+        "myrule(",
+        "    name = 'dep',",
+        ")",
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = ['foo.py'],",
+        "    deps = [':dep'],",
+        ")");
+  }
+
   @Test
   public void consumesModernProvider() throws Exception {
     scratch.file(

src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java

@@ -164,11 +164,13 @@ public void getHost_CopiesMostValues() throws Exception {
         parsePythonOptions(
             "--experimental_allow_python_version_transitions=true",
             "--experimental_remove_old_python_version_api=true",
-            "--build_python_zip=true");
+            "--build_python_zip=true",
+            "--experimental_disallow_legacy_py_provider=true");
     PythonOptions hostOpts = (PythonOptions) opts.getHost();
     assertThat(hostOpts.experimentalAllowPythonVersionTransitions).isTrue();
     assertThat(hostOpts.experimentalRemoveOldPythonVersionApi).isTrue();
     assertThat(hostOpts.buildPythonZip).isEqualTo(TriState.YES);
+    assertThat(hostOpts.experimentalDisallowLegacyPyProvider).isTrue();
   }
 
   @Test