Implement deferred srcs_version validation

When --experimental_allow_python_version_transitions is enabled, srcs_version will now be checked by py_binary/py_test instead of at every py_library. This allows building multiple py_library targets in the same invocation (e.g. via bazel build //pkg/...) regardless of their srcs_versions constraints.

The check occurs at analysis time, but any failure is reported at execution time. This allows a configured target to be created for diagnostic purposes (i.e. finding the bad transitive dependency). I've updated our analysis-time tests to account for this possibility by asserting that this deferred failure does not occur.

The "py" provider now has fields `has_py2_only_sources` and `has_py3_only_sources`. Will add a couple tests for accessing these from Starlark code in a follow-up CL.

Work towards bazelbuild#6583 and bazelbuild#1446.

RELNOTES: None
PiperOrigin-RevId: 229986854

src/main/java/com/google/devtools/build/lib/actions/FailAction.java

@@ -41,6 +41,10 @@ public Artifact getPrimaryInput() {
     return null;
   }
 
+  public String getErrorMessage() {
+    return errorMessage;
+  }
+
   @Override
   public ActionResult execute(ActionExecutionContext actionExecutionContext)
       throws ActionExecutionException {
@@ -54,8 +58,10 @@ protected void computeKey(ActionKeyContext actionKeyContext, Fingerprint fp) {
 
   @Override
   protected String getRawProgressMessage() {
-    return "Building unsupported rule " + getOwner().getLabel()
-        + " located at " + getOwner().getLocation();
+    return "Reporting failed target "
+        + getOwner().getLabel()
+        + " located at "
+        + getOwner().getLocation();
   }
 
   @Override

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

@@ -20,6 +20,7 @@
 import com.google.devtools.build.lib.actions.Action;
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.actions.FailAction;
 import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
 import com.google.devtools.build.lib.actions.extra.PythonInfo;
 import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
@@ -28,6 +29,7 @@
 import com.google.devtools.build.lib.analysis.PseudoAction;
 import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder;
 import com.google.devtools.build.lib.analysis.RuleContext;
+import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.Util;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
@@ -40,7 +42,9 @@
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.Rule;
+import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.packages.StructImpl;
+import com.google.devtools.build.lib.rules.cpp.CcInfo;
 import com.google.devtools.build.lib.rules.cpp.CppFileTypes;
 import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
 import com.google.devtools.build.lib.syntax.EvalException;
@@ -107,6 +111,18 @@ public void collectMetadataArtifacts(Iterable<Artifact> artifacts,
   /** Extra Python module import paths propagated or used by this target. */
   private final NestedSet<String> imports;
 
+  /**
+   * Whether any of this target's transitive {@code deps} have PY2-only source files, including this
+   * target itself.
+   */
+  private final boolean hasPy2OnlySources;
+
+  /**
+   * Whether any of this target's transitive {@code deps} have PY3-only source files, including this
+   * target itself.
+   */
+  private final boolean hasPy3OnlySources;
+
   /**
    * Symlink map from root-relative paths to 2to3 converted source artifacts.
    *
@@ -133,8 +149,10 @@ public PyCommon(RuleContext ruleContext, PythonSemantics semantics) {
     this.transitivePythonSources = initTransitivePythonSources(ruleContext);
     this.usesSharedLibraries = initUsesSharedLibraries(ruleContext);
     this.imports = initImports(ruleContext, semantics);
+    this.hasPy2OnlySources = initHasPy2OnlySources(ruleContext, this.sourcesVersion);
+    this.hasPy3OnlySources = initHasPy3OnlySources(ruleContext, this.sourcesVersion);
     this.convertedFiles = makeAndInitConvertedFiles(ruleContext, version, this.sourcesVersion);
-    validateSourceIsCompatible();
+    maybeValidateVersionCompatibleWithOwnSourcesAttr();
     validatePythonVersionAttr();
   }
 
@@ -268,6 +286,53 @@ private static NestedSet<String> initImports(RuleContext ruleContext, PythonSema
     return builder.build();
   }
 
+  /**
+   * Returns true if any of {@code deps} has a py provider with {@code has_py2_only_sources} set, or
+   * this target has a {@code srcs_version} of {@code PY2ONLY}.
+   */
+  // TODO(#1393): For Bazel, deprecate 2to3 support and treat PY2 the same as PY2ONLY.
+  private static boolean initHasPy2OnlySources(
+      RuleContext ruleContext, PythonVersion sourcesVersion) {
+    if (sourcesVersion == PythonVersion.PY2ONLY) {
+      return true;
+    }
+    for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
+      if (PyProvider.hasProvider(dep)) {
+        try {
+          if (PyProvider.getHasPy2OnlySources(PyProvider.getProvider(dep))) {
+            return true;
+          }
+        } catch (EvalException e) {
+          ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+        }
+      }
+    }
+    return false;
+  }
+
+  /**
+   * Returns true if any of {@code deps} has a py provider with {@code has_py3_only_sources} set, or
+   * this target has {@code srcs_version} of {@code PY3} or {@code PY3ONLY}.
+   */
+  private static boolean initHasPy3OnlySources(
+      RuleContext ruleContext, PythonVersion sourcesVersion) {
+    if (sourcesVersion == PythonVersion.PY3 || sourcesVersion == PythonVersion.PY3ONLY) {
+      return true;
+    }
+    for (TransitiveInfoCollection dep : ruleContext.getPrerequisites("deps", Mode.TARGET)) {
+      if (PyProvider.hasProvider(dep)) {
+        try {
+          if (PyProvider.getHasPy3OnlySources(PyProvider.getProvider(dep))) {
+            return true;
+          }
+        } catch (EvalException e) {
+          ruleContext.ruleError(String.format("In dep '%s': %s", dep.getLabel(), e.getMessage()));
+        }
+      }
+    }
+    return false;
+  }
+
   /**
    * If 2to3 conversion is to be done, creates the 2to3 actions and returns the map of converted
    * files; otherwise returns null.
@@ -288,8 +353,22 @@ private static Map<PathFragment, Artifact> makeAndInitConvertedFiles(
     }
   }
 
-  /** Checks that the source file version is compatible with the Python interpreter. */
-  private void validateSourceIsCompatible() {
+  /**
+   * Under the old version semantics ({@code
+   * --experimental_allow_python_version_transitions=false}), checks that the {@code srcs_version}
+   * attribute is compatible with the Python version as determined by the configuration.
+   *
+   * <p>A failure is reported as a rule error.
+   *
+   * <p>This check is local to the current target and intended to be enforced by each {@code
+   * py_library} up the dependency chain.
+   *
+   * <p>No-op under the new version semantics.
+   */
+  private void maybeValidateVersionCompatibleWithOwnSourcesAttr() {
+    if (ruleContext.getFragment(PythonConfiguration.class).useNewPyVersionSemantics()) {
+      return;
+    }
     // Treat PY3 as PY3ONLY: we'll never implement 3to2.
     if ((version == PythonVersion.PY2 || version == PythonVersion.PY2AND3)
         && (sourcesVersion == PythonVersion.PY3 || sourcesVersion == PythonVersion.PY3ONLY)) {
@@ -324,10 +403,53 @@ private void validatePythonVersionAttr() {
     }
   }
 
+  /**
+   * 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
+   * #hasPy2OnlySources} or {@link #hasPy3OnlySources}, emits a {@link FailAction} that "generates"
+   * the executable.
+   *
+   * <p>If the version is consistent, or if we are using the old semantics, no such action is
+   * emitted.
+   *
+   * <p>We use a {@code FailAction} rather than a rule error because we want to defer the error
+   * until the execution phase. This way, we still get a configured target that the user can query
+   * over with an aspect to find the exact transitive dependency that introduced the offending
+   * version constraint.
+   *
+   * @return true if a {@link FailAction} was created
+   */
+  private boolean maybeCreateFailActionDueToTransitiveSourcesVersion() {
+    if (!ruleContext.getFragment(PythonConfiguration.class).useNewPyVersionSemantics()) {
+      return false;
+    }
+    // TODO(brandjon): Add hints to the error message about how to locate the offending
+    // dependencies.
+    String error = null;
+    if (version == PythonVersion.PY2 && hasPy3OnlySources) {
+      error =
+          "target is being built for Python 2 but (transitively) includes Python 3-only sources";
+    } else if (version == PythonVersion.PY3 && hasPy2OnlySources) {
+      error =
+          "target is being built for Python 3 but (transitively) includes Python 2-only sources";
+    }
+    if (error == null) {
+      return false;
+    } else {
+      ruleContext.registerAction(
+          new FailAction(ruleContext.getActionOwner(), ImmutableList.of(executable), error));
+      return true;
+    }
+  }
+
   public PythonVersion getVersion() {
     return version;
   }
 
+  public PythonVersion getSourcesVersion() {
+    return sourcesVersion;
+  }
+
   /**
    * Returns the transitive Python sources collected from the deps attribute, not including sources
    * from the srcs attribute (unless they were separately reached via deps).
@@ -349,6 +471,14 @@ public NestedSet<String> getImports() {
     return imports;
   }
 
+  public boolean hasPy2OnlySources() {
+    return hasPy2OnlySources;
+  }
+
+  public boolean hasPy3OnlySources() {
+    return hasPy3OnlySources;
+  }
+
   public Map<PathFragment, Artifact> getConvertedFiles() {
     return convertedFiles;
   }
@@ -413,6 +543,8 @@ public void addCommonTransitiveInfoProviders(
                 .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.
@@ -562,6 +694,30 @@ public NestedSet<Artifact> getFilesToBuild() {
     return filesToBuild;
   }
 
+  /**
+   * Creates the actual executable artifact, i.e., emits a generating action for {@link
+   * #getExecutable()}.
+   *
+   * <p>If there is a transitive sources version conflict, may produce a {@link FailAction} to
+   * trigger an execution-time failure. See {@link
+   * #maybeCreateFailActionDueToTransitiveSourcesVersion}.
+   */
+  public Artifact createExecutable(
+      CcInfo ccInfo, NestedSet<String> givenImports, Runfiles.Builder defaultRunfilesBuilder)
+      throws InterruptedException, RuleErrorException {
+    boolean failed = maybeCreateFailActionDueToTransitiveSourcesVersion();
+    if (failed) {
+      return executable;
+    } else {
+      // TODO(#7054): We pass imports as an arg instead of taking them from the PyCommon field
+      // because the imports logic is a little inconsistent, and passing it explicitly may help
+      // avoid creating bugs that make the situation worse. We can eliminate this arg when we
+      // straighten up the other imports logic.
+      return semantics.createExecutable(
+          ruleContext, this, ccInfo, givenImports, defaultRunfilesBuilder);
+    }
+  }
+
   private static String buildMultipleMainMatchesErrorText(boolean explicit, String proposedMainName,
       String match1, String match2) {
     String errorText;

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

@@ -74,8 +74,7 @@ public ConfiguredTarget create(RuleContext ruleContext)
         .merge(commonRunfiles);
     semantics.collectDefaultRunfilesForBinary(ruleContext, defaultRunfilesBuilder);
 
-    Artifact realExecutable =
-        semantics.createExecutable(ruleContext, common, ccInfo, imports, defaultRunfilesBuilder);
+    Artifact realExecutable = common.createExecutable(ccInfo, imports, defaultRunfilesBuilder);
 
     Runfiles defaultRunfiles = defaultRunfilesBuilder.build();
 

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

@@ -60,6 +60,18 @@ private PyProvider() {}
   // with the one expected by the rules.
   public static final String IMPORTS = "imports";
 
+  /**
+   * Name of field holding a boolean indicating whether there are any transitive sources that
+   * require a Python 2 runtime.
+   */
+  public static final String HAS_PY2_ONLY_SOURCES = "has_py2_only_sources";
+
+  /**
+   * Name of field holding a boolean indicating whether there are any transitive sources that
+   * require a Python 3 runtime.
+   */
+  public static final String HAS_PY3_ONLY_SOURCES = "has_py3_only_sources";
+
   private static final ImmutableMap<String, Object> DEFAULTS;
 
   static {
@@ -69,6 +81,8 @@ private PyProvider() {}
     builder.put(
         IMPORTS,
         SkylarkNestedSet.of(String.class, NestedSetBuilder.<String>compileOrder().build()));
+    builder.put(HAS_PY2_ONLY_SOURCES, false);
+    builder.put(HAS_PY3_ONLY_SOURCES, false);
     DEFAULTS = builder.build();
   }
 
@@ -160,6 +174,40 @@ public static NestedSet<String> getImports(StructImpl info) throws EvalException
     return castValue.getSet(String.class);
   }
 
+  /**
+   * Casts and returns the py2-only-sources field (or its default value).
+   *
+   * @throws EvalException if the field exists and is not a boolean
+   */
+  public static boolean getHasPy2OnlySources(StructImpl info) throws EvalException {
+    Object fieldValue = getValue(info, HAS_PY2_ONLY_SOURCES);
+    return SkylarkType.cast(
+        fieldValue,
+        Boolean.class,
+        null,
+        "'%s' provider's '%s' field should be a boolean (got a '%s')",
+        PROVIDER_NAME,
+        HAS_PY2_ONLY_SOURCES,
+        EvalUtils.getDataTypeNameFromClass(fieldValue.getClass()));
+  }
+
+  /**
+   * Casts and returns the py3-only-sources field (or its default value).
+   *
+   * @throws EvalException if the field exists and is not a boolean
+   */
+  public static boolean getHasPy3OnlySources(StructImpl info) throws EvalException {
+    Object fieldValue = getValue(info, HAS_PY3_ONLY_SOURCES);
+    return SkylarkType.cast(
+        fieldValue,
+        Boolean.class,
+        null,
+        "'%s' provider's '%s' field should be a boolean (got a '%s')",
+        PROVIDER_NAME,
+        HAS_PY3_ONLY_SOURCES,
+        EvalUtils.getDataTypeNameFromClass(fieldValue.getClass()));
+  }
+
   public static Builder builder() {
     return new Builder();
   }
@@ -169,6 +217,8 @@ public static class Builder {
     SkylarkNestedSet transitiveSources = null;
     Boolean usesSharedLibraries = null;
     SkylarkNestedSet imports = null;
+    Boolean hasPy2OnlySources = null;
+    Boolean hasPy3OnlySources = null;
 
     // Use the static builder() method instead.
     private Builder() {}
@@ -188,6 +238,16 @@ public Builder setImports(NestedSet<String> imports) {
       return this;
     }
 
+    public Builder setHasPy2OnlySources(boolean hasPy2OnlySources) {
+      this.hasPy2OnlySources = hasPy2OnlySources;
+      return this;
+    }
+
+    public Builder setHasPy3OnlySources(boolean hasPy3OnlySources) {
+      this.hasPy3OnlySources = hasPy3OnlySources;
+      return this;
+    }
+
     private static void put(
         ImmutableMap.Builder<String, Object> fields, String fieldName, Object value) {
       fields.put(fieldName, value != null ? value : DEFAULTS.get(fieldName));
@@ -199,6 +259,8 @@ public StructImpl build() {
       put(fields, TRANSITIVE_SOURCES, transitiveSources);
       put(fields, USES_SHARED_LIBRARIES, usesSharedLibraries);
       put(fields, IMPORTS, imports);
+      put(fields, HAS_PY2_ONLY_SOURCES, hasPy2OnlySources);
+      put(fields, HAS_PY3_ONLY_SOURCES, hasPy3OnlySources);
       return StructProvider.STRUCT.create(fields.build(), "No such attribute '%s'");
     }
   }

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

@@ -47,12 +47,12 @@ public class PythonConfiguration extends BuildConfiguration.Fragment {
   private final boolean useNewPyVersionSemantics;
 
   PythonConfiguration(
-      PythonVersion defaultPythonVersion,
+      PythonVersion version,
       TriState buildPythonZip,
       boolean buildTransitiveRunfilesTrees,
       boolean oldPyVersionApiAllowed,
       boolean useNewPyVersionSemantics) {
-    this.version = defaultPythonVersion;
+    this.version = version;
     this.buildPythonZip = buildPythonZip;
     this.buildTransitiveRunfilesTrees = buildTransitiveRunfilesTrees;
     this.oldPyVersionApiAllowed = oldPyVersionApiAllowed;