Refactor some py_runtime code

Moved its configured target factory, provider, and test to the ...lib/rules/python directory instead of ...lib/bazel/rules/python. Simplified the test. Also moved BazelPythonConfigurationTest into the bazel dir.

The `files` attribute of py_runtime is no longer mandatory. Since a non-empty value is not permitted when `interpreter_path` is used, it makes more sense to omit it rather than explicitly set it to empty.

The error messages and rule logic are simplified. The provider now has an invariant that either interpreter is null or else interpreterPath and files are null; this will be clarified in a follow-up CL that exposes PyRuntimeProvider to Starlark.

Work toward #7375.

PiperOrigin-RevId: 234174755

src/main/java/com/google/devtools/build/lib/BUILD

@@ -1141,6 +1141,7 @@ java_library(
         "//src/main/java/com/google/devtools/common/options",
         "//src/main/protobuf:crosstool_config_java_proto",
         "//src/main/protobuf:extra_actions_base_java_proto",
+        "//third_party:auto_value",
         "//third_party:guava",
         "//third_party:jsr305",
         "//third_party/protobuf:protobuf_java",

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuntimeRule.java

@@ -24,10 +24,11 @@
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment;
 import com.google.devtools.build.lib.packages.RuleClass;
+import com.google.devtools.build.lib.rules.python.PyRuntime;
 import com.google.devtools.build.lib.rules.python.PythonConfiguration;
 import com.google.devtools.build.lib.util.FileTypeSet;
 
-/** Rule definition for {@code python_runtime} */
+/** Rule definition for {@code py_runtime} */
 public final class BazelPyRuntimeRule implements RuleDefinition {
 
   @Override
@@ -38,24 +39,19 @@ public RuleClass build(RuleClass.Builder builder, RuleDefinitionEnvironment env)
         /* <!-- #BLAZE_RULE(py_runtime).ATTRIBUTE(files) -->
         The set of files comprising this Python runtime.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("files", LABEL_LIST)
-            .allowedFileTypes(FileTypeSet.ANY_FILE)
-            .mandatory())
+        .add(attr("files", LABEL_LIST).allowedFileTypes(FileTypeSet.ANY_FILE))
 
         /* <!-- #BLAZE_RULE(py_runtime).ATTRIBUTE(interpreter) -->
         The Python interpreter used in this runtime. Binary rules will be executed using this
         binary.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
-        .add(attr("interpreter", LABEL)
-            .allowedFileTypes(FileTypeSet.ANY_FILE)
-            .singleArtifact())
+        .add(attr("interpreter", LABEL).allowedFileTypes(FileTypeSet.ANY_FILE).singleArtifact())
 
         /* <!-- #BLAZE_RULE(py_runtime).ATTRIBUTE(interpreter_path) -->
         The absolute path of a Python interpreter. This attribute and interpreter attribute cannot
         be set at the same time.
         <!-- #END_BLAZE_RULE.ATTRIBUTE --> */
         .add(attr("interpreter_path", STRING))
-
         .add(attr("output_licenses", LICENSE))
         .build();
   }
@@ -65,7 +61,7 @@ public Metadata getMetadata() {
     return Metadata.builder()
         .name("py_runtime")
         .ancestors(BaseRuleClasses.BaseRule.class)
-        .factoryClass(BazelPyRuntime.class)
+        .factoryClass(PyRuntime.class)
         .build();
   }
 }

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java

@@ -44,6 +44,7 @@
 import com.google.devtools.build.lib.rules.cpp.CcInfo;
 import com.google.devtools.build.lib.rules.python.PyCcLinkParamsProvider;
 import com.google.devtools.build.lib.rules.python.PyCommon;
+import com.google.devtools.build.lib.rules.python.PyRuntimeProvider;
 import com.google.devtools.build.lib.rules.python.PythonConfiguration;
 import com.google.devtools.build.lib.rules.python.PythonSemantics;
 import com.google.devtools.build.lib.util.FileTypeSet;
