Make Python rules use new PyInfo provider

This makes PyProviderUtils and the Python rules aware of the new PyInfo provider, which will replace the legacy "py" struct provider. With this CL, both providers are produced by the rules, and either can be consumed from a dependency. (If both providers are present on a dependency then the legacy provider is ignored.)

PyProviderUtils gets a builder that dispatches to the legacy and modern builders. In a follow-up CL we'll use this new builder to gate whether or not the legacy provider gets produced.

Work toward #7010.

RELNOTES: None
PiperOrigin-RevId: 231471616

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

@@ -32,8 +32,10 @@
 import com.google.devtools.build.lib.packages.Attribute.LabelLateBoundDefault;
 import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
+import com.google.devtools.build.lib.packages.SkylarkProviderIdentifier;
 import com.google.devtools.build.lib.packages.TriState;
 import com.google.devtools.build.lib.rules.python.PyCommon;
+import com.google.devtools.build.lib.rules.python.PyInfo;
 import com.google.devtools.build.lib.rules.python.PyRuleClasses;
 import com.google.devtools.build.lib.rules.python.PyStructUtils;
 import com.google.devtools.build.lib.rules.python.PythonVersion;
@@ -70,6 +72,14 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
           .override(
               builder
                   .copy("deps")
+                  .mandatoryProvidersList(
+                      ImmutableList.of(
+                          // Legacy provider.
+                          // TODO(#7010): Remove this legacy set.
+                          ImmutableList.of(
+                              SkylarkProviderIdentifier.forLegacy(PyStructUtils.PROVIDER_NAME)),
+                          // Modern provider.
+                          ImmutableList.of(PyInfo.PROVIDER.id())))
                   .legacyMandatoryProviders(PyStructUtils.PROVIDER_NAME)
                   .allowedFileTypes())
           /* <!-- #BLAZE_RULE($base_py).ATTRIBUTE(imports) -->

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

@@ -562,6 +562,14 @@ public void addCommonTransitiveInfoProviders(
       NestedSet<Artifact> filesToBuild,
       NestedSet<String> imports) {
 
+    PyProviderUtils.builder()
+        .setTransitiveSources(transitivePythonSources)
+        .setUsesSharedLibraries(usesSharedLibraries)
+        .setImports(imports)
+        .setHasPy2OnlySources(hasPy2OnlySources)
+        .setHasPy3OnlySources(hasPy3OnlySources)
+        .buildAndAddToTarget(builder);
+
     builder
         .addNativeDeclaredProvider(
             InstrumentedFilesCollector.collect(
@@ -570,15 +578,6 @@ public void addCommonTransitiveInfoProviders(
                 METADATA_COLLECTOR,
                 filesToBuild,
                 /* reportedToActualSources= */ NestedSetBuilder.create(Order.STABLE_ORDER)))
-        .addSkylarkTransitiveInfo(
-            PyStructUtils.PROVIDER_NAME,
-            PyStructUtils.builder()
-                .setTransitiveSources(transitivePythonSources)
-                .setUsesSharedLibraries(usesSharedLibraries)
-                .setImports(imports)
-                .setHasPy2OnlySources(hasPy2OnlySources)
-                .setHasPy3OnlySources(hasPy3OnlySources)
-                .build())
         // Python targets are not really compilable. The best we can do is make sure that all
         // generated source files are ready.
         .addOutputGroup(OutputGroupInfo.FILES_TO_COMPILE, transitivePythonSources)

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

