Make --incompatible_remove_old_python_version_api a no-op

This flag blocked use of the old --force_python flag, and the old default_python_version attribute. It has long since been flipped. This change deletes its uses in tests and its legacy code paths.

Work toward #7797.

RELNOTES: None
PiperOrigin-RevId: 303229827

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

@@ -163,19 +163,6 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
           match any filename in <code>srcs</code>, <code>main</code> must be specified.
           <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
           .add(attr("main", LABEL).allowedFileTypes(PYTHON_SOURCE))
-          /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(default_python_version) -->
-          A deprecated alias for <code>python_version</code>; use that instead. This attribute is
-          disabled under <code>--incompatible_remove_old_python_version_api</code>. For migration
-          purposes, if <code>python_version</code> is given then the value of
-          <code>default_python_version</code> is ignored.
-          <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-          .add(
-              attr(PyCommon.DEFAULT_PYTHON_VERSION_ATTRIBUTE, STRING)
-                  .value(PythonVersion._INTERNAL_SENTINEL.toString())
-                  .allowedValues(PyRuleClasses.TARGET_PYTHON_ATTR_VALUE_SET)
-                  .nonconfigurable(
-                      "read by PyRuleClasses.PYTHON_VERSION_TRANSITION, which doesn't have access"
-                          + " to the configuration"))
           /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(python_version) -->
           Whether to build this target (and its transitive <code>deps</code>) for Python 2 or Python
           3. Valid values are <code>"PY2"</code> and <code>"PY3"</code> (the default).

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

