diff options
author | 2017-09-27 05:59:43 -0400 | |
---|---|---|
committer | 2017-09-27 10:01:35 -0400 | |
commit | ab2fd25e1d915f79af4ab9195602be4cb6652cb1 (patch) | |
tree | 43b1db8be10e656f370383c4058808d2467edab4 /src | |
parent | e10526a072bcb1be05152d0a2f0e72f671c80754 (diff) |
Reorganise the srcs_version attribute of py_binary/py_test/py_library rules:
make the default for srcs_version be "PY2AND3" (while leaving
default_python_version at "PY2"), since that's a more likely situation and a
more obvious error condition. (Having PY2 as the default caused 2to3 to be
used a lot more than necessary, which in turn can both hide and cause bugs.)
Also, use attribute value restrictions to limit srcs_version and
default_python_version to sensible values for the specific rule type.
PiperOrigin-RevId: 170174546
Diffstat (limited to 'src')
3 files changed, 48 insertions, 15 deletions
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 dee60f9ba7..9dad2447cf 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 @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.RuleDefinition; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; import com.google.devtools.build.lib.bazel.rules.cpp.BazelCppRuleClasses; import com.google.devtools.build.lib.cmdline.Label; +import com.google.devtools.build.lib.packages.Attribute.AllowedValueSet; import com.google.devtools.build.lib.packages.Attribute.LateBoundDefault; import com.google.devtools.build.lib.packages.RuleClass; import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType; @@ -99,12 +100,15 @@ public final class BazelPyRuleClasses { <code>"PY2AND3"</code> - Code that is compatible with both Python 2 and 3 without <code>2to3</code> conversion.<br/> - <code>"PY3"</code> - + <code>"PY3ONLY"</code> - Python 3 code that will not run on Python 2.<br/> + <code>"PY3"</code> - + A synonym for PY3ONLY.<br/> <br/> <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ .add(attr("srcs_version", STRING) - .value(PythonVersion.defaultValue().toString())) + .value(PythonVersion.defaultSrcsVersion().toString()) + .allowedValues(new AllowedValueSet(PythonVersion.getAllValues()))) .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. @@ -154,7 +158,8 @@ public final class BazelPyRuleClasses { Python 3 support is experimental. <!-- #END_BLAZE_RULE.ATTRIBUTE --> */ .add(attr("default_python_version", STRING) - .value(PythonVersion.defaultValue().toString()) + .value(PythonVersion.defaultTargetPythonVersion().toString()) + .allowedValues(new AllowedValueSet(PythonVersion.getTargetPythonValues())) .nonconfigurable("read by PythonUtils.getNewPythonVersion, which doesn't have access" + " to configuration keys")) /* <!-- #BLAZE_RULE($base_py_binary).ATTRIBUTE(srcs) --> 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 8aad3cc53b..39ef76239e 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 @@ -96,7 +96,7 @@ public final class PyCommon { public void initCommon(PythonVersion defaultVersion) { this.sourcesVersion = getPythonVersionAttr( - ruleContext, "srcs_version", PythonVersion.getAllValues()); + ruleContext, "srcs_version", PythonVersion.getAllVersions()); this.version = ruleContext.getFragment(PythonConfiguration.class) .getPythonVersion(defaultVersion); @@ -204,10 +204,11 @@ public final class PyCommon { if (version != null) { return version; } + // Should already have been disallowed in the rule. ruleContext.attributeError(attrName, "'" + stringAttr + "' is not a valid value. Expected one of: " + Joiner.on(", ") .join(allowed)); - return PythonVersion.defaultValue(); + return PythonVersion.defaultTargetPythonVersion(); } /** @@ -399,14 +400,11 @@ public final class PyCommon { */ private void checkSourceIsCompatible(PythonVersion targetVersion, PythonVersion sourceVersion, Label source) { - if (targetVersion == PythonVersion.PY2 || targetVersion == PythonVersion.PY2AND3) { - if (sourceVersion == PythonVersion.PY3ONLY) { - ruleContext.ruleError("Rule '" + source - + "' can only be used with Python 3, and cannot be converted to Python 2"); - } else if (sourceVersion == PythonVersion.PY3) { - ruleContext.ruleError("Rule '" + source - + "' need to be converted to Python 2 (not yet implemented)"); - } + // Treat PY3 as PY3ONLY: we'll never implement 3to2. + if ((targetVersion == PythonVersion.PY2 || targetVersion == PythonVersion.PY2AND3) + && (sourceVersion == PythonVersion.PY3 || sourceVersion == PythonVersion.PY3ONLY)) { + ruleContext.ruleError("Rule '" + source + + "' can only be used with Python 3, and cannot be converted to Python 2"); } if ((targetVersion == PythonVersion.PY3 || targetVersion == PythonVersion.PY2AND3) && sourceVersion == PythonVersion.PY2ONLY) { 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 8ef077429b..59dddddca1 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 @@ -13,6 +13,10 @@ // limitations under the License. package com.google.devtools.build.lib.rules.python; +import static com.google.common.collect.Iterables.transform; + +import com.google.common.base.Functions; +import com.google.common.collect.ImmutableList; import java.util.Arrays; /** @@ -28,14 +32,40 @@ public enum PythonVersion { static final PythonVersion[] ALL_VALUES = new PythonVersion[] { PY2, PY3, PY2AND3, PY2ONLY, PY3ONLY }; - public static PythonVersion defaultValue() { + static final PythonVersion[] NON_CONVERSION_VALUES = + new PythonVersion[] { PY2AND3, PY2ONLY, PY3ONLY }; + + static final PythonVersion[] TARGET_PYTHON_VALUES = + new PythonVersion[] { PY2, PY3 }; + + public static PythonVersion defaultSrcsVersion() { + return PY2AND3; + } + + public static PythonVersion defaultTargetPythonVersion() { return PY2; } - public static PythonVersion[] getAllValues() { + private static Iterable<String> convertToStrings(PythonVersion[] values) { + return transform(ImmutableList.copyOf(values), Functions.toStringFunction()); + } + + public static PythonVersion[] getAllVersions() { return ALL_VALUES; } + public static Iterable<String> getAllValues() { + return convertToStrings(ALL_VALUES); + } + + public static Iterable<String> getNonConversionValues() { + return convertToStrings(NON_CONVERSION_VALUES); + } + + public static Iterable<String> getTargetPythonValues() { + return convertToStrings(TARGET_PYTHON_VALUES); + } + /** * Converts the string to PythonVersion, if it is one of the allowed values. * Returns null if the input is not valid. |