Skip to content

Commit

Permalink
Add Py2/3 mode migration flags to --all_incompatible_changes
Browse files Browse the repository at this point in the history
This makes available --incompatible_allow_python_version_transitions and --incompatible_remove_old_python_version_api, which were previously named "--experimental_...".

See #7307 and #7308 respectively to track the migration for each flag.

Work toward #6583, #7307, and #7308.

RELNOTES[INC]: Two changes to native Python rules: 1) `default_python_version` and `--force_python` are deprecated; use `python_version` and `--python_version` respectively instead. You can preview the removal of the deprecated names with --incompatible_remove_old_python_version_api. See [#7308](#7308). 2) The version flag will no longer override the declared version of a `py_binary` or `py_test` target. You can preview this new behavior with --incompatible_allow_python_version_transitions. See [#7307](#7307).

PiperOrigin-RevId: 231980615
  • Loading branch information
brandjon authored and Copybara-Service committed Feb 1, 2019
1 parent b9418ee commit fc4c1e7
Show file tree
Hide file tree
Showing 15 changed files with 100 additions and 94 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -106,10 +106,10 @@ executable Python rule (<code>py_binary</code> or <code>py_test</code>).
and should be avoided.
<p>Under the old semantics
(<code>--experimental_allow_python_version_transitions=false</code>), it is an error to
(<code>--incompatible_allow_python_version_transitions=false</code>), it is an error to
build any Python target for a version disallowed by its <code>srcs_version</code>
attribute. Under the new semantics
(<code>--experimental_allow_python_version_transitions=true</code>), this check is
(<code>--incompatible_allow_python_version_transitions=true</code>), this check is
deferred to the executable rule: You can build a <code>srcs_version = "PY3"</code>
<code>py_library</code> target for Python 2, but you cannot actually depend on it via
<code>deps</code> from a Python 3 <code>py_binary</code>.
Expand Down Expand Up @@ -170,7 +170,7 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen
.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>--experimental_remove_old_python_version_api</code>. For migration
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 --> */
Expand All @@ -186,13 +186,13 @@ Whether to build this target (and its transitive <code>deps</code>) for Python 2
3. Valid values are <code>"PY2"</code> (the default) and <code>"PY3"</code>.
<p>Under the old semantics
(<code>--experimental_allow_python_version_transitions=false</code>), the Python version
(<code>--incompatible_allow_python_version_transitions=false</code>), the Python version
generally cannot be changed once set. This means that the <code>--python_version</code>
flag overrides this attribute, and other Python binaries in the <code>data</code> deps of
this target are forced to use the same version as this target.
<p>Under the new semantics
(<code>--experimental_allow_python_version_transitions=true</code>), the Python version
(<code>--incompatible_allow_python_version_transitions=true</code>), the Python version
is always set (possibly by default) to whatever version is specified by this attribute,
regardless of the version specified on the command line or by other targets that depend on
this one.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -361,7 +361,7 @@ private static Map<PathFragment, Artifact> makeAndInitConvertedFiles(

/**
* Under the old version semantics ({@code
* --experimental_allow_python_version_transitions=false}), checks that the {@code srcs_version}
* --incompatible_allow_python_version_transitions=false}), checks that the {@code srcs_version}
* attribute is compatible with the Python version as determined by the configuration.
*
* <p>A failure is reported as a rule error.
Expand Down Expand Up @@ -438,7 +438,7 @@ private void validateOldVersionAttrNotUsedIfDisabled() {
ruleContext.attributeError(
DEFAULT_PYTHON_VERSION_ATTRIBUTE,
"the 'default_python_version' attribute is disabled by the "
+ "'--experimental_remove_old_python_version_api' flag");
+ "'--incompatible_remove_old_python_version_api' flag");
}
}

Expand All @@ -464,7 +464,7 @@ private void validateLegacyProviderNotUsedIfDisabled() {
}

/**
* Under the new version semantics ({@code --experimental_allow_python_version_transitions=true}),
* Under the new version semantics ({@code --incompatible_allow_python_version_transitions=true}),
* if the Python version (as determined by the configuration) is inconsistent with {@link
* #hasPy2OnlySources} or {@link #hasPy3OnlySources}, emits a {@link FailAction} that "generates"
* the executable.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -98,10 +98,10 @@ public String getOutputDirectoryName() {
@Override
public void reportInvalidOptions(EventHandler reporter, BuildOptions buildOptions) {
PythonOptions opts = buildOptions.get(PythonOptions.class);
if (opts.forcePython != null && opts.experimentalRemoveOldPythonVersionApi) {
if (opts.forcePython != null && opts.incompatibleRemoveOldPythonVersionApi) {
reporter.handle(
Event.error(
"`--force_python` is disabled by `--experimental_remove_old_python_version_api`"));
"`--force_python` is disabled by `--incompatible_remove_old_python_version_api`"));
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,8 +38,8 @@ public PythonConfiguration create(BuildOptions buildOptions)
pythonVersion,
pythonOptions.buildPythonZip,
pythonOptions.buildTransitiveRunfilesTrees,
/*oldPyVersionApiAllowed=*/ !pythonOptions.experimentalRemoveOldPythonVersionApi,
/*useNewPyVersionSemantics=*/ pythonOptions.experimentalAllowPythonVersionTransitions,
/*oldPyVersionApiAllowed=*/ !pythonOptions.incompatibleRemoveOldPythonVersionApi,
/*useNewPyVersionSemantics=*/ pythonOptions.incompatibleAllowPythonVersionTransitions,
/*disallowLegacyPyProvider=*/ pythonOptions.incompatibleDisallowLegacyPyProvider);
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -70,28 +70,34 @@ public String getTypeDescription() {
public TriState buildPythonZip;

@Option(
name = "experimental_remove_old_python_version_api",
name = "incompatible_remove_old_python_version_api",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
help =
"If true, disables use of the `--force_python` flag and 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 experimentalRemoveOldPythonVersionApi;
public boolean incompatibleRemoveOldPythonVersionApi;

@Option(
name = "experimental_allow_python_version_transitions",
name = "incompatible_allow_python_version_transitions",
defaultValue = "false",
documentationCategory = OptionDocumentationCategory.SKYLARK_SEMANTICS,
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS},
metadataTags = {OptionMetadataTag.EXPERIMENTAL},
metadataTags = {
OptionMetadataTag.INCOMPATIBLE_CHANGE,
OptionMetadataTag.TRIGGERED_BY_ALL_INCOMPATIBLE_CHANGES
},
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 experimentalAllowPythonVersionTransitions;
public boolean incompatibleAllowPythonVersionTransitions;

/**
* This field should be either null (unset), {@code PY2}, or {@code PY3}. Other {@code
Expand All @@ -112,7 +118,7 @@ public String getTypeDescription() {
},
help =
"The Python major version mode, either `PY2` or `PY3`. Note that under the new version "
+ "semantics (`--experimental_allow_python_version_transitions`) this is overridden "
+ "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;
Expand All @@ -124,7 +130,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>This flag is not accessible to the user when {@link #experimentalRemoveOldPythonVersionApi}
* <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
Expand All @@ -139,7 +145,7 @@ public String getTypeDescription() {
effectTags = {OptionEffectTag.LOADING_AND_ANALYSIS, OptionEffectTag.AFFECTS_OUTPUTS},
help =
"Deprecated alias for `--python_version`. Disabled by "
+ "`--experimental_remove_old_python_version_api`.")
+ "`--incompatible_remove_old_python_version_api`.")
public PythonVersion forcePython;

private static final OptionDefinition FORCE_PYTHON_DEFINITION =
Expand Down Expand Up @@ -193,7 +199,7 @@ public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
new SelectRestriction(
/*visibleWithinToolsPackage=*/ true,
"Use @bazel_tools//python/tools:python_version instead."));
if (experimentalRemoveOldPythonVersionApi) {
if (incompatibleRemoveOldPythonVersionApi) {
restrictions.put(
FORCE_PYTHON_DEFINITION,
new SelectRestriction(
Expand Down Expand Up @@ -227,14 +233,14 @@ public PythonVersion getPythonVersion() {
/**
* Returns whether a Python version transition to {@code version} is allowed and not a no-op.
*
* <p>Under the new semantics ({@link #experimentalAllowPythonVersionTransitions} is true),
* <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. However, to improve compatibility for unmigrated {@code
* select()}s that depend on {@code "force_python"}, if the old API is still enabled then
* transitioning is still done whenever {@link #forcePython} is not in agreement with the
* requested version, even if {@link #getPythonVersion}'s value would be unaffected.
*
* <p>Under the old semantics ({@link #experimentalAllowPythonVersionTransitions} is false),
* <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
Expand All @@ -244,10 +250,10 @@ public PythonVersion getPythonVersion() {
*/
public boolean canTransitionPythonVersion(PythonVersion version) {
Preconditions.checkArgument(version.isTargetValue());
if (experimentalAllowPythonVersionTransitions) {
if (incompatibleAllowPythonVersionTransitions) {
boolean currentVersionNeedsUpdating = !version.equals(getPythonVersion());
boolean forcePythonNeedsUpdating =
!experimentalRemoveOldPythonVersionApi && !version.equals(forcePython);
!incompatibleRemoveOldPythonVersionApi && !version.equals(forcePython);
return currentVersionNeedsUpdating || forcePythonNeedsUpdating;
} else {
boolean currentlyUnset = forcePython == null && pythonVersion == null;
Expand All @@ -264,7 +270,7 @@ public boolean canTransitionPythonVersion(PythonVersion version) {
* 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 #experimentalAllowPythonVersionTransitions} is
* <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
Expand All @@ -284,9 +290,9 @@ public void setPythonVersion(PythonVersion version) {
@Override
public FragmentOptions getHost() {
PythonOptions hostPythonOptions = (PythonOptions) getDefault();
hostPythonOptions.experimentalRemoveOldPythonVersionApi = experimentalRemoveOldPythonVersionApi;
hostPythonOptions.experimentalAllowPythonVersionTransitions =
experimentalAllowPythonVersionTransitions;
hostPythonOptions.incompatibleRemoveOldPythonVersionApi = incompatibleRemoveOldPythonVersionApi;
hostPythonOptions.incompatibleAllowPythonVersionTransitions =
incompatibleAllowPythonVersionTransitions;
PythonVersion hostVersion =
(hostForcePython != null) ? hostForcePython : PythonVersion.DEFAULT_TARGET_VALUE;
hostPythonOptions.setPythonVersion(hostVersion);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@ public void srcsVersionClashesWithForcePythonFlagUnderOldSemantics() throws Exce
// 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("--experimental_allow_python_version_transitions=false", "--force_python=PY3");
useConfiguration("--incompatible_allow_python_version_transitions=false", "--force_python=PY3");
checkError("pkg", "foo",
// error:
"'//pkg:foo' can only be used with Python 2",
Expand Down Expand Up @@ -106,7 +106,7 @@ public void versionIs3IfForcedByFlagUnderOldSemantics() throws Exception {
// would only make sense for py_library; see PyLibraryConfiguredTargetTest for the analogous
// test.
ensureDefaultIsPY2();
useConfiguration("--experimental_allow_python_version_transitions=false", "--force_python=PY3");
useConfiguration("--incompatible_allow_python_version_transitions=false", "--force_python=PY3");
scratch.file(
"pkg/BUILD", //
ruleName + "(",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -51,15 +51,15 @@ private void declareBinDependingOnLibWithVersions(String binVersion, String libS
@Test
public void python2WithPy3SrcsVersionDependency_OldSemantics() throws Exception {
reporter.removeHandler(failFastHandler); // expect errors
useConfiguration("--experimental_allow_python_version_transitions=false");
useConfiguration("--incompatible_allow_python_version_transitions=false");
declareBinDependingOnLibWithVersions("PY2", "PY3");
assertThat(view.hasErrors(getConfiguredTarget("//pkg:bin"))).isTrue();
assertContainsEvent("//pkg:lib: Rule '//pkg:lib' can only be used with Python 3");
}

@Test
public void python2WithPy3SrcsVersionDependency_NewSemantics() throws Exception {
useConfiguration("--experimental_allow_python_version_transitions=true");
useConfiguration("--incompatible_allow_python_version_transitions=true");
declareBinDependingOnLibWithVersions("PY2", "PY3");
assertThat(getPyExecutableDeferredError("//pkg:bin"))
.contains("being built for Python 2 but (transitively) includes Python 3-only sources");
Expand All @@ -68,15 +68,15 @@ public void python2WithPy3SrcsVersionDependency_NewSemantics() throws Exception
@Test
public void python2WithPy3OnlySrcsVersionDependency_OldSemantics() throws Exception {
reporter.removeHandler(failFastHandler); // expect errors
useConfiguration("--experimental_allow_python_version_transitions=false");
useConfiguration("--incompatible_allow_python_version_transitions=false");
declareBinDependingOnLibWithVersions("PY2", "PY3ONLY");
assertThat(view.hasErrors(getConfiguredTarget("//pkg:bin"))).isTrue();
assertContainsEvent("//pkg:lib: Rule '//pkg:lib' can only be used with Python 3");
}

@Test
public void python2WithPy3OnlySrcsVersionDependency_NewSemantics() throws Exception {
useConfiguration("--experimental_allow_python_version_transitions=true");
useConfiguration("--incompatible_allow_python_version_transitions=true");
declareBinDependingOnLibWithVersions("PY2", "PY3ONLY");
assertThat(getPyExecutableDeferredError("//pkg:bin"))
.contains("being built for Python 2 but (transitively) includes Python 3-only sources");
Expand All @@ -85,15 +85,15 @@ public void python2WithPy3OnlySrcsVersionDependency_NewSemantics() throws Except
@Test
public void python3WithPy2OnlySrcsVersionDependency_OldSemantics() throws Exception {
reporter.removeHandler(failFastHandler); // expect errors
useConfiguration("--experimental_allow_python_version_transitions=false");
useConfiguration("--incompatible_allow_python_version_transitions=false");
declareBinDependingOnLibWithVersions("PY3", "PY2ONLY");
assertThat(view.hasErrors(getConfiguredTarget("//pkg:bin"))).isTrue();
assertContainsEvent("//pkg:lib: Rule '//pkg:lib' can only be used with Python 2");
}

@Test
public void python3WithPy2OnlySrcsVersionDependency_NewSemantics() throws Exception {
useConfiguration("--experimental_allow_python_version_transitions=true");
useConfiguration("--incompatible_allow_python_version_transitions=true");
declareBinDependingOnLibWithVersions("PY3", "PY2ONLY");
assertThat(getPyExecutableDeferredError("//pkg:bin"))
.contains("being built for Python 3 but (transitively) includes Python 2-only sources");
Expand Down
Loading

0 comments on commit fc4c1e7

Please sign in to comment.