@@ -16,6 +16,7 @@
 
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.analysis.FileProvider;
+import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
@@ -36,17 +37,30 @@ public class PyProviderUtils {
   // Disable construction.
   private PyProviderUtils() {}
 
-  /** Returns whether a given target has the py provider. */
-  public static boolean hasProvider(TransitiveInfoCollection target) {
+  /** Returns whether a given target has the {@code PyInfo} provider. */
+  public static boolean hasModernProvider(TransitiveInfoCollection target) {
+    return target.get(PyInfo.PROVIDER) != null;
+  }
+
+  /**
+   * Returns the {@code PyInfo} provider from the given target info, or null if the provider is not
+   * present.
+   */
+  public static PyInfo getModernProvider(TransitiveInfoCollection target) {
+    return target.get(PyInfo.PROVIDER);
+  }
+
+  /** Returns whether a given target has the legacy "py" provider. */
+  public static boolean hasLegacyProvider(TransitiveInfoCollection target) {
     return target.get(PyStructUtils.PROVIDER_NAME) != null;
   }
 
   /**
-   * Returns the struct representing the py provider, from the given target info.
+   * Returns the struct representing the legacy "py" provider, from the given target info.
    *
    * @throws EvalException if the provider does not exist or has the wrong type.
    */
-  public static StructImpl getProvider(TransitiveInfoCollection target) throws EvalException {
+  public static StructImpl getLegacyProvider(TransitiveInfoCollection target) throws EvalException {
     Object info = target.get(PyStructUtils.PROVIDER_NAME);
     if (info == null) {
       throw new EvalException(/*location=*/ null, "Target does not have 'py' provider");
@@ -71,8 +85,10 @@ public static StructImpl getProvider(TransitiveInfoCollection target) throws Eva
   // the relevant rules.
   public static NestedSet<Artifact> getTransitiveSources(TransitiveInfoCollection target)
       throws EvalException {
-    if (hasProvider(target)) {
-      return PyStructUtils.getTransitiveSources(getProvider(target));
+    if (hasModernProvider(target)) {
+      return getModernProvider(target).getTransitiveSources().getSet(Artifact.class);
+    } else if (hasLegacyProvider(target)) {
+      return PyStructUtils.getTransitiveSources(getLegacyProvider(target));
     } else {
       NestedSet<Artifact> files = target.getProvider(FileProvider.class).getFilesToBuild();
       return NestedSetBuilder.<Artifact>compileOrder()
@@ -92,8 +108,10 @@ public static NestedSet<Artifact> getTransitiveSources(TransitiveInfoCollection
    */
   public static boolean getUsesSharedLibraries(TransitiveInfoCollection target)
       throws EvalException {
-    if (hasProvider(target)) {
-      return PyStructUtils.getUsesSharedLibraries(getProvider(target));
+    if (hasModernProvider(target)) {
+      return getModernProvider(target).getUsesSharedLibraries();
+    } else if (hasLegacyProvider(target)) {
+      return PyStructUtils.getUsesSharedLibraries(getLegacyProvider(target));
     } else {
       NestedSet<Artifact> files = target.getProvider(FileProvider.class).getFilesToBuild();
       return FileType.contains(files, CppFileTypes.SHARED_LIBRARY);
@@ -127,8 +145,10 @@ public static NestedSet<String> getImports(TransitiveInfoCollection target) thro
    * to false.
    */
   public static boolean getHasPy2OnlySources(TransitiveInfoCollection target) throws EvalException {
-    if (hasProvider(target)) {
-      return PyStructUtils.getHasPy2OnlySources(getProvider(target));
+    if (hasModernProvider(target)) {
+      return getModernProvider(target).getHasPy2OnlySources();
+    } else if (hasLegacyProvider(target)) {
+      return PyStructUtils.getHasPy2OnlySources(getLegacyProvider(target));
     } else {
       return false;
     }
@@ -141,10 +161,62 @@ public static boolean getHasPy2OnlySources(TransitiveInfoCollection target) thro
    * to false.
    */
   public static boolean getHasPy3OnlySources(TransitiveInfoCollection target) throws EvalException {
-    if (hasProvider(target)) {
-      return PyStructUtils.getHasPy3OnlySources(getProvider(target));
+    if (hasModernProvider(target)) {
+      return getModernProvider(target).getHasPy3OnlySources();
+    } else if (hasLegacyProvider(target)) {
+      return PyStructUtils.getHasPy3OnlySources(getLegacyProvider(target));
     } else {
       return false;
     }
   }
+
+  public static Builder builder() {
+    return new Builder();
+  }
+
+  /** 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();
+
+    // Use the static builder() method instead.
+    private Builder() {}
+
+    public Builder setTransitiveSources(NestedSet<Artifact> transitiveSources) {
+      modernBuilder.setTransitiveSources(transitiveSources);
+      legacyBuilder.setTransitiveSources(transitiveSources);
+      return this;
+    }
+
+    public Builder setUsesSharedLibraries(boolean usesSharedLibraries) {
+      modernBuilder.setUsesSharedLibraries(usesSharedLibraries);
+      legacyBuilder.setUsesSharedLibraries(usesSharedLibraries);
+      return this;
+    }
+
+    public Builder setImports(NestedSet<String> imports) {
+      modernBuilder.setImports(imports);
+      legacyBuilder.setImports(imports);
+      return this;
+    }
+
+    public Builder setHasPy2OnlySources(boolean hasPy2OnlySources) {
+      modernBuilder.setHasPy2OnlySources(hasPy2OnlySources);
+      legacyBuilder.setHasPy2OnlySources(hasPy2OnlySources);
+      return this;
+    }
+
+    public Builder setHasPy3OnlySources(boolean hasPy3OnlySources) {
+      modernBuilder.setHasPy3OnlySources(hasPy3OnlySources);
+      legacyBuilder.setHasPy3OnlySources(hasPy3OnlySources);
+      return this;
+    }
+
+    public RuleConfiguredTargetBuilder buildAndAddToTarget(
+        RuleConfiguredTargetBuilder targetBuilder) {
+      targetBuilder.addSkylarkTransitiveInfo(PyStructUtils.PROVIDER_NAME, legacyBuilder.build());
+      targetBuilder.addNativeDeclaredProvider(modernBuilder.build());
+      return targetBuilder;
+    }
+  }
 }

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

@@ -225,8 +225,10 @@ java_test(
     name = "PythonStarlarkApiTest",
     srcs = ["PythonStarlarkApiTest.java"],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib:build-base",
         "//src/main/java/com/google/devtools/build/lib:packages-internal",
         "//src/main/java/com/google/devtools/build/lib:python-rules",
+        "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/test/java/com/google/devtools/build/lib:analysis_testutil",
         "//third_party:junit4",
         "//third_party:truth",

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

@@ -57,7 +57,8 @@ public void badSrcsVersionValue() throws Exception {
 
   @Test
   public void goodSrcsVersionValue() throws Exception {
-    scratch.file("pkg/BUILD",
+    scratch.file(
+        "pkg/BUILD",
         ruleName + "(",
         "    name = 'foo',",
         "    srcs_version = 'PY2',",
@@ -89,7 +90,8 @@ public void srcsVersionClashesWithForcePythonFlagUnderOldSemantics() throws Exce
   @Test
   public void versionIs2IfUnspecified() throws Exception {
     ensureDefaultIsPY2();
-    scratch.file("pkg/BUILD",
+    scratch.file(
+        "pkg/BUILD", //
         ruleName + "(",
         "    name = 'foo',",
         "    srcs = ['foo.py'])");
@@ -105,7 +107,8 @@ public void versionIs3IfForcedByFlagUnderOldSemantics() throws Exception {
     // test.
     ensureDefaultIsPY2();
     useConfiguration("--experimental_allow_python_version_transitions=false", "--force_python=PY3");
-    scratch.file("pkg/BUILD",
+    scratch.file(
+        "pkg/BUILD", //
         ruleName + "(",
         "    name = 'foo',",
         "    srcs = ['foo.py'])");
@@ -125,7 +128,8 @@ public void packageNameCannotHaveHyphen() throws Exception {
 
   @Test
   public void srcsPackageNameCannotHaveHyphen() throws Exception {
-    scratch.file("pkg-hyphenated/BUILD",
+    scratch.file(
+        "pkg-hyphenated/BUILD", //
         "exports_files(['bar.py'])");
     checkError("otherpkg", "foo",
         // error:
@@ -135,4 +139,92 @@ public void srcsPackageNameCannotHaveHyphen() throws Exception {
         "    name = 'foo',",
         "    srcs = ['foo.py', '//pkg-hyphenated:bar.py'])");
   }
+
+  @Test
+  public void producesBothModernAndLegacyProviders() throws Exception {
+    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)).isNotNull();
+  }
+
+  @Test
+  public void consumesLegacyProvider() throws Exception {
+    scratch.file(
+        "pkg/rules.bzl",
+        "def _myrule_impl(ctx):",
+        "    return struct(py=struct(transitive_sources=depset([])))",
+        "myrule = rule(",
+        "    implementation = _myrule_impl,",
+        ")");
+    scratch.file(
+        "pkg/BUILD",
+        "load(':rules.bzl', 'myrule')",
+        "myrule(",
+        "    name = 'dep',",
+        ")",
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = ['foo.py'],",
+        "    deps = [':dep'],",
+        ")");
+    ConfiguredTarget target = getConfiguredTarget("//pkg:foo");
+    assertThat(target).isNotNull();
+    assertNoEvents();
+  }
+
+  @Test
+  public void consumesModernProvider() throws Exception {
+    scratch.file(
+        "pkg/rules.bzl",
+        "def _myrule_impl(ctx):",
+        "    return [PyInfo(transitive_sources=depset([]))]",
+        "myrule = rule(",
+        "    implementation = _myrule_impl,",
+        ")");
+    scratch.file(
+        "pkg/BUILD",
+        "load(':rules.bzl', 'myrule')",
+        "myrule(",
+        "    name = 'dep',",
+        ")",
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = ['foo.py'],",
+        "    deps = [':dep'],",
+        ")");
+    ConfiguredTarget target = getConfiguredTarget("//pkg:foo");
+    assertThat(target).isNotNull();
+    assertNoEvents();
+  }
+
+  @Test
+  public void requiresProvider() throws Exception {
+    scratch.file(
+        "pkg/rules.bzl",
+        "def _myrule_impl(ctx):",
+        "    return []",
+        "myrule = rule(",
+        "    implementation = _myrule_impl,",
+        ")");
+    checkError(
+        "pkg",
+        "foo",
+        // error:
+        "'//pkg:dep' does not have mandatory providers",
+        // build file:
+        "load(':rules.bzl', 'myrule')",
+        "myrule(",
+        "    name = 'dep',",
+        ")",
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = ['foo.py'],",
+        "    deps = [':dep'],",
+        ")");
+  }
 }

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

@@ -107,7 +107,7 @@ protected void assertPythonVersionIs_UnderNewConfigs(
     }
   }
 
-  private String join(String... lines) {
+  private static String join(String... lines) {
     return String.join("\n", lines);
   }