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
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
index c5e8d93..9acc4cf 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfiguration.java
@@ -90,8 +90,7 @@
* Returns the Python version to use.
*
* <p>Specified using either the {@code --python_version} flag and {@code python_version} rule
- * attribute (new API), or the {@code --force_python} flag and {@code default_python_version} rule
- * attribute (old API).
+ * attribute (new API), or the {@code default_python_version} rule attribute (old API).
*/
public PythonVersion getPythonVersion() {
return version;
@@ -135,11 +134,6 @@
@Override
public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
PythonOptions opts = buildOptions.get(PythonOptions.class);
- if (opts.forcePython != null && opts.incompatibleRemoveOldPythonVersionApi) {
- reporter.handle(
- Event.error(
- "`--force_python` is disabled by `--incompatible_remove_old_python_version_api`"));
- }
if (opts.incompatiblePy3IsDefault && !opts.incompatibleAllowPythonVersionTransitions) {
reporter.handle(
Event.error(
@@ -168,10 +162,7 @@
return buildTransitiveRunfilesTrees;
}
- /**
- * Returns whether use of {@code --force_python} flag and {@code default_python_version} attribute
- * is allowed.
- */
+ /** Returns whether use of the {@code default_python_version} attribute is allowed. */
public boolean oldPyVersionApiAllowed() {
return oldPyVersionApiAllowed;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
index aca45d8..44b835c4 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonOptions.java
@@ -79,7 +79,7 @@
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
- "If true, disables use of the `--force_python` flag and the `default_python_version` "
+ "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`.")
@@ -169,25 +169,17 @@
OptionsParser.getOptionDefinitionByName(PythonOptions.class, "python_version");
/**
- * 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.
+ * Deprecated machinery for setting the Python version; will be removed soon.
*
- * <p>This flag is not accessible to the user when {@link #incompatibleRemoveOldPythonVersionApi}
- * is true.
- *
- * <p>Native rule logic should call {@link #getPythonVersion} / {@link #setPythonVersion} instead
- * of accessing this option directly. BUILD/.bzl code should {@code select()} on {@code <tools
- * repo>//tools/python:python_version} rather than on this option directly.
+ * <p>Not in GraveyardOptions because we still want to prohibit users from select()ing on it.
*/
@Option(
name = "force_python",
defaultValue = "null",
converter = TargetPythonVersionConverter.class,
- documentationCategory = OptionDocumentationCategory.OUTPUT_PARAMETERS,
+ documentationCategory = OptionDocumentationCategory.UNDOCUMENTED,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
- help =
- "Deprecated alias for `--python_version`. Disabled by "
- + "`--incompatible_remove_old_python_version_api`.")
+ help = "No-op, will be removed soon.")
public PythonVersion forcePython;
private static final OptionDefinition FORCE_PYTHON_DEFINITION =
@@ -322,14 +314,12 @@
/**
* 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 #getDefaultPythonVersion}.
+ * <p>The version is taken as the value of {@code --python_version} if not null, otherwise {@link
+ * #getDefaultPythonVersion}.
*/
public PythonVersion getPythonVersion() {
if (pythonVersion != null) {
return pythonVersion;
- } else if (forcePython != null) {
- return forcePython;
} else {
return getDefaultPythonVersion();
}
@@ -343,10 +333,10 @@
* 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 #forcePython}
- * or {@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.
+ * 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}
*/
@@ -355,7 +345,7 @@
if (incompatibleAllowPythonVersionTransitions) {
return !version.equals(getPythonVersion());
} else {
- boolean currentlyUnset = forcePython == null && pythonVersion == null;
+ boolean currentlyUnset = pythonVersion == null;
boolean transitioningToNonDefault = !version.equals(getDefaultPythonVersion());
return currentlyUnset && transitioningToNonDefault;
}
@@ -372,22 +362,11 @@
* <p>If the old semantics are in effect ({@link #incompatibleAllowPythonVersionTransitions} is
* false), after this method is called {@link #canTransitionPythonVersion} will return false.
*
- * <p>To help avoid breaking old-API {@code select()} expressions that check the value of {@code
- * "force_python"}, both the old and new flags are updated even though {@code --python_version}
- * takes precedence over {@code --force_python}.
- *
* @throws IllegalArgumentException if {@code version} is not {@code PY2} or {@code PY3}
*/
public void setPythonVersion(PythonVersion version) {
Preconditions.checkArgument(version.isTargetValue());
this.pythonVersion = version;
- // If the old version API is enabled, update forcePython for consistency. If the old API is
- // disabled, don't update it because 1) no one can read it anyway, and 2) updating it during
- // normalization would cause analysis-time validation of the flag to spuriously fail (it'd think
- // the user set the flag).
- if (!incompatibleRemoveOldPythonVersionApi) {
- this.forcePython = version;
- }
}
@Override
@@ -416,7 +395,7 @@
public FragmentOptions getNormalized() {
// 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 (--force_python would always be on).
+ // semantics because that breaks backwards compatibility.
PythonOptions newOptions = (PythonOptions) clone();
if (incompatibleAllowPythonVersionTransitions) {
newOptions.setPythonVersion(newOptions.getPythonVersion());
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java
index c7ba4da..d3767f6 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyBaseConfiguredTargetTestBase.java
@@ -102,7 +102,7 @@
@Test
public void versionIs3IfForcedByFlagUnderOldSemantics() throws Exception {
- // Under the old version semantics, --force_python takes precedence over the rule's own
+ // 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
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java b/src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java
index a5d3092..5cd8e8b 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PyExecutableConfiguredTargetTestBase.java
@@ -359,18 +359,11 @@
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY3"));
- // Test against both flags.
- assertPythonVersionIs_UnderNewConfigs(
+ assertPythonVersionIs_UnderNewConfig(
"//pkg:foo",
PythonVersion.PY3,
- new String[] {
- "--incompatible_allow_python_version_transitions=true",
- "--incompatible_remove_old_python_version_api=false",
- "--force_python=PY2"
- },
- new String[] {
- "--incompatible_allow_python_version_transitions=true", "--python_version=PY2"
- });
+ "--incompatible_allow_python_version_transitions=true",
+ "--python_version=PY2");
}
@Test
@@ -378,18 +371,11 @@
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY2"));
- // Test against both flags.
- assertPythonVersionIs_UnderNewConfigs(
+ assertPythonVersionIs_UnderNewConfig(
"//pkg:foo",
PythonVersion.PY2,
- new String[] {
- "--incompatible_allow_python_version_transitions=true",
- "--incompatible_remove_old_python_version_api=false",
- "--force_python=PY3"
- },
- new String[] {
- "--incompatible_allow_python_version_transitions=true", "--python_version=PY3"
- });
+ "--incompatible_allow_python_version_transitions=true",
+ "--python_version=PY3");
}
@Test
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java
index 3632960..998fc0c 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PythonConfigurationTest.java
@@ -38,29 +38,18 @@
@Test
public void invalidTargetPythonValue_NotATargetValue() {
OptionsParsingException expected =
- assertThrows(OptionsParsingException.class, () -> create("--force_python=PY2AND3"));
+ assertThrows(OptionsParsingException.class, () -> create("--python_version=PY2AND3"));
assertThat(expected).hasMessageThat().contains("Not a valid Python major version");
}
@Test
public void invalidTargetPythonValue_UnknownValue() {
OptionsParsingException expected =
- assertThrows(
- OptionsParsingException.class,
- () -> create("--force_python=BEETLEJUICE"));
+ assertThrows(OptionsParsingException.class, () -> create("--python_version=BEETLEJUICE"));
assertThat(expected).hasMessageThat().contains("Not a valid Python major version");
}
@Test
- public void oldVersionFlagGatedByIncompatibleFlag() throws Exception {
- create("--incompatible_remove_old_python_version_api=false", "--force_python=PY2");
- checkError(
- "`--force_python` is disabled by `--incompatible_remove_old_python_version_api`",
- "--incompatible_remove_old_python_version_api=true",
- "--force_python=PY2");
- }
-
- @Test
public void py3IsDefaultFlagRequiresNewSemanticsFlag() throws Exception {
checkError(
"cannot enable `--incompatible_py3_is_default` without also enabling "
@@ -102,28 +91,6 @@
}
@Test
- public void getPythonVersion_NewFlagTakesPrecedence() throws Exception {
- assumesDefaultIsPY2();
- // --force_python is superseded by --python_version.
- PythonOptions opts =
- parsePythonOptions(
- "--incompatible_remove_old_python_version_api=false",
- "--force_python=PY2",
- "--python_version=PY3");
- assertThat(opts.getPythonVersion()).isEqualTo(PythonVersion.PY3);
- }
-
- @Test
- public void getPythonVersion_FallBackOnOldFlag() throws Exception {
- assumesDefaultIsPY2();
- // --force_python is used because --python_version is absent.
- PythonOptions opts =
- parsePythonOptions(
- "--incompatible_remove_old_python_version_api=false", "--force_python=PY3");
- assertThat(opts.getPythonVersion()).isEqualTo(PythonVersion.PY3);
- }
-
- @Test
public void canTransitionPythonVersion_OldSemantics_Yes() throws Exception {
assumesDefaultIsPY2();
PythonOptions opts =
@@ -134,16 +101,10 @@
@Test
public void canTransitionPythonVersion_OldSemantics_NoBecauseAlreadySet() throws Exception {
assumesDefaultIsPY2();
- PythonOptions optsWithOldFlag =
- parsePythonOptions(
- "--incompatible_allow_python_version_transitions=false",
- "--incompatible_remove_old_python_version_api=false",
- "--force_python=PY2");
- PythonOptions optsWithNewFlag =
+ PythonOptions opts =
parsePythonOptions(
"--incompatible_allow_python_version_transitions=false", "--python_version=PY2");
- assertThat(optsWithOldFlag.canTransitionPythonVersion(PythonVersion.PY3)).isFalse();
- assertThat(optsWithNewFlag.canTransitionPythonVersion(PythonVersion.PY3)).isFalse();
+ assertThat(opts.canTransitionPythonVersion(PythonVersion.PY3)).isFalse();
}
@Test
@@ -168,23 +129,7 @@
PythonOptions opts =
parsePythonOptions(
"--incompatible_allow_python_version_transitions=true",
- // Set --force_python too, or else we fall into the "make --force_python consistent"
- // case.
"--incompatible_remove_old_python_version_api=false",
- "--force_python=PY3",
- "--python_version=PY3");
- assertThat(opts.canTransitionPythonVersion(PythonVersion.PY3)).isFalse();
- }
-
- @Test
- public void canTransitionPythonVersion_NewApi_NoEvenWhenForcePythonDisagrees() throws Exception {
- PythonOptions opts =
- parsePythonOptions(
- "--incompatible_allow_python_version_transitions=true",
- "--incompatible_remove_old_python_version_api=false",
- // Test that even though --force_python's value isn't in sync, we don't transition
- // because getPythonVersion() would be unaffected by the transition.
- "--force_python=PY2",
"--python_version=PY3");
assertThat(opts.canTransitionPythonVersion(PythonVersion.PY3)).isFalse();
}
@@ -194,10 +139,8 @@
PythonOptions opts =
parsePythonOptions(
"--incompatible_remove_old_python_version_api=false",
- "--force_python=PY2",
"--python_version=PY2");
opts.setPythonVersion(PythonVersion.PY3);
- assertThat(opts.forcePython).isEqualTo(PythonVersion.PY3);
assertThat(opts.pythonVersion).isEqualTo(PythonVersion.PY3);
}
@@ -207,7 +150,6 @@
parsePythonOptions(
"--incompatible_remove_old_python_version_api=true", "--python_version=PY2");
opts.setPythonVersion(PythonVersion.PY3);
- assertThat(opts.forcePython).isNull();
assertThat(opts.pythonVersion).isEqualTo(PythonVersion.PY3);
}
@@ -235,11 +177,6 @@
@Test
public void getHost_AppliesHostForcePython() throws Exception {
assumesDefaultIsPY2();
- PythonOptions optsWithForcePythonFlag =
- parsePythonOptions(
- "--incompatible_remove_old_python_version_api=false",
- "--force_python=PY2",
- "--host_force_python=PY3");
PythonOptions optsWithPythonVersionFlag =
parsePythonOptions("--python_version=PY2", "--host_force_python=PY3");
PythonOptions optsWithPy3IsDefaultFlag =
@@ -250,11 +187,9 @@
// It's more interesting to set the incompatible flag true and force host to PY2, than
// it is to set the flag false and force host to PY3.
"--host_force_python=PY2");
- PythonOptions hostOptsWithForcePythonFlag = (PythonOptions) optsWithForcePythonFlag.getHost();
PythonOptions hostOptsWithPythonVersionFlag =
(PythonOptions) optsWithPythonVersionFlag.getHost();
PythonOptions hostOptsWithPy3IsDefaultFlag = (PythonOptions) optsWithPy3IsDefaultFlag.getHost();
- assertThat(hostOptsWithForcePythonFlag.getPythonVersion()).isEqualTo(PythonVersion.PY3);
assertThat(hostOptsWithPythonVersionFlag.getPythonVersion()).isEqualTo(PythonVersion.PY3);
assertThat(hostOptsWithPy3IsDefaultFlag.getPythonVersion()).isEqualTo(PythonVersion.PY2);
}
diff --git a/src/test/java/com/google/devtools/build/lib/rules/python/PythonVersionSelectTest.java b/src/test/java/com/google/devtools/build/lib/rules/python/PythonVersionSelectTest.java
index 413b483..6b39058 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/python/PythonVersionSelectTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/python/PythonVersionSelectTest.java
@@ -72,6 +72,8 @@
public void canSelectOnForcePythonFlagsUnderOldApi() throws Exception {
// For backwards compatibility purposes, select()-ing on --force_python and --host_force_python
// is allowed while the old API is still enabled.
+ // TODO(brandjon): Although --force_python is a no-op, this test is still present because the
+ // behavior belongs to the remove-old-api flag. Delete the test when we no-op that flag too.
useConfiguration("--incompatible_remove_old_python_version_api=false");
scratch.file("fp/BUILD", makeFooThatSelectsOnFlag("force_python", "PY2"));
scratch.file("hfp/BUILD", makeFooThatSelectsOnFlag("host_force_python", "PY2"));
@@ -119,7 +121,7 @@
" }),",
")");
- // Neither --python_version nor --force_python, use default value.
+ // No --python_version, use default value.
doTestSelectOnPythonVersionTarget(py2, "--incompatible_py3_is_default=false");
doTestSelectOnPythonVersionTarget(
py3,
@@ -128,17 +130,9 @@
// enabled before we're allowed to set the default to PY3.
"--incompatible_allow_python_version_transitions=true");
- // No --python_version, trust --force_python.
- doTestSelectOnPythonVersionTarget(py2, "--force_python=PY2");
- doTestSelectOnPythonVersionTarget(py3, "--force_python=PY3");
-
- // --python_version overrides --force_python.
+ // --python_version is given, use it.
doTestSelectOnPythonVersionTarget(py2, "--python_version=PY2");
- doTestSelectOnPythonVersionTarget(py2, "--python_version=PY2", "--force_python=PY2");
- doTestSelectOnPythonVersionTarget(py2, "--python_version=PY2", "--force_python=PY3");
doTestSelectOnPythonVersionTarget(py3, "--python_version=PY3");
- doTestSelectOnPythonVersionTarget(py3, "--python_version=PY3", "--force_python=PY2");
- doTestSelectOnPythonVersionTarget(py3, "--python_version=PY3", "--force_python=PY3");
}
private void doTestSelectOnPythonVersionTarget(Artifact expected, String... flags)
