Add incompatible flag to make PY2 outputs suffixed

This adds --incompatible_py2_outputs_are_suffixed, which makes it so Python 2 targets get built under an output root with the "-py2" suffix, and Python 3 targets get built under a root with no additional suffix. This means that the `bazel-bin` symlink contains Python 3 targets instead of Python 2 targets.

This goes hand-in-hand with --incompatible_py3_is_default, which controls whether targets get built in the Python 2 or Python 3 configuration by default. Enabling just one flag or the other will cause the majority of targets to get built under an output root other than the one pointed to by `bazel-bin`, which is not very convenient.

In general users and scripts are not supposed to care what the exact output path of a target is, but in practice they do, hence why this is an incompatible change.

Work toward #6647. See #7593 for migration info.

RELNOTES[INC]: Added --incompatible_py2_outputs_are_suffixed, for switching the bazel-bin symlink to point to Python 3 outputs instead of Python 2 outputs. See [#7593](#7593).

PiperOrigin-RevId: 236207234

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

@@ -14,7 +14,6 @@
 
 package com.google.devtools.build.lib.rules.python;
 
-import com.google.common.base.Ascii;
 import com.google.common.base.Preconditions;
 import com.google.common.base.Verify;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
@@ -47,6 +46,9 @@ public class PythonConfiguration extends BuildConfiguration.Fragment {
   private final boolean oldPyVersionApiAllowed;
   private final boolean useNewPyVersionSemantics;
 
+  // TODO(brandjon): Remove this once migration to PY3-as-default is complete.
+  private final boolean py2OutputsAreSuffixed;
+
   // TODO(brandjon): Remove this once migration to the new provider is complete (#7010).
   private final boolean disallowLegacyPyProvider;
 
@@ -57,13 +59,15 @@ public class PythonConfiguration extends BuildConfiguration.Fragment {
       boolean buildTransitiveRunfilesTrees,
       boolean oldPyVersionApiAllowed,
       boolean useNewPyVersionSemantics,
+      boolean py2OutputsAreSuffixed,
       boolean disallowLegacyPyProvider) {
     this.version = version;
     this.defaultVersion = defaultVersion;
     this.buildPythonZip = buildPythonZip;
     this.buildTransitiveRunfilesTrees = buildTransitiveRunfilesTrees;
     this.oldPyVersionApiAllowed = oldPyVersionApiAllowed;
     this.useNewPyVersionSemantics = useNewPyVersionSemantics;
+    this.py2OutputsAreSuffixed = py2OutputsAreSuffixed;
     this.disallowLegacyPyProvider = disallowLegacyPyProvider;
   }
 
@@ -95,21 +99,21 @@ public PythonVersion getDefaultPythonVersion() {
 
   @Override
   public String getOutputDirectoryName() {
-    // TODO(brandjon): Implement alternative semantics for controlling which python version(s) get
-    // suffixed roots.
     Preconditions.checkState(version.isTargetValue());
-    // The only possible Python target version values are PY2 and PY3. For now, PY2 gets the normal
-    // output directory name, and PY3 gets a "-py3" suffix.
+    // The only possible Python target version values are PY2 and PY3. Historically, PY3 targets got
+    // a "-py3" suffix and PY2 targets got the empty suffix, so that the bazel-bin symlink pointed
+    // to Python 2 targets. When --incompatible_py2_outputs_are_suffixed is enabled, this is
+    // reversed: PY2 targets get "-py2" and PY3 targets get the empty suffix.
     Verify.verify(
         PythonVersion.TARGET_VALUES.size() == 2, // If there is only 1, we don't need this method.
         "Detected a change in PythonVersion.TARGET_VALUES so that there are no longer two Python "
             + "versions. Please check that PythonConfiguration#getOutputDirectoryName() is still "
             + "needed and is still able to avoid output directory clashes, then update this "
             + "canary message.");
-    if (version.equals(PythonVersion.PY2)) {
-      return null;
+    if (py2OutputsAreSuffixed) {
+      return version == PythonVersion.PY2 ? "py2" : null;
     } else {
-      return Ascii.toLowerCase(version.toString());
+      return version == PythonVersion.PY3 ? "py3" : null;
     }
   }
 

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

@@ -41,6 +41,7 @@ public PythonConfiguration create(BuildOptions buildOptions)
         pythonOptions.buildTransitiveRunfilesTrees,
         /*oldPyVersionApiAllowed=*/ !pythonOptions.incompatibleRemoveOldPythonVersionApi,
         /*useNewPyVersionSemantics=*/ pythonOptions.incompatibleAllowPythonVersionTransitions,
+        /*py2OutputsAreSuffixed=*/ pythonOptions.incompatiblePy2OutputsAreSuffixed,
         /*disallowLegacyPyProvider=*/ pythonOptions.incompatibleDisallowLegacyPyProvider);
   }
 

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

@@ -119,9 +119,28 @@ public String getTypeDescription() {
           "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`.")
+              + "`--incompatible_allow_python_version_transitions`. If you set this flag it is "
+              + "also recommended to set `--incompatible_py2_outputs_are_suffixed`.")
   public boolean incompatiblePy3IsDefault;
 
+  @Option(
+      name = "incompatible_py2_outputs_are_suffixed",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.GENERIC_INPUTS,
+      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
+      },
+      help =
+          "If true, targets built in the Python 2 configuration will appear under an output root "
+              + "that includes the suffix '-py2', while targets built for Python 3 will appear "
+              + "in a root with no Python-related suffix. This means that the `bazel-bin` "
+              + "convenience symlink will point to Python 3 targets rather than Python 2. "
+              + "If you enable this option it is also recommended to enable "
+              + "`--incompatible_py3_is_default`.")
+  public boolean incompatiblePy2OutputsAreSuffixed;
+
   /**
    * 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.
@@ -336,6 +355,7 @@ public FragmentOptions getHost() {
         (hostForcePython != null) ? hostForcePython : getDefaultPythonVersion();
     hostPythonOptions.setPythonVersion(hostVersion);
     hostPythonOptions.incompatiblePy3IsDefault = incompatiblePy3IsDefault;
+    hostPythonOptions.incompatiblePy2OutputsAreSuffixed = incompatiblePy2OutputsAreSuffixed;
     hostPythonOptions.buildPythonZip = buildPythonZip;
     hostPythonOptions.incompatibleDisallowLegacyPyProvider = incompatibleDisallowLegacyPyProvider;
     return hostPythonOptions;

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

@@ -199,12 +199,14 @@ public void getHost_CopiesMostValues() throws Exception {
             "--incompatible_allow_python_version_transitions=true",
             "--incompatible_remove_old_python_version_api=true",
             "--incompatible_py3_is_default=true",
+            "--incompatible_py2_outputs_are_suffixed=true",
             "--build_python_zip=true",
             "--incompatible_disallow_legacy_py_provider=true");
     PythonOptions hostOpts = (PythonOptions) opts.getHost();
     assertThat(hostOpts.incompatibleAllowPythonVersionTransitions).isTrue();
     assertThat(hostOpts.incompatibleRemoveOldPythonVersionApi).isTrue();
     assertThat(hostOpts.incompatiblePy3IsDefault).isTrue();
+    assertThat(hostOpts.incompatiblePy2OutputsAreSuffixed).isTrue();
     assertThat(hostOpts.buildPythonZip).isEqualTo(TriState.YES);
     assertThat(hostOpts.incompatibleDisallowLegacyPyProvider).isTrue();
   }

src/test/shell/bazel/python_version_test.sh

@@ -118,9 +118,7 @@ py_runtime(
 EOF
 }
 
-function set_up() {
-  use_system_python_2_3_runtimes
-}
+use_system_python_2_3_runtimes
 
 #### TESTS #############################################################
 
@@ -510,4 +508,58 @@ EOF
   expect_log "I am lib!"
 }
 
+# Tests that targets appear under the expected roots.
+function test_output_roots() {
+  # It's hard to get build output paths reliably, so we'll just check the output
+  # of bazel info.
+
+  # Legacy behavior, PY2 case.
+  bazel info bazel-bin \
+      --incompatible_py2_outputs_are_suffixed=false --python_version=PY2 \
+      &> $TEST_log || fail "bazel info failed"
+  expect_log "bazel-out/.*/bin"
+  expect_not_log "bazel-out/.*-py2.*/bin"
+
+  # Legacy behavior, PY3 case.
+  bazel info bazel-bin \
+      --incompatible_py2_outputs_are_suffixed=false --python_version=PY3 \
+      &> $TEST_log || fail "bazel info failed"
+  expect_log "bazel-out/.*-py3.*/bin"
+
+  # New behavior, PY2 case.
+  bazel info bazel-bin \
+      --incompatible_py2_outputs_are_suffixed=true --python_version=PY2 \
+      &> $TEST_log || fail "bazel info failed"
+  expect_log "bazel-out/.*-py2.*/bin"
+
+  # New behavior, PY3 case.
+  bazel info bazel-bin \
+      --incompatible_py2_outputs_are_suffixed=true --python_version=PY3 \
+      &> $TEST_log || fail "bazel info failed"
+  expect_log "bazel-out/.*/bin"
+  expect_not_log "bazel-out/.*-py3.*/bin"
+}
+
+# Tests that bazel-bin points to where targets get built by default (or at least
+# not to a directory with a -py2 or -py3 suffix), provided that
+# --incompatible_py3_is_default and --incompatible_py2_outputs_are_suffixed are
+# flipped together.
+function test_default_output_root_is_bazel_bin() {
+  bazel info bazel-bin \
+      --incompatible_py3_is_default=false \
+      --incompatible_py2_outputs_are_suffixed=false \
+      &> $TEST_log || fail "bazel info failed"
+  expect_log "bazel-out/.*/bin"
+  expect_not_log "bazel-out/.*-py2.*/bin"
+  expect_not_log "bazel-out/.*-py3.*/bin"
+
+  bazel info bazel-bin \
+      --incompatible_py3_is_default=true \
+      --incompatible_py2_outputs_are_suffixed=true \
+      &> $TEST_log || fail "bazel info failed"
+  expect_log "bazel-out/.*/bin"
+  expect_not_log "bazel-out/.*-py2.*/bin"
+  expect_not_log "bazel-out/.*-py3.*/bin"
+}
+
 run_suite "Tests for how the Python rules handle Python 2 vs Python 3"