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 6de6ea4ac1b29e..174a36cd6cc72a 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 @@ -66,9 +66,11 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env) py_binary rules, py_library rules. */ - .override(builder.copy("deps") - .legacyMandatoryProviders(PyCommon.PYTHON_SKYLARK_PROVIDER_NAME) - .allowedFileTypes()) + .override( + builder + .copy("deps") + .legacyMandatoryProviders(PyCommon.PYTHON_SKYLARK_PROVIDER_NAME) + .allowedFileTypes()) /* List of import directories to be added to the PYTHONPATH.

@@ -105,19 +107,21 @@ A string specifying the Python major version(s) that the .py source A synonym for PY3ONLY.

*/ - .add(attr("srcs_version", STRING) - .value(PythonVersion.defaultSrcsVersion().toString()) - .allowedValues(new AllowedValueSet(PythonVersion.getAllValues()))) + .add( + attr("srcs_version", STRING) + .value(PythonVersion.getDefaultSrcsValue().toString()) + .allowedValues(new AllowedValueSet(PythonVersion.getAllStrings()))) // TODO(brandjon): Consider adding to py_interpreter a .mandatoryNativeProviders() of // BazelPyRuntimeProvider. (Add a test case to PythonConfigurationTest for violations // of this requirement.) .add(attr(":py_interpreter", LABEL).value(PY_INTERPRETER)) // do not depend on lib2to3:2to3 rule, because it creates circular dependencies // 2to3 is itself written in Python and depends on many libraries. - .add(attr("$python2to3", LABEL) - .cfg(HostTransition.INSTANCE) - .exec() - .value(env.getToolsLabel("//tools/python:2to3"))) + .add( + attr("$python2to3", LABEL) + .cfg(HostTransition.INSTANCE) + .exec() + .value(env.getToolsLabel("//tools/python:2to3"))) .setPreferredDependencyPredicate(PyRuleClasses.PYTHON_SOURCE) .build(); } @@ -163,8 +167,8 @@ public RuleClass build(RuleClass.Builder builder, final RuleDefinitionEnvironmen */ .add( attr("default_python_version", STRING) - .value(PythonVersion.defaultTargetPythonVersion().toString()) - .allowedValues(new AllowedValueSet(PythonVersion.getTargetPythonValues())) + .value(PythonVersion.getDefaultTargetValue().toString()) + .allowedValues(new AllowedValueSet(PythonVersion.getTargetStrings())) .nonconfigurable( "read by PythonUtils.getNewPythonVersion, which doesn't have access" + " to configuration keys")) 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 cd83ea12416c9e..63e18ccebf63c5 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 @@ -95,8 +95,8 @@ public PyCommon(RuleContext ruleContext) { } public void initCommon(PythonVersion defaultVersion) { - this.sourcesVersion = getPythonVersionAttr( - ruleContext, "srcs_version", PythonVersion.getAllVersions()); + this.sourcesVersion = + getPythonVersionAttr(ruleContext, "srcs_version", PythonVersion.getAllValues()); this.version = ruleContext.getFragment(PythonConfiguration.class) .getPythonVersion(defaultVersion); @@ -217,7 +217,7 @@ public static PythonVersion getPythonVersionAttr(RuleContext ruleContext, ruleContext.attributeError(attrName, "'" + stringAttr + "' is not a valid value. Expected one of: " + Joiner.on(", ") .join(allowed)); - return PythonVersion.defaultTargetPythonVersion(); + return PythonVersion.getDefaultTargetValue(); } /** 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 23696ce6928e6c..99f23eda82d08f 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 @@ -27,16 +27,19 @@ public class PyRuleClasses { public static final FileType PYTHON_SOURCE = FileType.of(".py", ".py3"); /** - * Input for {@link RuleClass.Builder#cfg(RuleTransitionFactory)}: if - * {@link PythonOptions#forcePython} is unset, sets the Python version according to the rule's - * default Python version. Assumes the rule has the expected attribute for this setting. + * Input for {@link RuleClass.Builder#cfg(RuleTransitionFactory)}: if {@link + * PythonOptions#forcePython} is unset, sets the Python version according to the rule's default + * Python version. Assumes the rule has the expected attribute for this setting. * *

