Add --incompatible_py3_is_default to change py_binary and `py_tes…

…t`'s default version

When this flag is enabled, `py_binary` and `py_test` targets that do not specify a version (by setting the `python_version` attribute, or its deprecated alias `default_python_version`) will default to `PY3` instead of `PY2`. In addition, the host configuration will use `PY3`, unless `--host_force_python=PY2` is set to override it.

The flag requires the new version semantics (`--incompatible_allow_python_version_transitions=true`), and will fail the build if it is not set.

The flag is implemented by eliminating PythonVersion#DEFAULT_TARGET_VALUE and replacing it with an accessor PythonOptions#getDefaultPythonVersion. For the case of rule transition factories in rule definitions, which previously relied on DEFAULT_TARGET_VALUE and which do not have access to a configuration, they are updated to instead pass a callback function that retrieves the default version from the configuration at the time of the transition.

Work toward #6647, #7359.

RELNOTES[INC]: Added --incompatible_py3_is_default to test switching the default Python version to PY3 for py_binary/py_test targets that do not specify a version. See #7359.

PiperOrigin-RevId: 232939284

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

@@ -57,6 +57,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.UUID;
+import javax.annotation.Nullable;
 
 /** A helper class for analyzing a Python configured target. */
 public final class PyCommon {
@@ -70,17 +71,15 @@ public final class PyCommon {
    *
    * <p>It is expected that both attributes are defined, string-typed, and default 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, then
-   * finally {@code default}. In all cases the return value is a target version value (either {@code
-   * PY2} or {@code PY3}).
+   * 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.
    *
    * @throws IllegalArgumentException if the attributes are not present, not string-typed, or not
-   *     parsable as target {@link PythonVersion} values or as the sentinel value; or if {@code
-   *     default} is not a target version value
+   *     parsable as target {@link PythonVersion} values or as the sentinel value
    */
-  public static PythonVersion readPythonVersionFromAttributes(
-      AttributeMap attrs, PythonVersion defaultVersion) {
-    Preconditions.checkArgument(defaultVersion.isTargetValue());
+  @Nullable
+  public static PythonVersion readPythonVersionFromAttributes(AttributeMap attrs) {
     PythonVersion pythonVersionAttr =
         PythonVersion.parseTargetOrSentinelValue(attrs.get(PYTHON_VERSION_ATTRIBUTE, Type.STRING));
     PythonVersion defaultPythonVersionAttr =
@@ -91,7 +90,7 @@ public static PythonVersion readPythonVersionFromAttributes(
     } else if (defaultPythonVersionAttr != PythonVersion._INTERNAL_SENTINEL) {
       return defaultPythonVersionAttr;
     } else {
-      return defaultVersion;
+      return null;
     }
   }
 
@@ -158,7 +157,7 @@ public void collectMetadataArtifacts(Iterable<Artifact> artifacts,
    *
    * <p>Null if no 2to3 conversion is required.
    */
-  private final Map<PathFragment, Artifact> convertedFiles;
+  @Nullable private final Map<PathFragment, Artifact> convertedFiles;
 
   private Artifact executable = null;
 
@@ -345,6 +344,7 @@ private static boolean initHasPy3OnlySources(
    */
   // TODO(#1393): 2to3 conversion doesn't work in Bazel and the attempt to invoke it for Bazel
   // should be removed / factored away into PythonSemantics.
+  @Nullable
   private static Map<PathFragment, Artifact> makeAndInitConvertedFiles(
       RuleContext ruleContext, PythonVersion version, PythonVersion sourcesVersion) {
     if (sourcesVersion == PythonVersion.PY2 && version == PythonVersion.PY3) {

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

@@ -17,7 +17,6 @@
 import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet;
 import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.packages.RawAttributeMapper;
-import com.google.devtools.build.lib.packages.RuleClass;
 import com.google.devtools.build.lib.packages.RuleTransitionFactory;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.util.FileType;
@@ -43,19 +42,19 @@ 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 makes a transition to set the Python version to the value specified by the
-   * rule's {@code python_version} attribute if it is given, or otherwise the {@code
-   * default_python_version} attribute if it is given, or otherwise the default value passed into
-   * this function.
+   * <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 throws {@link IllegalArgumentException} if used on a rule whose {@link
-   * RuleClass} does not define both attributes. If both are defined, but one of their values cannot
-   * be parsed as a Python version, the given default value is used as a fallback instead; in this
-   * case 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 is defined correctly.
+   * <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.
    */
-  public static RuleTransitionFactory makeVersionTransition(PythonVersion defaultVersion) {
+  public static RuleTransitionFactory makeVersionTransition(
+      PythonVersionTransition defaultTransition) {
     return (rule) -> {
       AttributeMap attrs = RawAttributeMapper.of(rule);
       // Fail fast if we're used on an ill-defined rule class.
@@ -69,20 +68,25 @@ public static RuleTransitionFactory makeVersionTransition(PythonVersion defaultV
       // 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.
-      PythonVersion version;
+      PythonVersionTransition transition;
       try {
-        version = PyCommon.readPythonVersionFromAttributes(attrs, defaultVersion);
+        PythonVersion versionFromAttributes = PyCommon.readPythonVersionFromAttributes(attrs);
+        if (versionFromAttributes == null) {
+          transition = defaultTransition;
+        } else {
+          transition = PythonVersionTransition.toConstant(versionFromAttributes);
+        }
       } catch (IllegalArgumentException ex) {
-        version = defaultVersion;
+        transition = defaultTransition;
       }
-      return new PythonVersionTransition(version);
+      return transition;
     };
   }
 
   /**
    * A Python version transition that sets the version as specified by the target's attributes, with
-   * a default of {@link PythonVersion#DEFAULT_TARGET_VALUE}.
+   * a default determined by {@link PythonOptions#getDefaultPythonVersion}.
    */
   public static final RuleTransitionFactory VERSION_TRANSITION =
-      makeVersionTransition(PythonVersion.DEFAULT_TARGET_VALUE);
+      makeVersionTransition(PythonVersionTransition.toDefault());
 }

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

@@ -103,6 +103,12 @@ public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOption
           Event.error(
               "`--force_python` is disabled by `--incompatible_remove_old_python_version_api`"));
     }
+    if (opts.incompatiblePy3IsDefault && !opts.incompatibleAllowPythonVersionTransitions) {
+      reporter.handle(
+          Event.error(
+              "cannot enable `--incompatible_py3_is_default` without also enabling "
+                  + "`--incompatible_allow_python_version_transitions`"));
+    }
   }
 
   /** Returns whether to build the executable zip file for Python binaries. */

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

@@ -32,9 +32,9 @@
 /**
  * Python-related command-line options.
  *
- * <p>Due to the migration from the old Python version API to the new (see #6583), the Python major
- * version mode ({@code PY2} vs {@code PY3}) is a function of multiple flags. See {@link
- * #getPythonVersion} for more details.
+ * <p>Due to the migration of the Python version API (see #6583) and the default Python version (see
+ * (see #6647), the Python major version mode ({@code PY2} vs {@code PY3}) is a function of multiple
+ * flags. See {@link #getPythonVersion} for more details.
  */
 public class PythonOptions extends FragmentOptions {
 
@@ -99,6 +99,29 @@ public String getTypeDescription() {
               + "the documentation for `py_binary`'s `python_version` attribute.")
   public boolean incompatibleAllowPythonVersionTransitions;
 
+  /**
+   * Native rule logic should call {@link #getDefaultPythonVersion} instead of accessing this option
+   * directly.
+   */
+  @Option(
+      name = "incompatible_py3_is_default",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
+      effectTags = {
+        OptionEffectTag.LOADING_AND_ANALYSIS,
+        OptionEffectTag.AFFECTS_OUTPUTS // because of "-py2"/"-py3" output root
+      },
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help =
+          "If true, `py_binary` and `py_test` targets that do not set their `python_version` (or "
+              + "`default_python_version`) attribute will default to PY3 rather than to PY2. It is "
+              + "an error to set this flag without also enabling "
+              + "`--incompatible_allow_python_version_transitions`.")
+  public boolean incompatiblePy3IsDefault;
+
   /**
    * This field should be either null (unset), {@code PY2}, or {@code PY3}. Other {@code
    * PythonVersion} values do not represent distinct Python versions and are not allowed.
@@ -114,7 +137,7 @@ public String getTypeDescription() {
       documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
       effectTags = {
         OptionEffectTag.LOADING_AND_ANALYSIS,
-        OptionEffectTag.AFFECTS_OUTPUTS // because of "-py3" output root
+        OptionEffectTag.AFFECTS_OUTPUTS // because of "-py2"/"-py3" output root
       },
       help =
           "The Python major version mode, either `PY2` or `PY3`. Note that under the new version "
@@ -155,7 +178,7 @@ public String getTypeDescription() {
    * This field should be either null (unset), {@code PY2}, or {@code PY3}. Other {@code
    * PythonVersion} values do not represent distinct Python versions and are not allowed.
    *
-   * <p>Null is treated the same as the default ({@link PythonVersion#DEFAULT_TARGET_VALUE}).
+   * <p>Null means to use the default ({@link #getDefaultPythonVersion}).
    *
    * <p>This option is only read by {@link #getHost}. It should not be read by other native code or
    * by {@code select()}s in user code.
@@ -166,9 +189,7 @@ public String getTypeDescription() {
       converter = TargetPythonVersionConverter.class,
       documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
-      help =
-          "Overrides default_python_version attribute for the host configuration."
-              + " Can be \"PY2\" or \"PY3\".")
+      help = "Overrides the Python version for the host configuration. Can be \"PY2\" or \"PY3\".")
   public PythonVersion hostForcePython;
 
   private static final OptionDefinition HOST_FORCE_PYTHON_DEFINITION =
@@ -189,6 +210,16 @@ public String getTypeDescription() {
               + "Python target will be an error.")
   public boolean incompatibleDisallowLegacyPyProvider;
 
+  @Option(
+      name = "experimental_build_transitive_python_runfiles",
+      defaultValue = "true",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
+      help =
+          "Build the runfiles trees of py_binary targets that appear in the transitive "
+              + "data runfiles of another binary.")
+  public boolean buildTransitiveRunfilesTrees;
+
   @Override
   public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
     // TODO(brandjon): Instead of referencing the python_version target, whose path depends on the
@@ -214,19 +245,27 @@ public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
     return restrictions.build();
   }
 
+  /**
+   * Returns the Python major version ({@code PY2} or {@code PY3}) that targets that do not specify
+   * a version should be built for.
+   */
+  public PythonVersion getDefaultPythonVersion() {
+    return incompatiblePy3IsDefault ? PythonVersion.PY3 : PythonVersion.PY2;
+  }
+
   /**
    * Returns the Python major version ({@code PY2} or {@code PY3}) that targets should be built for.
    *
    * <p>The version is taken as the value of {@code --python_version} if not null, otherwise {@code
-   * --force_python} if not null, otherwise {@link PythonVersion#DEFAULT_TARGET_VALUE}.
+   * --force_python} if not null, otherwise {@link #getDefaultPythonVersion}.
    */
   public PythonVersion getPythonVersion() {
     if (pythonVersion != null) {
       return pythonVersion;
     } else if (forcePython != null) {
       return forcePython;
     } else {
-      return PythonVersion.DEFAULT_TARGET_VALUE;
+      return getDefaultPythonVersion();
     }
   }
 
@@ -257,7 +296,7 @@ public boolean canTransitionPythonVersion(PythonVersion version) {
       return currentVersionNeedsUpdating || forcePythonNeedsUpdating;
     } else {
       boolean currentlyUnset = forcePython == null && pythonVersion == null;
-      boolean transitioningToNonDefault = !version.equals(PythonVersion.DEFAULT_TARGET_VALUE);
+      boolean transitioningToNonDefault = !version.equals(getDefaultPythonVersion());
       return currentlyUnset && transitioningToNonDefault;
     }
   }
@@ -294,20 +333,11 @@ public FragmentOptions getHost() {
     hostPythonOptions.incompatibleAllowPythonVersionTransitions =
         incompatibleAllowPythonVersionTransitions;
     PythonVersion hostVersion =
-        (hostForcePython != null) ? hostForcePython : PythonVersion.DEFAULT_TARGET_VALUE;
+        (hostForcePython != null) ? hostForcePython : getDefaultPythonVersion();
     hostPythonOptions.setPythonVersion(hostVersion);
+    hostPythonOptions.incompatiblePy3IsDefault = incompatiblePy3IsDefault;
     hostPythonOptions.buildPythonZip = buildPythonZip;
     hostPythonOptions.incompatibleDisallowLegacyPyProvider = incompatibleDisallowLegacyPyProvider;
     return hostPythonOptions;
   }
-
-  @Option(
-      name = "experimental_build_transitive_python_runfiles",
-      defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
-      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
-      help =
-          "Build the runfiles trees of py_binary targets that appear in the transitive "
-              + "data runfiles of another binary.")
-  public boolean buildTransitiveRunfilesTrees;
 }

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

@@ -77,7 +77,8 @@ public enum PythonVersion {
    * ({@code native.existing_rules()}) or bazel query, we give it the scary "_internal" prefix
    * instead.
    *
-   * <p>The logical meaning of this value is the same as {@link #DEFAULT_TARGET_VALUE}.
+   * <p>The logical meaning of this value is the same as {@link
+   * PythonOptions#getDefaultPythonVersion}.
    */
   _INTERNAL_SENTINEL;
 
@@ -118,8 +119,6 @@ private static ImmutableList<String> convertToStrings(List<PythonVersion> values
   public static final ImmutableList<String> NON_CONVERSION_STRINGS =
       convertToStrings(NON_CONVERSION_VALUES);
 
-  public static final PythonVersion DEFAULT_TARGET_VALUE = PY2;
-
   public static final PythonVersion DEFAULT_SRCS_VALUE = PY2AND3;
 
   /** Returns whether or not this value is a distinct Python version. */