Skip to content

Commit

Permalink
Always use target's attributes to set Python version (#16959)
Browse files Browse the repository at this point in the history
Fixes: #16935

RELNOTES[INC]: This changes the behavior of Python version in exec/host configuration. Mitigation is to set Python version on the targets.

PiperOrigin-RevId: 493804390
Change-Id: I3a4d787e7075d2b76835faf04d4c4e04c9de85b4

Co-authored-by: Googler <ilist@google.com>
  • Loading branch information
Wyverald and comius authored Dec 8, 2022
1 parent cccb0d6 commit 14925b5
Show file tree
Hide file tree
Showing 2 changed files with 14 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,9 @@ public String getTypeDescription() {
+ "https://github.com/bazelbuild/bazel/issues/10076.")
public boolean incompatibleDefaultToExplicitInitPy;

// Helper field to store hostForcePython in exec configuration
private PythonVersion defaultPythonVersion = null;

@Override
public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
// TODO(brandjon): Instead of referencing the python_version target, whose path depends on the
Expand All @@ -256,7 +259,7 @@ public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
restrictions.put(
PYTHON_VERSION_DEFINITION,
new SelectRestriction(
/*visibleWithinToolsPackage=*/ true,
/* visibleWithinToolsPackage= */ true,
"Use @bazel_tools//python/tools:python_version instead."));
restrictions.put(
FORCE_PYTHON_DEFINITION,
Expand All @@ -276,6 +279,9 @@ public Map<OptionDefinition, SelectRestriction> getSelectRestrictions() {
* a version should be built for.
*/
public PythonVersion getDefaultPythonVersion() {
if (defaultPythonVersion != null) {
return defaultPythonVersion;
}
return incompatiblePy3IsDefault ? PythonVersion.PY3 : PythonVersion.PY2;
}

Expand Down Expand Up @@ -320,8 +326,11 @@ public void setPythonVersion(PythonVersion version) {
@Override
public FragmentOptions getHost() {
PythonOptions hostPythonOptions = (PythonOptions) getDefault();
PythonVersion hostVersion =
(hostForcePython != null) ? hostForcePython : getDefaultPythonVersion();
PythonVersion hostVersion = getDefaultPythonVersion();
if (hostForcePython != null) {
hostVersion = hostForcePython;
hostPythonOptions.defaultPythonVersion = hostForcePython;
}
hostPythonOptions.setPythonVersion(hostVersion);
hostPythonOptions.incompatiblePy3IsDefault = incompatiblePy3IsDefault;
hostPythonOptions.incompatiblePy2OutputsAreSuffixed = incompatiblePy2OutputsAreSuffixed;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import com.google.devtools.build.lib.analysis.config.BuildOptions;
import com.google.devtools.build.lib.analysis.config.BuildOptionsCache;
import com.google.devtools.build.lib.analysis.config.BuildOptionsView;
import com.google.devtools.build.lib.analysis.config.CoreOptions;
import com.google.devtools.build.lib.analysis.config.FragmentOptions;
import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
import com.google.devtools.build.lib.events.EventHandler;
Expand Down Expand Up @@ -69,17 +68,12 @@ private PythonVersionTransition() {}

@Override
public ImmutableSet<Class<? extends FragmentOptions>> requiresOptionFragments() {
return ImmutableSet.of(PythonOptions.class, CoreOptions.class);
return ImmutableSet.of(PythonOptions.class);
}

@Override
public BuildOptions patch(BuildOptionsView options, EventHandler eventHandler) {
// If this happens after exec transition, keep the same version (to reproduce and keep behaviour
// of the host transition, that happens after this one)
PythonVersion newVersion =
options.get(CoreOptions.class).isExec
? options.get(PythonOptions.class).getPythonVersion()
: determineNewVersion(options);
PythonVersion newVersion = determineNewVersion(options);
checkArgument(newVersion.isTargetValue(), newVersion);

PythonOptions opts = options.get(PythonOptions.class);
Expand Down

0 comments on commit 14925b5

Please sign in to comment.