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

1 files changed, 8 insertions, 10 deletions

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) {