Only add the runfiles of artifacts that are actual inputs of the spaw…

…n action. The previous solution added too many unneeded runfiles, which caused problems with our workers. -- MOS_MIGRATED_REVID=135782773
ManishSeal · Oct 11, 2016 · e23b79c · e23b79c
1 parent 8d36a34
commit e23b79c
Show file tree

Hide file tree

Showing 2 changed files with 13 additions and 32 deletions.
diff --git a/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java b/src/main/java/com/google/devtools/build/lib/rules/SkylarkRuleImplementationFunctions.java
@@ -25,7 +25,6 @@
 import com.google.devtools.build.lib.analysis.FilesToRunProvider;
 import com.google.devtools.build.lib.analysis.LocationExpander;
 import com.google.devtools.build.lib.analysis.PseudoAction;
-import com.google.devtools.build.lib.analysis.RuleConfiguredTarget.Mode;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.Runfiles;
 import com.google.devtools.build.lib.analysis.RunfilesProvider;
@@ -38,9 +37,6 @@
 import com.google.devtools.build.lib.collect.nestedset.NestedSet;
 import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder;
 import com.google.devtools.build.lib.events.Location;
-import com.google.devtools.build.lib.packages.Attribute;
-import com.google.devtools.build.lib.packages.Attribute.ConfigurationTransition;
-import com.google.devtools.build.lib.packages.AttributeMap;
 import com.google.devtools.build.lib.skylarkinterface.Param;
 import com.google.devtools.build.lib.skylarkinterface.ParamType;
 import com.google.devtools.build.lib.skylarkinterface.SkylarkSignature;
@@ -233,7 +229,8 @@ public Runtime.NoneType invoke(
           SpawnAction.Builder builder = new SpawnAction.Builder();
           // TODO(bazel-team): builder still makes unnecessary copies of inputs, outputs and args.
           boolean hasCommand = commandUnchecked != Runtime.NONE;
-          builder.addInputs(inputs.getContents(Artifact.class, "inputs"));
+          Iterable<Artifact> inputArtifacts = inputs.getContents(Artifact.class, "inputs");
+          builder.addInputs(inputArtifacts);
           builder.addOutputs(outputs.getContents(Artifact.class, "outputs"));
           if (hasCommand && arguments.size() > 0) {
             // When we use a shell command, add an empty argument before other arguments.
@@ -286,10 +283,14 @@ public Runtime.NoneType invoke(
             }
           }
 
-          // The actual command can refer to an executable from the inputs, which could
-          // require some runfiles. Consequently, we add the runfiles of every executable
-          // input file that is in HOST configuration to the action as a precaution.
-          addRequiredIndirectRunfiles(ctx, builder);
+          // The actual command can refer to an executable from the inputs, which could require
+          // some runfiles. Consequently, we add the runfiles of all inputs of this action manually.
+          for (Artifact current : inputArtifacts) {
+            FilesToRunProvider provider = ctx.getExecutableRunfiles(current);
+            if (provider != null) {
+              builder.addTool(provider);
+            }
+          }
 
           String mnemonic = mnemonicUnchecked == Runtime.NONE
               ? "Generating" : (String) mnemonicUnchecked;
@@ -332,27 +333,6 @@ public Runtime.NoneType invoke(
         }
       };
 
-  /**
-   * Adds the runfiles of the given input files to the action builder when they are executable and
-   * in HOST configuration.
-   */
-  private static void addRequiredIndirectRunfiles(
-      SkylarkRuleContext ctx, SpawnAction.Builder builder) {
-    RuleContext ruleContext = ctx.getRuleContext();
-    AttributeMap attrMap = ruleContext.attributes();
-
-    for (String attrName : attrMap.getAttributeNames()) {
-      Attribute attr = attrMap.getAttributeDefinition(attrName);
-      if (attr.isExecutable()
-          && (attr.getConfigurationTransition() == ConfigurationTransition.HOST)) {
-        FilesToRunProvider prov = ruleContext.getExecutablePrerequisite(attrName, Mode.HOST);
-        if (prov != null) {
-          builder.addTool(prov);
-        }
-      }
-    }
-  }
-
   @SkylarkSignature(name = "expand_location",
       doc =
       "Expands all <code>$(location ...)</code> templates in the given string by replacing "

diff --git a/tools/build_rules/genproto.bzl b/tools/build_rules/genproto.bzl
@@ -31,7 +31,8 @@ def gensrcjar_impl(ctx):
         ctx.executable._gensrcjar.path,
     ]),
     inputs=([ctx.file.src] + ctx.files._gensrcjar + ctx.files._jar +
-            ctx.files._jdk + ctx.files._proto_compiler),
+            ctx.files._jdk + ctx.files._proto_compiler +
+            ctx.files.grpc_java_plugin),
     outputs=[out],
     mnemonic="GenProtoSrcJar",
     use_default_shell_env=True)
@@ -51,7 +52,7 @@ gensrcjar = rule(
             single_file = True,
         ),
         "_gensrcjar": attr.label(
-            default = Label(str(Label("//tools/build_rules:gensrcjar"))),
+            default = Label("//tools/build_rules:gensrcjar"),
             cfg = "host",
             executable = True,
         ),