Since this is a configuration transition, this propagates to the rules' transitive deps. */ public static final RuleTransitionFactory DEFAULT_PYTHON_VERSION_TRANSITION = (rule) -> new PythonVersionTransition( + // In case of a parse error, this will return null, which means that the transition + // would use the hard-coded default (PythonVersion#getDefaultTargetValue). But the + // attribute is already validated to allow only PythonVersion#getTargetStrings anyway. PythonVersion.parse( RawAttributeMapper.of(rule).get("default_python_version", Type.STRING), - PythonVersion.ALL_VALUES)); + PythonVersion.getAllValues())); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PyTest.java b/src/main/java/com/google/devtools/build/lib/rules/python/PyTest.java index b60b5a221e0030..4554321abda429 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PyTest.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PyTest.java @@ -18,7 +18,6 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleConfiguredTargetFactory; import com.google.devtools.build.lib.analysis.RuleContext; -import com.google.devtools.build.lib.syntax.Type; /** * An implementation for {@code py_test} rules. @@ -35,7 +34,7 @@ public ConfiguredTarget create(RuleContext ruleContext) throws InterruptedException, RuleErrorException, ActionConflictException { PythonSemantics semantics = createSemantics(); PyCommon common = new PyCommon(ruleContext); - common.initCommon(getDefaultPythonVersion(ruleContext)); + common.initCommon(common.getDefaultPythonVersion()); RuleConfiguredTargetBuilder builder = PyBinary.init(ruleContext, semantics, common); if (builder == null) { @@ -43,12 +42,5 @@ public ConfiguredTarget create(RuleContext ruleContext) } return builder.build(); } - - private PythonVersion getDefaultPythonVersion(RuleContext ruleContext) { - return ruleContext.getRule().isAttrDefined("default_python_version", Type.STRING) - ? PyCommon.getPythonVersionAttr(ruleContext, "default_python_version", PythonVersion.PY2, - PythonVersion.PY3, PythonVersion.PY2AND3) - : null; - } } 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 2323f46679ec28..4fce82cb7c1e43 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 @@ -65,7 +65,7 @@ public PythonVersion getPythonVersion(PythonVersion attributeVersion) { @Override public String getOutputDirectoryName() { - List allowedVersions = Arrays.asList(PythonVersion.TARGET_PYTHON_VALUES); + List allowedVersions = Arrays.asList(PythonVersion.getTargetValues()); Verify.verify( allowedVersions.size() == 2, // If allowedVersions.size() == 1, we don't need this method. ">2 possible defaultPythonVersion values makes output directory clashes possible"); 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 7ace9604af70f6..3e8d36931c1c04 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 @@ -63,7 +63,7 @@ public PythonVersionConverter() { public PythonVersion hostForcePython; public PythonVersion getPythonVersion() { - return getPythonVersion(PythonVersion.DEFAULT); + return getPythonVersion(PythonVersion.getDefaultTargetValue()); } public PythonVersion getPythonVersion(PythonVersion defaultVersion) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/python/PythonVersion.java b/src/main/java/com/google/devtools/build/lib/rules/python/PythonVersion.java index 586b3a9d608727..0ad8d61f8c2ec1 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/python/PythonVersion.java +++ b/src/main/java/com/google/devtools/build/lib/rules/python/PythonVersion.java @@ -11,6 +11,7 @@ // WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. // See the License for the specific language governing permissions and // limitations under the License. + package com.google.devtools.build.lib.rules.python; import static com.google.common.collect.Iterables.transform; @@ -20,61 +21,133 @@ import java.util.Arrays; /** - * Python version for Python rules. + * An enum representing Python major versions. + * + *

This enum has two interpretations. The "target" interpretation is when this enum is used in a + * command line flag or in a rule attribute to denote a particular version of the Python language. + * Only {@code PY2} and {@code PY3} can be used as target values. The "sources" interpretation is + * when this enum is used to denote the degree of compatibility of source code with the target + * values. */ public enum PythonVersion { + + // TODO(#6445): Remove PY2ONLY and PY3ONLY. + + /** + * Target value Python 2. Represents source code that is naturally compatible with Python 2. + * + *

Deprecated meaning: Also indicates that source code is compatible with Python 3 under + * 2to3 transformation. 2to3 transformation is not implemented in Bazel and this meaning will be + * removed from Bazel (#1393). + */ PY2, + + /** + * Target value Python 3. Represents source code that is naturally compatible with Python 3. + * + *

Deprecated meaning: Also indicates that source code is compatible with Python 2 under + * 3to2 transformation. 3to2 transformation was never implemented and this meaning should not be + * relied on. + */ PY3, + + /** + * Represents source code that is naturally compatible with both Python 2 and Python 3, i.e. code + * that lies in the intersection of both languages. + */ PY2AND3, - PY2ONLY, - PY3ONLY; - static final PythonVersion[] ALL_VALUES = - new PythonVersion[] { PY2, PY3, PY2AND3, PY2ONLY, PY3ONLY }; + /** + * Alias for {@code PY2}. Deprecated in Bazel; prefer {@code PY2}. + * + *

Deprecated meaning: Indicates code that cannot be processed by 2to3. + */ + PY2ONLY, /** - * The Python version to use if not overridden by {@link PythonOptions#forcePython} or a rule's - * {@code default_python_version} or {@code srcs_version} attributes. + * Deprecated alias for {@code PY3}. + * + *

Deprecated meaning: Indicates code that cannot be processed by 3to2. */ - static final PythonVersion DEFAULT = PY2; + PY3ONLY; - static final PythonVersion[] NON_CONVERSION_VALUES = - new PythonVersion[] { PY2AND3, PY2ONLY, PY3ONLY }; + private static Iterable convertToStrings(PythonVersion[] values) { + return transform(ImmutableList.copyOf(values), Functions.toStringFunction()); + } + + private static final PythonVersion[] allValues = + new PythonVersion[] {PY2, PY3, PY2AND3, PY2ONLY, PY3ONLY}; + + private static final Iterable allStrings = convertToStrings(allValues); + + private static final PythonVersion[] targetValues = new PythonVersion[] {PY2, PY3}; + + private static final Iterable targetStrings = convertToStrings(targetValues); - static final PythonVersion[] TARGET_PYTHON_VALUES = - new PythonVersion[] { PY2, PY3 }; + private static final PythonVersion[] nonConversionValues = + new PythonVersion[] {PY2AND3, PY2ONLY, PY3ONLY}; - public static PythonVersion defaultSrcsVersion() { - return PY2AND3; + private static final Iterable nonConversionStrings = + convertToStrings(nonConversionValues); + + private static final PythonVersion DEFAULT_TARGET_VALUE = PY2; + + private static final PythonVersion DEFAULT_SRCS_VALUE = PY2AND3; + + /** Returns all values as a new array. */ + public static PythonVersion[] getAllValues() { + return Arrays.copyOf(allValues, allValues.length); } - public static PythonVersion defaultTargetPythonVersion() { - return DEFAULT; + /** Returns an iterable of all values as strings. */ + public static Iterable getAllStrings() { + return allStrings; } - private static Iterable convertToStrings(PythonVersion[] values) { - return transform(ImmutableList.copyOf(values), Functions.toStringFunction()); + /** Returns all values representing a specific version, as a new array. */ + public static PythonVersion[] getTargetValues() { + return Arrays.copyOf(targetValues, targetValues.length); } - public static PythonVersion[] getAllVersions() { - return ALL_VALUES; + /** Returns an iterable of all values representing a specific version, as strings. */ + public static Iterable getTargetStrings() { + return targetStrings; } - public static Iterable getAllValues() { - return convertToStrings(ALL_VALUES); + /** + * Returns all values that do not imply running a transpiler to convert between versions, as a new + * array. + */ + public static PythonVersion[] getNonConversionValues() { + return Arrays.copyOf(nonConversionValues, nonConversionValues.length); } - public static Iterable getNonConversionValues() { - return convertToStrings(NON_CONVERSION_VALUES); + /** + * Returns all values that do not imply running a transpiler to convert between versions, as + * strings. + */ + public static Iterable getNonConversionStrings() { + return nonConversionStrings; } - public static Iterable getTargetPythonValues() { - return convertToStrings(TARGET_PYTHON_VALUES); + /** Returns the Python version to use if not otherwise specified by a flag or attribute. */ + public static PythonVersion getDefaultTargetValue() { + return DEFAULT_TARGET_VALUE; + } + + /** + * Returns the level of source compatibility assumed if not otherwise specified by an attribute. + */ + public static PythonVersion getDefaultSrcsValue() { + return DEFAULT_SRCS_VALUE; } + // TODO(brandjon): Refactor this into parseTargetValue and parseSourcesValue methods. Throw + // IllegalArgumentException on bad values instead of returning null, and modify callers to + // tolerate the exception. /** - * Converts the string to PythonVersion, if it is one of the allowed values. - * Returns null if the input is not valid. + * Converts the string to PythonVersion, if it is one of the allowed values. Returns null if the + * input is not valid. */ public static PythonVersion parse(String str, PythonVersion... allowed) { if (str == null) { 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 171f114a0f2435..e947418cf4a5c7 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 @@ -172,7 +172,7 @@ public void srcsVersionClashesWithDefaultVersionAttr() throws Exception { public void srcsVersionClashesWithDefaultVersionAttr_Implicitly() throws Exception { // Canary assertion: This'll fail when we flip the default to PY3. At that point change this // test to use srcs_version = 'PY2ONLY' instead. - assertThat(PythonVersion.defaultTargetPythonVersion()).isEqualTo(PythonVersion.PY2); + assertThat(PythonVersion.getDefaultTargetValue()).isEqualTo(PythonVersion.PY2); // Fails because default_python_version is PY2 by default, so the config is set to PY2 // regardless of srcs_version.