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
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
index 36a1d9e..e9c357d 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuleClasses.java
@@ -163,19 +163,6 @@
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).
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
index b6fed72..2c0ae4d 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java
@@ -151,7 +151,7 @@
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();
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
index ff5ecd0..0aa9166 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyCommon.java
+++ b/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 @@
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 @@
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,56 +497,28 @@
}
/**
- * 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 @@
* </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 @@
}
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;
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java
index 1891f75..50678b06 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyRuleClasses.java
@@ -55,16 +55,15 @@
* 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) {
@@ -72,10 +71,9 @@
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
@@ -83,11 +81,11 @@
// 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;
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 ab03ab7..c8efbc4 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
@@ -40,9 +40,6 @@
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 @@
PythonVersion defaultVersion,
TriState buildPythonZip,
boolean buildTransitiveRunfilesTrees,
- boolean oldPyVersionApiAllowed,
boolean py2OutputsAreSuffixed,
boolean disallowLegacyPyProvider,
boolean useToolchains,
@@ -72,7 +68,6 @@
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 @@
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}.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java
index 805e333..3515f11 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonConfigurationLoader.java
@@ -39,7 +39,6 @@
pythonOptions.getDefaultPythonVersion(),
pythonOptions.buildPythonZip,
pythonOptions.buildTransitiveRunfilesTrees,
- /*oldPyVersionApiAllowed=*/ !pythonOptions.incompatibleRemoveOldPythonVersionApi,
/*py2OutputsAreSuffixed=*/ pythonOptions.incompatiblePy2OutputsAreSuffixed,
/*disallowLegacyPyProvider=*/ pythonOptions.incompatibleDisallowLegacyPyProvider,
/*useToolchains=*/ pythonOptions.incompatibleUsePythonToolchains,
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 02846d6..6c0e0e6 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
@@ -69,20 +69,22 @@
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 @@
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 @@
@Override
public FragmentOptions getHost() {
PythonOptions hostPythonOptions = (PythonOptions) getDefault();
- hostPythonOptions.incompatibleRemoveOldPythonVersionApi = incompatibleRemoveOldPythonVersionApi;
PythonVersion hostVersion =
(hostForcePython != null) ? hostForcePython : getDefaultPythonVersion();
hostPythonOptions.setPythonVersion(hostVersion);
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 23fd678..0de8c77 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
@@ -116,15 +116,6 @@
return String.join("\n", lines);
}
- private String ruleDeclWithDefaultPyVersionAttr(String name, String version) {
- return join(
- ruleName + "(",
- " name = '" + name + "',",
- " srcs = ['" + name + ".py'],",
- " default_python_version = '" + version + "',",
- ")");
- }
-
private String ruleDeclWithPyVersionAttr(String name, String version) {
return join(
ruleName + "(",
@@ -147,20 +138,7 @@
}
@Test
- public void oldVersionAttr_UnknownValue() throws Exception {
- useConfiguration("--incompatible_remove_old_python_version_api=false");
- checkError(
- "pkg",
- "foo",
- // error:
- "invalid value in 'default_python_version' attribute: "
- + "has to be one of 'PY2' or 'PY3' instead of 'doesnotexist'",
- // build file:
- ruleDeclWithDefaultPyVersionAttr("foo", "doesnotexist"));
- }
-
- @Test
- public void newVersionAttr_UnknownValue() throws Exception {
+ public void versionAttr_UnknownValue() throws Exception {
checkError(
"pkg",
"foo",
@@ -172,20 +150,7 @@
}
@Test
- public void oldVersionAttr_BadValue() throws Exception {
- useConfiguration("--incompatible_remove_old_python_version_api=false");
- checkError(
- "pkg",
- "foo",
- // error:
- "invalid value in 'default_python_version' attribute: "
- + "has to be one of 'PY2' or 'PY3' instead of 'PY2AND3'",
- // build file:
- ruleDeclWithDefaultPyVersionAttr("foo", "PY2AND3"));
- }
-
- @Test
- public void newVersionAttr_BadValue() throws Exception {
+ public void versionAttr_BadValue() throws Exception {
checkError(
"pkg",
"foo",
@@ -197,69 +162,13 @@
}
@Test
- public void oldVersionAttr_GoodValue() throws Exception {
- useConfiguration("--incompatible_remove_old_python_version_api=false");
- scratch.file("pkg/BUILD", ruleDeclWithDefaultPyVersionAttr("foo", "PY2"));
- getOkPyTarget("//pkg:foo");
- assertNoEvents();
- }
-
- @Test
- public void newVersionAttr_GoodValue() throws Exception {
+ public void versionAttr_GoodValue() throws Exception {
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY2"));
getOkPyTarget("//pkg:foo");
assertNoEvents();
}
@Test
- public void cannotUseOldVersionAttrWithRemovalFlag() throws Exception {
- useConfiguration("--incompatible_remove_old_python_version_api=true");
- checkError(
- "pkg",
- "foo",
- // error:
- "the 'default_python_version' attribute is disabled by the "
- + "'--incompatible_remove_old_python_version_api' flag",
- // build file:
- ruleDeclWithDefaultPyVersionAttr("foo", "PY2"));
- }
-
- /**
- * Regression test for #7071: Don't let prohibiting the old attribute get in the way of cloning a
- * target using {@code native.existing_rules()}.
- *
- * <p>The use case of cloning a target is pretty dubious and brittle. But as long as it's possible
- * and not proscribed, we won't let version attribute validation get in the way.
- */
- @Test
- public void canCopyTargetWhenOldAttrDisallowed() throws Exception {
- useConfiguration("--incompatible_remove_old_python_version_api=true");
- scratch.file(
- "pkg/rules.bzl",
- "def copy_target(rulefunc, src, dest):",
- " t = native.existing_rule(src)",
- " t.pop('kind')",
- " t.pop('name')",
- " # Also remove these because they get in the way of creating the new target but aren't",
- " # related to the attribute under test.",
- " t.pop('restricted_to')",
- " t.pop('shard_count', default=None)",
- " rulefunc(name = dest, **t)");
- scratch.file(
- "pkg/BUILD",
- "load(':rules.bzl', 'copy_target')",
- ruleName + "(",
- " name = 'foo',",
- " srcs = ['foo.py'],",
- " main = 'foo.py',",
- " python_version = 'PY2',",
- ")",
- "copy_target(" + ruleName + ", 'foo', 'bar')");
- ConfiguredTarget target = getConfiguredTarget("//pkg:bar");
- assertThat(target).isNotNull();
- }
-
- @Test
public void py3IsDefaultFlag_SetsDefaultPythonVersion() throws Exception {
scratch.file(
"pkg/BUILD", //
@@ -295,20 +204,6 @@
}
@Test
- public void newVersionAttrTakesPrecedenceOverOld() throws Exception {
- scratch.file(
- "pkg/BUILD",
- ruleName + "(",
- " name = 'foo',",
- " srcs = ['foo.py'],",
- " default_python_version = 'PY2',",
- " python_version = 'PY3',",
- ")");
- assertPythonVersionIs_UnderNewConfig(
- "//pkg:foo", PythonVersion.PY3, "--incompatible_remove_old_python_version_api=false");
- }
-
- @Test
public void versionAttrWorks_WhenNotDefaultValue() throws Exception {
assumesDefaultIsPY2();
scratch.file("pkg/BUILD", ruleDeclWithPyVersionAttr("foo", "PY3"));
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 267142fd..633e19a 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
@@ -81,20 +81,8 @@
}
@Test
- public void setPythonVersion_OldApiEnabled() throws Exception {
- PythonOptions opts =
- parsePythonOptions(
- "--incompatible_remove_old_python_version_api=false",
- "--python_version=PY2");
- opts.setPythonVersion(PythonVersion.PY3);
- assertThat(opts.pythonVersion).isEqualTo(PythonVersion.PY3);
- }
-
- @Test
- public void setPythonVersion_OldApiDisabled() throws Exception {
- PythonOptions opts =
- parsePythonOptions(
- "--incompatible_remove_old_python_version_api=true", "--python_version=PY2");
+ public void setPythonVersion() throws Exception {
+ PythonOptions opts = parsePythonOptions("--python_version=PY2");
opts.setPythonVersion(PythonVersion.PY3);
assertThat(opts.pythonVersion).isEqualTo(PythonVersion.PY3);
}
@@ -103,14 +91,12 @@
public void getHost_CopiesMostValues() throws Exception {
PythonOptions opts =
parsePythonOptions(
- "--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",
"--incompatible_use_python_toolchains=true");
PythonOptions hostOpts = (PythonOptions) opts.getHost();
- assertThat(hostOpts.incompatibleRemoveOldPythonVersionApi).isTrue();
assertThat(hostOpts.incompatiblePy3IsDefault).isTrue();
assertThat(hostOpts.incompatiblePy2OutputsAreSuffixed).isTrue();
assertThat(hostOpts.buildPythonZip).isEqualTo(TriState.YES);
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 ee54d6e..cdbe595 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
@@ -16,13 +16,11 @@
import static com.google.common.truth.Truth.assertThat;
-import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.analysis.FileProvider;
import com.google.devtools.build.lib.analysis.util.BuildViewTestCase;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
import com.google.devtools.build.lib.testutil.TestConstants;
-import java.util.Arrays;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.junit.runners.JUnit4;
@@ -68,22 +66,9 @@
makeFooThatSelectsOnFlag("python_version", "PY2"));
}
+ // TODO(brandjon): Delete this test case when we delete these flags.
@Test
- 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"));
- assertThat(getConfiguredTarget("//fp:foo")).isNotNull();
- assertThat(getConfiguredTarget("//hfp:foo")).isNotNull();
- }
-
- @Test
- public void cannotSelectOnForcePythonFlagsWithoutOldApi() throws Exception {
- useConfiguration("--incompatible_remove_old_python_version_api=true");
+ public void cannotSelectOnForcePythonFlags() throws Exception {
checkError(
"fp",
"foo",
@@ -132,12 +117,7 @@
private void doTestSelectOnPythonVersionTarget(Artifact expected, String... flags)
throws Exception {
- ImmutableList<String> modifiedFlags =
- ImmutableList.<String>builder()
- .addAll(Arrays.asList(flags))
- .add("--incompatible_remove_old_python_version_api=false")
- .build();
- useConfiguration(modifiedFlags.toArray(new String[] {}));
+ useConfiguration(flags);
NestedSet<Artifact> files =
getConfiguredTarget("//pkg:foo").getProvider(FileProvider.class).getFilesToBuild();
assertThat(files.toList()).contains(expected);
diff --git a/src/test/shell/integration/python_stub_test.sh b/src/test/shell/integration/python_stub_test.sh
index 97ad092..734e20a 100755
--- a/src/test/shell/integration/python_stub_test.sh
+++ b/src/test/shell/integration/python_stub_test.sh
@@ -175,7 +175,6 @@
# Build cc at the top level, along with the outer halves of both paths to cc.
bazel build --nobuild //test:cc //test:path_A_outer //test:path_B_outer \
- --incompatible_remove_old_python_version_api=true \
&> $TEST_log || fail "bazel run failed"
}