@@ -313,17 +314,17 @@ private static void createPythonZipAction(
   }
 
   private static void addRuntime(RuleContext ruleContext, Runfiles.Builder builder) {
-    BazelPyRuntimeProvider provider = ruleContext.getPrerequisite(
-        ":py_interpreter", Mode.TARGET, BazelPyRuntimeProvider.class);
-    if (provider != null && provider.interpreter() != null) {
-      builder.addArtifact(provider.interpreter());
+    PyRuntimeProvider provider =
+        ruleContext.getPrerequisite(":py_interpreter", Mode.TARGET, PyRuntimeProvider.class);
+    if (provider != null && provider.isHermetic()) {
+      builder.addArtifact(provider.getInterpreter());
       // WARNING: we are adding the all Python runtime files here,
       // and it would fail if the filenames of them contain spaces.
       // Currently, we need to exclude them in py_runtime rules.
       // Possible files in Python runtime which contain spaces in filenames:
       // - https://github.com/pypa/setuptools/blob/master/setuptools/script%20(dev).tmpl
       // - https://github.com/pypa/setuptools/blob/master/setuptools/command/launcher%20manifest.xml
-      builder.addTransitiveArtifacts(provider.files());
+      builder.addTransitiveArtifacts(provider.getFiles());
     }
   }
 
@@ -333,20 +334,20 @@ private static String getPythonBinary(
 
     String pythonBinary;
 
-    BazelPyRuntimeProvider provider = ruleContext.getPrerequisite(
-        ":py_interpreter", Mode.TARGET, BazelPyRuntimeProvider.class);
+    PyRuntimeProvider provider =
+        ruleContext.getPrerequisite(":py_interpreter", Mode.TARGET, PyRuntimeProvider.class);
 
     if (provider != null) {
       // make use of py_runtime defined by --python_top
-      if (!provider.interpreterPath().isEmpty()) {
+      if (!provider.isHermetic()) {
         // absolute Python path in py_runtime
-        pythonBinary = provider.interpreterPath();
+        pythonBinary = provider.getInterpreterPath().getPathString();
       } else {
         // checked in Python interpreter in py_runtime
         PathFragment workspaceName =
             PathFragment.create(ruleContext.getRule().getPackage().getWorkspaceName());
         pythonBinary =
-            workspaceName.getRelative(provider.interpreter().getRunfilesPath()).getPathString();
+            workspaceName.getRelative(provider.getInterpreter().getRunfilesPath()).getPathString();
       }
     } else  {
       // make use of the Python interpreter in an absolute path

src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPyRuntime.java → src/main/java/com/google/devtools/build/lib/rules/python/PyRuntime.java

@@ -12,7 +12,7 @@
 // See the License for the specific language governing permissions and
 // limitations under the License.
 
-package com.google.devtools.build.lib.bazel.rules.python;
+package com.google.devtools.build.lib.rules.python;
 
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
@@ -24,56 +24,48 @@
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
 import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
-import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.syntax.Type;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
-/**
- * Implementation for the {@code py_runtime} rule.
- */
-public final class BazelPyRuntime implements RuleConfiguredTargetFactory {
+/** Implementation for the {@code py_runtime} rule. */
+public final class PyRuntime implements RuleConfiguredTargetFactory {
 
   @Override
-  public ConfiguredTarget create(RuleContext ruleContext)
-      throws InterruptedException, RuleErrorException, ActionConflictException {
+  public ConfiguredTarget create(RuleContext ruleContext) throws ActionConflictException {
     NestedSet<Artifact> files =
         PrerequisiteArtifacts.nestedSet(ruleContext, "files", Mode.TARGET);
-    Artifact interpreter =
-        ruleContext.getPrerequisiteArtifact("interpreter", Mode.TARGET);
-    String interpreterPath =
-        ruleContext.attributes().get("interpreter_path", Type.STRING);
+    Artifact interpreter = ruleContext.getPrerequisiteArtifact("interpreter", Mode.TARGET);
+    PathFragment interpreterPath =
+        PathFragment.create(ruleContext.attributes().get("interpreter_path", Type.STRING));
 
-    NestedSet<Artifact> all = NestedSetBuilder.<Artifact>stableOrder()
-        .addTransitive(files)
-        .build();
-
-    if (interpreter != null && !interpreterPath.isEmpty()) {
-      ruleContext.ruleError("interpreter and interpreter_path cannot be set at the same time.");
+    // Determine whether we're pointing to an in-build target (hermetic) or absolute system path
+    // (non-hermetic).
+    if ((interpreter == null) == interpreterPath.isEmpty()) {
+      ruleContext.ruleError(
+          "exactly one of the 'interpreter' or 'interpreter_path' attributes must be specified");
     }
-
-    if (interpreter == null && interpreterPath.isEmpty()) {
-      ruleContext.ruleError("interpreter and interpreter_path cannot be empty at the same time.");
+    boolean hermetic = interpreter != null;
+    // Validate attributes.
+    if (!hermetic && !files.isEmpty()) {
+      ruleContext.ruleError("if 'interpreter_path' is given then 'files' must be empty");
     }
-
-    if (!interpreterPath.isEmpty() && !PathFragment.create(interpreterPath).isAbsolute()) {
+    if (!hermetic && !interpreterPath.isAbsolute()) {
       ruleContext.attributeError("interpreter_path", "must be an absolute path.");
     }
 
-    if (!interpreterPath.isEmpty() && !files.isEmpty()) {
-      ruleContext.ruleError("interpreter with an absolute path requires files to be empty.");
-    }
-
     if (ruleContext.hasErrors()) {
       return null;
     }
 
-    BazelPyRuntimeProvider provider = BazelPyRuntimeProvider
-        .create(files, interpreter, interpreterPath);
+    PyRuntimeProvider provider =
+        hermetic
+            ? PyRuntimeProvider.create(files, interpreter, /*interpreterPath=*/ null)
+            : PyRuntimeProvider.create(/*files=*/ null, /*interpreter=*/ null, interpreterPath);
 
     return new RuleConfiguredTargetBuilder(ruleContext)
+        .setFilesToBuild(files)
         .addProvider(RunfilesProvider.class, RunfilesProvider.EMPTY)
-        .setFilesToBuild(all)
-        .addProvider(BazelPyRuntimeProvider.class, provider)
+        .addProvider(PyRuntimeProvider.class, provider)
         .build();
   }
 

src/main/java/com/google/devtools/build/lib/rules/python/PyRuntimeProvider.java

@@ -0,0 +1,60 @@
+// Copyright 2017 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// 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 com.google.auto.value.AutoValue;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
+import com.google.devtools.build.lib.collect.nestedset.NestedSet;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import javax.annotation.Nullable;
+
+/** Information about the Python runtime used by the <code>py_*</code> rules. */
+@AutoValue
+@Immutable
+public abstract class PyRuntimeProvider implements TransitiveInfoProvider {
+
+  public static PyRuntimeProvider create(
+      @Nullable NestedSet<Artifact> files,
+      @Nullable Artifact interpreter,
+      @Nullable PathFragment interpreterPath) {
+    return new AutoValue_PyRuntimeProvider(files, interpreter, interpreterPath);
+  }
+
+  /**
+   * Returns whether this runtime is hermetic, i.e. represents an in-build interpreter as opposed to
+   * a system interpreter.
+   *
+   * <p>Hermetic runtimes have non-null values for {@link #getInterpreter} and {@link #getFiles},
+   * while non-hermetic runtimes have non-null {@link #getInterpreterPath}.
+   *
+   * <p>Note: Despite the name, it is still possible for a hermetic runtime to reference in-build
+   * files that have non-hermetic behavior. For example, {@link #getInterpreter} could reference a
+   * checked-in wrapper script that calls the system interpreter at execution time.
+   */
+  public boolean isHermetic() {
+    return getInterpreter() != null;
+  }
+
+  @Nullable
+  public abstract NestedSet<Artifact> getFiles();
+
+  @Nullable
+  public abstract Artifact getInterpreter();
+
+  @Nullable
+  public abstract PathFragment getInterpreterPath();
+}

src/test/java/com/google/devtools/build/lib/bazel/BUILD

@@ -48,7 +48,6 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib:core-workspace-rules",
         "//src/main/java/com/google/devtools/build/lib:events",
         "//src/main/java/com/google/devtools/build/lib:packages-internal",
-        "//src/main/java/com/google/devtools/build/lib:python-rules",
         "//src/main/java/com/google/devtools/build/lib:syntax",
         "//src/main/java/com/google/devtools/build/lib:util",
         "//src/main/java/com/google/devtools/build/lib/actions",