Turn deprecated --force_python into a no-op

    The flag now behaves as if it is always unset. It will be removed entirely in a separate CL. In the meantime, it is still disallowed to select() on the flag.

    Work toward #7797.

    RELNOTES: None
    PiperOrigin-RevId: 302665836

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java

@@ -16,8 +16,11 @@
 
 import com.google.common.base.Preconditions;
 import com.google.common.base.Verify;
-import com.google.devtools.build.lib.analysis.config.Fragment;
+import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
+import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.events.Event;
+import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModule;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkModuleCategory;
 import com.google.devtools.build.lib.syntax.StarlarkValue;
@@ -33,13 +36,17 @@
     name = "py",
     doc = "A configuration fragment for Python.",
     category = SkylarkModuleCategory.CONFIGURATION_FRAGMENT)
-public class PythonConfiguration extends Fragment implements StarlarkValue {
+public class PythonConfiguration extends BuildConfiguration.Fragment implements StarlarkValue {
 
   private final PythonVersion version;
   private final PythonVersion defaultVersion;
   private final TriState buildPythonZip;
   private final boolean buildTransitiveRunfilesTrees;
 
+  // TODO(brandjon): Remove these once migration to the new version API is complete (#6583).
+  private final boolean oldPyVersionApiAllowed;
+  private final boolean useNewPyVersionSemantics;
+
   // TODO(brandjon): Remove this once migration to PY3-as-default is complete.
   private final boolean py2OutputsAreSuffixed;
 
@@ -59,6 +66,8 @@ public class PythonConfiguration extends Fragment implements StarlarkValue {
       PythonVersion defaultVersion,
       TriState buildPythonZip,
       boolean buildTransitiveRunfilesTrees,
+      boolean oldPyVersionApiAllowed,
+      boolean useNewPyVersionSemantics,
       boolean py2OutputsAreSuffixed,
       boolean disallowLegacyPyProvider,
       boolean useToolchains,
@@ -68,6 +77,8 @@ public class PythonConfiguration extends Fragment implements StarlarkValue {
     this.defaultVersion = defaultVersion;
     this.buildPythonZip = buildPythonZip;
     this.buildTransitiveRunfilesTrees = buildTransitiveRunfilesTrees;
+    this.oldPyVersionApiAllowed = oldPyVersionApiAllowed;
+    this.useNewPyVersionSemantics = useNewPyVersionSemantics;
     this.py2OutputsAreSuffixed = py2OutputsAreSuffixed;
     this.disallowLegacyPyProvider = disallowLegacyPyProvider;
     this.useToolchains = useToolchains;
@@ -120,6 +131,17 @@ public String getOutputDirectoryName() {
     }
   }
 
+  @Override
+  public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
+    PythonOptions opts = buildOptions.get(PythonOptions.class);
+    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. */
   public boolean buildPythonZip() {
     switch (buildPythonZip) {
@@ -140,6 +162,16 @@ 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 the new semantics should be used for transitions on the Python version. */
+  public boolean useNewPyVersionSemantics() {
+    return useNewPyVersionSemantics;
+  }
+
   /**
    * Returns true if Python rules should omit the legacy "py" provider and fail-fast when given this
    * provider from their {@code deps}.

dataset/GitHub_Java/bazelbuild.bazel/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java

@@ -69,40 +69,34 @@ 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.UNDOCUMENTED,
+      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {
         OptionMetadataTag.INCOMPATIBLE_CHANGE,
         OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
       },
-      help = "No-op, will be removed soon.")
+      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`.")
   public boolean incompatibleRemoveOldPythonVersionApi;
 
-  /**
-   * 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_allow_python_version_transitions",
       defaultValue = "true",
-      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      documentationCategory = OptionDocumentationCategory.STARLARK_SEMANTICS,
       effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
       metadataTags = {
         OptionMetadataTag.INCOMPATIBLE_CHANGE,
         OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
       },
-      help = "No-op, will be removed soon.")
+      help =
+          "If true, Python rules use the new PY2/PY3 version semantics. For more information, see "
+              + "the documentation for `py_binary`'s `python_version` attribute.")
   public boolean incompatibleAllowPythonVersionTransitions;
 
   /**
@@ -123,9 +117,10 @@ public String getTypeDescription() {
       },
       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. If "
-              + "you set this flag it is also recommended to set "
-              + "`--incompatible_py2_outputs_are_suffixed`.")
+              + "`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`. If you set this flag it is "
+              + "also recommended to set `--incompatible_py2_outputs_are_suffixed`.")
   public boolean incompatiblePy3IsDefault;
 
   @Option(
@@ -164,8 +159,9 @@ public String getTypeDescription() {
         OptionEffectTag.AFFECTS_OUTPUTS // because of "-py2"/"-py3" output root
       },
       help =
-          "The Python major version mode, either `PY2` or `PY3`. Note that this is overridden by "
-              + "`py_binary` and `py_test` targets (even if they don't explicitly specify a "
+          "The Python major version mode, either `PY2` or `PY3`. Note that under the new version "
+              + "semantics (`--incompatible_allow_python_version_transitions`) this is overridden "
+              + "by `py_binary` and `py_test` targets (even if they don't explicitly specify a "
               + "version) so there is usually not much reason to supply this flag.")
   public PythonVersion pythonVersion;
 
@@ -225,7 +221,6 @@ public String getTypeDescription() {
               + "Python target will be an error.")
   public boolean incompatibleDisallowLegacyPyProvider;
 
-  // TODO(b/153369373): Delete this flag.
   @Option(
       name = "incompatible_use_python_toolchains",
       defaultValue = "true",
@@ -241,6 +236,21 @@ public String getTypeDescription() {
               + "--python_top.")
   public boolean incompatibleUsePythonToolchains;
 
+  @Option(
+      name = "incompatible_load_python_rules_from_bzl",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
+      effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
+      metadataTags = {
+        OptionMetadataTag.INCOMPATIBLE_CHANGE,
+        OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES,
+      },
+      help =
+          "If enabled, direct use of the native Python rules is disabled. Please use the Starlark "
+              + "rules instead: https://github.com/bazelbuild/rules_python. See also "
+              + "https://github.com/bazelbuild/bazel/issues/9006.")
+  public boolean loadPythonRulesFromBzl;
+
   @Option(
       name = "experimental_build_transitive_python_runfiles",
       defaultValue = "true",
@@ -278,16 +288,18 @@ public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
         new SelectRestriction(
             /*visibleWithinToolsPackage=*/ true,
             "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."));
+    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."));
+    }
     return restrictions.build();
   }
 
@@ -314,24 +326,44 @@ public PythonVersion getPythonVersion() {
   }
 
   /**
-   * Returns whether a Python version transition to {@code version} is not a no-op.
+   * Returns whether a Python version transition to {@code version} is allowed and not a no-op.
+   *
+   * <p>Under the new semantics ({@link #incompatibleAllowPythonVersionTransitions} is true),
+   * version transitions are always allowed, so this essentially returns whether the new version is
+   * different from the existing one.
+   *
+   * <p>Under the old semantics ({@link #incompatibleAllowPythonVersionTransitions} is false),
+   * version transitions are not allowed once the version has already been set ({@link
+   * #pythonVersion} is non-null). Due to a historical bug, it is also not allowed to transition the
+   * version to the hard-coded default value. Under these constraints, there is only one transition
+   * possible, from null to the non-default value, and it is never a no-op.
    *
    * @throws IllegalArgumentException if {@code version} is not {@code PY2} or {@code PY3}
    */
   public boolean canTransitionPythonVersion(PythonVersion version) {
     Preconditions.checkArgument(version.isTargetValue());
-    return !version.equals(getPythonVersion());
+    if (incompatibleAllowPythonVersionTransitions) {
+      return !version.equals(getPythonVersion());
+    } else {
+      boolean currentlyUnset = pythonVersion == null;
+      boolean transitioningToNonDefault = !version.equals(getDefaultPythonVersion());
+      return currentlyUnset && transitioningToNonDefault;
+    }
   }
 
   /**
-   * Sets the Python version to {@code version}.
+   * Manipulates the Python version fields so that {@link #getPythonVersion()} returns {@code
+   * version}.
    *
-   * <p>Since this is a mutation, it should only be called on a newly constructed instance.
+   * <p>This method is a mutation on the current instance, so it should only be invoked on a newly
+   * constructed instance. The mutation does not depend on whether or not {@link
+   * #canTransitionPythonVersion} would return true.
+   *
+   * <p>If the old semantics are in effect ({@link #incompatibleAllowPythonVersionTransitions} is
+   * false), after this method is called {@link #canTransitionPythonVersion} will return false.
    *
    * @throws IllegalArgumentException if {@code version} is not {@code PY2} or {@code PY3}
    */
-  // TODO(brandjon): Consider removing this mutator now that the various flags and semantics it
-  // used to consider are gone. We'd revert to just setting the public option field directly.
   public void setPythonVersion(PythonVersion version) {
     Preconditions.checkArgument(version.isTargetValue());
     this.pythonVersion = version;
@@ -340,6 +372,9 @@ public void setPythonVersion(PythonVersion version) {
   @Override
   public FragmentOptions getHost() {
     PythonOptions hostPythonOptions = (PythonOptions) getDefault();
+    hostPythonOptions.incompatibleRemoveOldPythonVersionApi = incompatibleRemoveOldPythonVersionApi;
+    hostPythonOptions.incompatibleAllowPythonVersionTransitions =
+        incompatibleAllowPythonVersionTransitions;
     PythonVersion hostVersion =
         (hostForcePython != null) ? hostForcePython : getDefaultPythonVersion();
     hostPythonOptions.setPythonVersion(hostVersion);
@@ -348,6 +383,7 @@ public FragmentOptions getHost() {
     hostPythonOptions.buildPythonZip = buildPythonZip;
     hostPythonOptions.incompatibleDisallowLegacyPyProvider = incompatibleDisallowLegacyPyProvider;
     hostPythonOptions.incompatibleUsePythonToolchains = incompatibleUsePythonToolchains;
+    hostPythonOptions.loadPythonRulesFromBzl = loadPythonRulesFromBzl;
 
     // Save host options in case of a further exec->host transition.
     hostPythonOptions.hostForcePython = hostForcePython;
@@ -357,10 +393,13 @@ public FragmentOptions getHost() {
 
   @Override
   public FragmentOptions getNormalized() {
-    // We want to ensure that options with "null" physical default values are normalized, to avoid
-    // #7808.
+    // Under the new version semantics, we want to ensure that options with "null" physical default
+    // values are normalized, to avoid #7808. We don't want to normalize with the old version
+    // semantics because that breaks backwards compatibility.
     PythonOptions newOptions = (PythonOptions) clone();
-    newOptions.setPythonVersion(newOptions.getPythonVersion());
+    if (incompatibleAllowPythonVersionTransitions) {
+      newOptions.setPythonVersion(newOptions.getPythonVersion());
+    }
     return newOptions;
   }
 }

dataset/GitHub_Java/bazelbuild.bazel/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java

@@ -68,6 +68,27 @@ public void goodSrcsVersionValue() throws Exception {
     assertNoEvents();
   }
 
+  @Test
+  public void srcsVersionClashesWithVersionFlagUnderOldSemantics() throws Exception {
+    // Under the old version semantics, we fail on any Python target the moment a conflict between
+    // srcs_version and the configuration is detected. Under the new semantics, py_binary and
+    // py_test care if there's a conflict but py_library does not. This test case checks the old
+    // semantics; the new semantics are checked in PyLibraryConfiguredTargetTest and
+    // PyExecutableConfiguredTargetTestBase. Note that under the new semantics py_binary and
+    // py_library ignore the version flag, so those tests use the attribute to set the version
+    // instead.
+    useConfiguration(
+        "--incompatible_allow_python_version_transitions=false", "--python_version=PY3");
+    checkError("pkg", "foo",
+        // error:
+        "'//pkg:foo' can only be used with Python 2",
+        // build file:
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = [':foo.py'],",
+        "    srcs_version = 'PY2ONLY')");
+  }
+
   @Test
   public void versionIs2IfUnspecified() throws Exception {
     assumesDefaultIsPY2();
@@ -79,6 +100,49 @@ public void versionIs2IfUnspecified() throws Exception {
     assertThat(getPythonVersion(getConfiguredTarget("//pkg:foo"))).isEqualTo(PythonVersion.PY2);
   }
 
+  @Test
+  public void versionIs3IfForcedByFlagUnderOldSemantics() throws Exception {
+    // Under the old version semantics, --python_version takes precedence over the rule's own
+    // default_python_version attribute, so this test case applies equally well to py_library,
+    // py_binary, and py_test. Under the new semantics the rule attribute takes precedence, so this
+    // would only make sense for py_library; see PyLibraryConfiguredTargetTest for the analogous
+    // test.
+    assumesDefaultIsPY2();
+    useConfiguration(
+        "--incompatible_allow_python_version_transitions=false", "--python_version=PY3");
+    scratch.file(
+        "pkg/BUILD", //
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = ['foo.py'])");
+    assertThat(getPythonVersion(getConfiguredTarget("//pkg:foo"))).isEqualTo(PythonVersion.PY3);
+  }
+
+  @Test
+  public void packageNameCannotHaveHyphen() throws Exception {
+    checkError("pkg-hyphenated", "foo",
+        // error:
+        "paths to Python packages may not contain '-'",
+        // build file:
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = ['foo.py'])");
+  }
+
+  @Test
+  public void srcsPackageNameCannotHaveHyphen() throws Exception {
+    scratch.file(
+        "pkg-hyphenated/BUILD", //
+        "exports_files(['bar.py'])");
+    checkError("otherpkg", "foo",
+        // error:
+        "paths to Python packages may not contain '-'",
+        // build file:
+        ruleName + "(",
+        "    name = 'foo',",
+        "    srcs = ['foo.py', '//pkg-hyphenated:bar.py'])");
+  }
+
   @Test
   public void producesBothModernAndLegacyProviders_WithoutIncompatibleFlag() throws Exception {
     useConfiguration("--incompatible_disallow_legacy_py_provider=false");