@@ -151,7 +151,7 @@ private static void createStubFile(
     String pythonBinary = getPythonBinary(ruleContext, common, bazelConfig);
 
     // Version information for host config diagnostic warning.
-    PythonVersion attrVersion = PyCommon.readPythonVersionFromAttributes(ruleContext.attributes());
+    PythonVersion attrVersion = PyCommon.readPythonVersionFromAttribute(ruleContext.attributes());
     boolean attrVersionSpecifiedExplicitly = attrVersion != null;
     if (!attrVersionSpecifiedExplicitly) {
       attrVersion = config.getDefaultPythonVersion();

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

@@ -67,8 +67,6 @@
 /** A helper class for analyzing a Python configured target. */
 public final class PyCommon {
 
-  /** Deprecated name of the version attribute. */
-  public static final String DEFAULT_PYTHON_VERSION_ATTRIBUTE = "default_python_version";
   /** Name of the version attribute. */
   public static final String PYTHON_VERSION_ATTRIBUTE = "python_version";
 
@@ -85,32 +83,22 @@ public final class PyCommon {
       ImmutableList.of("py_library", "py_binary", "py_test", "py_runtime");
 
   /**
-   * Returns the Python version based on the {@code python_version} and {@code
-   * default_python_version} attributes of the given {@code AttributeMap}.
+   * Returns the Python version based on the {@code python_version} attribute of the given {@code
+   * AttributeMap}.
    *
-   * <p>It is expected that both attributes are defined, string-typed, and default to {@link
+   * <p>It is expected that the attribute is defined, string-typed, and defaults to {@link
    * PythonVersion#_INTERNAL_SENTINEL}. The returned version is the value of {@code python_version}
-   * if it is not the sentinel, then {@code default_python_version} if it is not the sentinel,
-   * otherwise null (when both attributes are sentinels). In all cases the return value is either a
-   * target version value ({@code PY2} or {@code PY3}) or null.
+   * if it is not the sentinel (in which case it is either {@code PY2} or {@code PY3}), or null if
+   * it is the sentinel.
    *
-   * @throws IllegalArgumentException if the attributes are not present, not string-typed, or not
-   *     parsable as target {@link PythonVersion} values or as the sentinel value
+   * @throws IllegalArgumentException if the attribute is not present, not string-typed, or not
+   *     parsable as a target {@link PythonVersion} value or the sentinel value
    */
   @Nullable
-  public static PythonVersion readPythonVersionFromAttributes(AttributeMap attrs) {
+  public static PythonVersion readPythonVersionFromAttribute(AttributeMap attrs) {
     PythonVersion pythonVersionAttr =
         PythonVersion.parseTargetOrSentinelValue(attrs.get(PYTHON_VERSION_ATTRIBUTE, Type.STRING));
-    PythonVersion defaultPythonVersionAttr =
-        PythonVersion.parseTargetOrSentinelValue(
-            attrs.get(DEFAULT_PYTHON_VERSION_ATTRIBUTE, Type.STRING));
-    if (pythonVersionAttr != PythonVersion._INTERNAL_SENTINEL) {
-      return pythonVersionAttr;
-    } else if (defaultPythonVersionAttr != PythonVersion._INTERNAL_SENTINEL) {
-      return defaultPythonVersionAttr;
-    } else {
-      return null;
-    }
+    return pythonVersionAttr != PythonVersion._INTERNAL_SENTINEL ? pythonVersionAttr : null;
   }
 
   private static final LocalMetadataCollector METADATA_COLLECTOR = new LocalMetadataCollector() {
@@ -216,9 +204,7 @@ public PyCommon(RuleContext ruleContext, PythonSemantics semantics) {
     this.hasPy3OnlySources = initHasPy3OnlySources(ruleContext, this.sourcesVersion);
     this.runtimeFromToolchain = initRuntimeFromToolchain(ruleContext, this.version);
     this.convertedFiles = makeAndInitConvertedFiles(ruleContext, version, this.sourcesVersion);
-    validateTargetPythonVersionAttr(DEFAULT_PYTHON_VERSION_ATTRIBUTE);
-    validateTargetPythonVersionAttr(PYTHON_VERSION_ATTRIBUTE);
-    validateOldVersionAttrNotUsedIfDisabled();
+    validatePythonVersionAttr();
     validateLegacyProviderNotUsedIfDisabled();
     maybeValidateLoadedFromBzl();
   }
@@ -511,55 +497,27 @@ private static Map<PathFragment, Artifact> makeAndInitConvertedFiles(
   }
 
   /**
-   * Reports an attribute error if the given target Python version attribute ({@code
-   * default_python_version} or {@code python_version}) cannot be parsed as {@code PY2}, {@code
+   * Reports an attribute error if {@code python_version} cannot be parsed as {@code PY2}, {@code
    * PY3}, or the sentinel value.
    *
    * <p>This *should* be enforced by rule attribute validation ({@link
    * Attribute.Builder.allowedValues}), but this check is here to fail-fast just in case.
    */
-  private void validateTargetPythonVersionAttr(String attr) {
+  private void validatePythonVersionAttr() {
     AttributeMap attrs = ruleContext.attributes();
-    if (!attrs.has(attr, Type.STRING)) {
+    if (!attrs.has(PYTHON_VERSION_ATTRIBUTE, Type.STRING)) {
       return;
     }
-    String attrValue = attrs.get(attr, Type.STRING);
+    String attrValue = attrs.get(PYTHON_VERSION_ATTRIBUTE, Type.STRING);
     try {
       PythonVersion.parseTargetOrSentinelValue(attrValue);
     } catch (IllegalArgumentException ex) {
       ruleContext.attributeError(
-          attr,
+          PYTHON_VERSION_ATTRIBUTE,
           String.format("'%s' is not a valid value. Expected either 'PY2' or 'PY3'", attrValue));
     }
   }
 
-  /**
-   * Reports an attribute error if the {@code default_python_version} attribute is set but
-   * disallowed by the configuration.
-   */
-  private void validateOldVersionAttrNotUsedIfDisabled() {
-    AttributeMap attrs = ruleContext.attributes();
-    if (!attrs.has(DEFAULT_PYTHON_VERSION_ATTRIBUTE, Type.STRING)) {
-      return;
-    }
-    PythonVersion value;
-    try {
-      value =
-          PythonVersion.parseTargetOrSentinelValue(
-              attrs.get(DEFAULT_PYTHON_VERSION_ATTRIBUTE, Type.STRING));
-    } catch (IllegalArgumentException e) {
-      // Should be reported by validateTargetPythonVersionAttr(); no action required here.
-      return;
-    }
-    PythonConfiguration config = ruleContext.getFragment(PythonConfiguration.class);
-    if (value != PythonVersion._INTERNAL_SENTINEL && !config.oldPyVersionApiAllowed()) {
-      ruleContext.attributeError(
-          DEFAULT_PYTHON_VERSION_ATTRIBUTE,
-          "the 'default_python_version' attribute is disabled by the "
-              + "'--incompatible_remove_old_python_version_api' flag");
-    }
-  }
-
   /**
    * Reports an attribute error if a target in {@code deps} passes the legacy "py" provider but this
    * is disallowed by the configuration.
@@ -691,7 +649,7 @@ public PythonVersion getSourcesVersion() {
    * </ol>
    *
    * @throws IllegalArgumentException if there is a problem parsing the Python version from the
-   *     attributes; see {@link #readPythonVersionFromAttributes}.
+   *     attributes; see {@link #readPythonVersionFromAttribute}.
    */
   // TODO(#6443): Remove this logic and the corresponding stub script logic once we no longer have
   // the possibility of Python binaries appearing in the host configuration.
@@ -707,7 +665,7 @@ public boolean shouldWarnAboutHostVersionUponFailure() {
     }
 
     PythonVersion configVersion = config.getPythonVersion();
-    PythonVersion attrVersion = readPythonVersionFromAttributes(ruleContext.attributes());
+    PythonVersion attrVersion = readPythonVersionFromAttribute(ruleContext.attributes());
     if (attrVersion == null) {
       // Warn if the version wasn't set explicitly.
       return true;

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

@@ -55,39 +55,37 @@ public String getErrorReason(Object value) {
    * Returns a rule transition factory for Python binary rules and other rules that may change the
    * Python version.
    *
-   * <p>The factory reads the version specified by the target's {@code python_version} attribute if
-   * given, falling back on the {@code default_python_version} attribute otherwise. Both attributes
-   * must exist on the rule class. If a value was read successfully, the factory returns a
-   * transition that sets the version to that value. Otherwise if neither attribute was set, the
-   * factory returns {@code defaultTransition} instead.
+   * <p>The factory reads the version specified by the target's {@code python_version} attribute (if
+   * given), which must exist on the rule class. If a value was read successfully, the factory
+   * returns a transition that sets the version to that value. Otherwise, the factory returns {@code
+   * defaultTransition} instead.
    *
-   * <p>If either attribute has an unparsable value on the target, then the factory returns {@code
-   * defaultTransition} and it is up to the rule's analysis phase ({@link
-   * PyCommon#validateTargetPythonVersionAttr}) to report an attribute error to the user. This case
-   * should be prevented by attribute validation if the rule class is defined correctly.
+   * <p>If the attribute has an unparsable value, then the factory returns {@code defaultTransition}
+   * and it is up to the rule's analysis phase ({@link PyCommon#validatePythonVersionAttr}) to
+   * report an attribute error to the user. This case should be prevented by attribute validation if
+   * the rule class is defined correctly.
    */
   public static TransitionFactory<Rule> makeVersionTransition(
       PythonVersionTransition defaultTransition) {
     return (rule) -> {
       AttributeMap attrs = RawAttributeMapper.of(rule);
       // Fail fast if we're used on an ill-defined rule class.
       Preconditions.checkArgument(
-          attrs.has(PyCommon.PYTHON_VERSION_ATTRIBUTE, Type.STRING)
-              && attrs.has(PyCommon.DEFAULT_PYTHON_VERSION_ATTRIBUTE, Type.STRING),
-          "python version transitions require that the RuleClass define both "
-              + "'default_python_version' and 'python_version'");
+          attrs.has(PyCommon.PYTHON_VERSION_ATTRIBUTE, Type.STRING),
+          "python version transitions require that the RuleClass define a "
+              + "'python_version' attribute");
       // Attribute validation should enforce that the attribute string value is either a target
       // value ("PY2" or "PY3") or the sentinel value ("_INTERNAL_SENTINEL"). But just in case,
       // we'll, treat an invalid value as the default value rather than propagate an unchecked
       // exception in this context. That way the user can at least get a clean error message
       // instead of a crash.
       PythonVersionTransition transition;
       try {
-        PythonVersion versionFromAttributes = PyCommon.readPythonVersionFromAttributes(attrs);
-        if (versionFromAttributes == null) {
+        PythonVersion versionFromAttribute = PyCommon.readPythonVersionFromAttribute(attrs);
+        if (versionFromAttribute == null) {
           transition = defaultTransition;
         } else {
-          transition = PythonVersionTransition.toConstant(versionFromAttributes);
+          transition = PythonVersionTransition.toConstant(versionFromAttribute);
         }
       } catch (IllegalArgumentException ex) {
         transition = defaultTransition;

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

@@ -40,9 +40,6 @@ public class PythonConfiguration extends BuildConfiguration.Fragment implements
   private final TriState buildPythonZip;
   private final boolean buildTransitiveRunfilesTrees;
 
-  // TODO(brandjon): Remove this (#7797).
-  private final boolean oldPyVersionApiAllowed;
-
   // TODO(brandjon): Remove this once migration to PY3-as-default is complete.
   private final boolean py2OutputsAreSuffixed;
 
@@ -62,7 +59,6 @@ public class PythonConfiguration extends BuildConfiguration.Fragment implements
       PythonVersion defaultVersion,
       TriState buildPythonZip,
       boolean buildTransitiveRunfilesTrees,
-      boolean oldPyVersionApiAllowed,
       boolean py2OutputsAreSuffixed,
       boolean disallowLegacyPyProvider,
       boolean useToolchains,
@@ -72,7 +68,6 @@ public class PythonConfiguration extends BuildConfiguration.Fragment implements
     this.defaultVersion = defaultVersion;
     this.buildPythonZip = buildPythonZip;
     this.buildTransitiveRunfilesTrees = buildTransitiveRunfilesTrees;
-    this.oldPyVersionApiAllowed = oldPyVersionApiAllowed;
     this.py2OutputsAreSuffixed = py2OutputsAreSuffixed;
     this.disallowLegacyPyProvider = disallowLegacyPyProvider;
     this.useToolchains = useToolchains;
@@ -145,11 +140,6 @@ public boolean buildTransitiveRunfilesTrees() {
     return buildTransitiveRunfilesTrees;
   }
 
-  /** Returns whether use of the {@code default_python_version} attribute is allowed. */
-  public boolean oldPyVersionApiAllowed() {
-    return oldPyVersionApiAllowed;
-  }
-
   /**
    * Returns true if Python rules should omit the legacy "py" provider and fail-fast when given this
    * provider from their {@code deps}.

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

@@ -39,7 +39,6 @@ public PythonConfiguration create(BuildOptions buildOptions)
         pythonOptions.getDefaultPythonVersion(),
         pythonOptions.buildPythonZip,
         pythonOptions.buildTransitiveRunfilesTrees,
-        /*oldPyVersionApiAllowed=*/ !pythonOptions.incompatibleRemoveOldPythonVersionApi,
         /*py2OutputsAreSuffixed=*/ pythonOptions.incompatiblePy2OutputsAreSuffixed,
         /*disallowLegacyPyProvider=*/ pythonOptions.incompatibleDisallowLegacyPyProvider,
         /*useToolchains=*/ pythonOptions.incompatibleUsePythonToolchains,

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

@@ -69,20 +69,22 @@ public String getTypeDescription() {
       help = "Build python executable zip; on on Windows, off on other platforms")
   public TriState buildPythonZip;
 
+  /**
+   * Deprecated machinery for setting the Python version; will be removed soon.
+   *
+   * <p>Not GraveyardOptions'd because we'll delete this alongside other soon-to-be-removed options
+   * in this file.
+   */
   @Option(
       name = "incompatible_remove_old_python_version_api",
       defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {
         OptionMetadataTag.INCOMPATIBLE_CHANGE,
         OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
       },
-      help =
-          "If true, disables use of the `default_python_version` "
-              + "attribute for `py_binary` and `py_test`. Use the `--python_version` flag and "
-              + "`python_version` attribute instead, which have exactly the same meaning. This "
-              + "flag also disables `select()`-ing over `--host_force_python`.")
+      help = "No-op, will be removed soon.")
   public boolean incompatibleRemoveOldPythonVersionApi;
 
   /**
@@ -290,18 +292,16 @@ public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
         new SelectRestriction(
             /*visibleWithinToolsPackage=*/ true,
             "Use @bazel_tools//python/tools:python_version instead."));
-    if (incompatibleRemoveOldPythonVersionApi) {
-      restrictions.put(
-          FORCE_PYTHON_DEFINITION,
-          new SelectRestriction(
-              /*visibleWithinToolsPackage=*/ true,
-              "Use @bazel_tools//python/tools:python_version instead."));
-      restrictions.put(
-          HOST_FORCE_PYTHON_DEFINITION,
-          new SelectRestriction(
-              /*visibleWithinToolsPackage=*/ false,
-              "Use @bazel_tools//python/tools:python_version instead."));
-    }
+    restrictions.put(
+        FORCE_PYTHON_DEFINITION,
+        new SelectRestriction(
+            /*visibleWithinToolsPackage=*/ true,
+            "Use @bazel_tools//python/tools:python_version instead."));
+    restrictions.put(
+        HOST_FORCE_PYTHON_DEFINITION,
+        new SelectRestriction(
+            /*visibleWithinToolsPackage=*/ false,
+            "Use @bazel_tools//python/tools:python_version instead."));
     return restrictions.build();
   }
 
@@ -354,7 +354,6 @@ public void setPythonVersion(PythonVersion version) {
   @Override
   public FragmentOptions getHost() {
     PythonOptions hostPythonOptions = (PythonOptions) getDefault();
-    hostPythonOptions.incompatibleRemoveOldPythonVersionApi = incompatibleRemoveOldPythonVersionApi;
     PythonVersion hostVersion =
         (hostForcePython != null) ? hostForcePython : getDefaultPythonVersion();
     hostPythonOptions.setPythonVersion(hostVersion);