Add arguments parameter to RunEnvironmentInfo

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

@@ -1025,6 +1025,7 @@ java_library(
     name = "run_environment_info",
     srcs = ["RunEnvironmentInfo.java"],
     deps = [
+        "//src/main/java/com/google/devtools/build/lib/actions",
         "//src/main/java/com/google/devtools/build/lib/concurrent",
         "//src/main/java/com/google/devtools/build/lib/packages",
         "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi",

src/main/java/com/google/devtools/build/lib/analysis/RuleConfiguredTargetBuilder.java

@@ -474,6 +474,7 @@ private TestProvider initializeTestProvider(FilesToRunProvider filesToRunProvide
     if (environmentProvider != null) {
       testActionBuilder.addExtraEnv(environmentProvider.getEnvironment());
       testActionBuilder.addExtraInheritedEnv(environmentProvider.getInheritedEnvironment());
+      testActionBuilder.setStarlarkTargetArgs(environmentProvider.getArguments());
     }
 
     TestParams testParams =

src/main/java/com/google/devtools/build/lib/analysis/RunEnvironmentInfo.java

@@ -17,15 +17,18 @@
 import com.google.common.base.Preconditions;
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
+import com.google.devtools.build.lib.actions.CommandLine;
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.BuiltinProvider;
 import com.google.devtools.build.lib.packages.NativeInfo;
 import com.google.devtools.build.lib.starlarkbuildapi.RunEnvironmentInfoApi;
 import java.util.List;
 import java.util.Map;
+import javax.annotation.Nullable;
 import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Sequence;
+import net.starlark.java.eval.Starlark;
 import net.starlark.java.eval.StarlarkList;
 
 /**
@@ -40,16 +43,19 @@ public final class RunEnvironmentInfo extends NativeInfo implements RunEnvironme
 
   private final ImmutableMap<String, String> environment;
   private final ImmutableList<String> inheritedEnvironment;
+  @Nullable private final CommandLine arguments;
   private final boolean shouldErrorOnNonExecutableRule;
 
   /** Constructs a new provider with the given fixed and inherited environment variables. */
   public RunEnvironmentInfo(
       Map<String, String> environment,
       List<String> inheritedEnvironment,
+      @Nullable CommandLine arguments,
       boolean shouldErrorOnNonExecutableRule) {
     this.environment = ImmutableMap.copyOf(Preconditions.checkNotNull(environment));
     this.inheritedEnvironment =
         ImmutableList.copyOf(Preconditions.checkNotNull(inheritedEnvironment));
+    this.arguments = arguments;
     this.shouldErrorOnNonExecutableRule = shouldErrorOnNonExecutableRule;
   }
 
@@ -76,6 +82,11 @@ public ImmutableList<String> getInheritedEnvironment() {
     return inheritedEnvironment;
   }
 
+  @Nullable
+  public CommandLine getArguments() {
+    return arguments;
+  }
+
   /**
    * Returns whether advertising this provider on a non-executable (and thus non-test) rule should
    * result in an error or a warning. The latter is required to not break testing.TestEnvironment,
@@ -95,11 +106,12 @@ private RunEnvironmentInfoProvider() {
 
     @Override
     public RunEnvironmentInfoApi constructor(
-        Dict<?, ?> environment, Sequence<?> inheritedEnvironment) throws EvalException {
+        Dict<?, ?> environment, Sequence<?> inheritedEnvironment, Object arguments) throws EvalException {
       return new RunEnvironmentInfo(
           Dict.cast(environment, String.class, String.class, "environment"),
           StarlarkList.immutableCopyOf(
               Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")),
+          arguments == Starlark.NONE ? null : CommandLine.of(Sequence.cast(arguments, String.class, "arguments")),
           /* shouldErrorOnNonExecutableRule= */ true);
     }
   }

src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java

@@ -420,6 +420,20 @@ public static RunfilesSupport withExecutable(
         computeActionEnvironment(ruleContext));
   }
 
+  /**
+   * Creates and returns a {@link RunfilesSupport} object for the given rule and executable without
+   * computing arguments based on the value of the "args" attribute.
+   */
+  public static RunfilesSupport withExecutableNoArgs(
+      RuleContext ruleContext, Runfiles runfiles, Artifact executable) {
+    return RunfilesSupport.create(
+        ruleContext,
+        executable,
+        runfiles,
+        CommandLine.EMPTY,
+        computeActionEnvironment(ruleContext));
+  }
+
   /**
    * Creates and returns a {@link RunfilesSupport} object for the given rule and executable. Note
    * that this method calls back into the passed in rule to obtain the runfiles.

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleConfiguredTargetUtil.java

@@ -343,30 +343,40 @@ private static void addProviders(
       }
     }
 
+    DefaultInfo defaultInfo = null;
     boolean defaultProviderProvidedExplicitly = false;
+    boolean starlarkArgsProvided = false;
 
     for (Info declaredProvider : declaredProviders.values()) {
       if (getProviderKey(declaredProvider).equals(DefaultInfo.PROVIDER.getKey())) {
-        parseDefaultProviderFields((DefaultInfo) declaredProvider, context, builder);
+        defaultInfo = (DefaultInfo) declaredProvider;
         defaultProviderProvidedExplicitly = true;
-      } else if (getProviderKey(declaredProvider).equals(RunEnvironmentInfo.PROVIDER.getKey())
-          && !(context.isExecutable() || context.getRuleContext().isTestTarget())) {
-        String message =
-            "Returning RunEnvironmentInfo from a non-executable, non-test target has no effect";
-        RunEnvironmentInfo runEnvironmentInfo = (RunEnvironmentInfo) declaredProvider;
-        if (runEnvironmentInfo.shouldErrorOnNonExecutableRule()) {
-          context.getRuleContext().ruleError(message);
-        } else {
-          context.getRuleContext().ruleWarning(message);
-          builder.addStarlarkDeclaredProvider(declaredProvider);
+      } else if (getProviderKey(declaredProvider).equals(RunEnvironmentInfo.PROVIDER.getKey())) {
+        if (((RunEnvironmentInfo) declaredProvider).getArguments() != null) {
+          // Ensure that RunfilesSupport does not parse the "args" attribute, the rule handles it in
+          // a custom way.
+          starlarkArgsProvided = true;
+        }
+        if (!(context.isExecutable() || context.getRuleContext().isTestTarget())) {
+          String message =
+              "Returning RunEnvironmentInfo from a non-executable, non-test target has no effect";
+          RunEnvironmentInfo runEnvironmentInfo = (RunEnvironmentInfo) declaredProvider;
+          if (runEnvironmentInfo.shouldErrorOnNonExecutableRule()) {
+            context.getRuleContext().ruleError(message);
+          } else {
+            context.getRuleContext().ruleWarning(message);
+          }
         }
+        builder.addStarlarkDeclaredProvider(declaredProvider);
       } else {
         builder.addStarlarkDeclaredProvider(declaredProvider);
       }
     }
 
-    if (!defaultProviderProvidedExplicitly) {
-      parseDefaultProviderFields(oldStyleProviders, context, builder);
+    if (defaultProviderProvidedExplicitly) {
+      parseDefaultProviderFields(defaultInfo, context, builder, starlarkArgsProvided);
+    } else {
+      parseDefaultProviderFields(oldStyleProviders, context, builder, starlarkArgsProvided);
     }
 
     for (String field : oldStyleProviders.getFieldNames()) {
@@ -491,7 +501,10 @@ private static Provider.Key getProviderKey(Info info) throws EvalException {
    * throws an {@link EvalException} if there are unknown fields.
    */
   private static void parseDefaultProviderFields(
-      StructImpl info, StarlarkRuleContext context, RuleConfiguredTargetBuilder builder)
+      StructImpl info,
+      StarlarkRuleContext context,
+      RuleConfiguredTargetBuilder builder,
+      boolean starlarkArgsProvided)
       throws EvalException {
     Depset files = null;
     Runfiles statelessRunfiles = null;
@@ -591,7 +604,8 @@ private static void parseDefaultProviderFields(
         files,
         statelessRunfiles,
         dataRunfiles,
-        defaultRunfiles);
+        defaultRunfiles,
+        starlarkArgsProvided);
   }
 
   private static void addSimpleProviders(
@@ -601,7 +615,8 @@ private static void addSimpleProviders(
       @Nullable Depset files,
       Runfiles statelessRunfiles,
       Runfiles dataRunfiles,
-      Runfiles defaultRunfiles)
+      Runfiles defaultRunfiles,
+      boolean starlarkArgsProvided)
       throws EvalException {
 
     // TODO(bazel-team) if both 'files' and 'executable' are provided, 'files' overrides
@@ -643,8 +658,13 @@ private static void addSimpleProviders(
       RunfilesSupport runfilesSupport = null;
       if (!computedDefaultRunfiles.isEmpty()) {
         Preconditions.checkNotNull(executable, "executable must not be null");
-        runfilesSupport =
-            RunfilesSupport.withExecutable(ruleContext, computedDefaultRunfiles, executable);
+        if (starlarkArgsProvided) {
+          runfilesSupport = RunfilesSupport.withExecutableNoArgs(
+              ruleContext, computedDefaultRunfiles, executable);
+        } else {
+          runfilesSupport =
+              RunfilesSupport.withExecutable(ruleContext, computedDefaultRunfiles, executable);
+        }
         assertExecutableSymlinkPresent(runfilesSupport.getRunfiles(), executable);
       }
       builder.setRunfilesSupport(runfilesSupport, executable);

src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java

@@ -25,6 +25,7 @@
 import com.google.devtools.build.lib.actions.ActionOwner;
 import com.google.devtools.build.lib.actions.Artifact;
 import com.google.devtools.build.lib.actions.ArtifactRoot;
+import com.google.devtools.build.lib.actions.CommandLine;
 import com.google.devtools.build.lib.actions.RunfilesSupplier;
 import com.google.devtools.build.lib.analysis.Allowlist;
 import com.google.devtools.build.lib.analysis.AnalysisEnvironment;
@@ -98,12 +99,14 @@ public NestedSet<PackageGroupContents> getPackageSpecifications() {
   private int explicitShardCount;
   private final Map<String, String> extraEnv;
   private final Set<String> extraInheritedEnv;
+  @Nullable private CommandLine starlarkTargetArgs;
 
   public TestActionBuilder(RuleContext ruleContext) {
     this.ruleContext = ruleContext;
     this.extraEnv = new TreeMap<>();
     this.extraInheritedEnv = new TreeSet<>();
     this.additionalTools = new ImmutableList.Builder<>();
+    this.starlarkTargetArgs = null;
   }
 
   /**
@@ -180,6 +183,12 @@ public TestActionBuilder addExtraInheritedEnv(List<String> extraInheritedEnv) {
     return this;
   }
 
+  @CanIgnoreReturnValue
+  public TestActionBuilder setStarlarkTargetArgs(@Nullable CommandLine starlarkTargetArgs) {
+    this.starlarkTargetArgs = starlarkTargetArgs;
+    return this;
+  }
+
   /** Set the explicit shard count. Note that this may be overridden by the sharding strategy. */
   @CanIgnoreReturnValue
   public TestActionBuilder setShardCount(int explicitShardCount) {
@@ -340,6 +349,7 @@ private TestParams createTestAction(int shards)
               ruleContext,
               runfilesSupport,
               executable,
+              starlarkTargetArgs,
               instrumentedFileManifest,
               shards,
               runsPerTest);
@@ -352,7 +362,13 @@ private TestParams createTestAction(int shards)
     } else {
       executionSettings =
           new TestTargetExecutionSettings(
-              ruleContext, runfilesSupport, executable, null, shards, runsPerTest);
+              ruleContext,
+              runfilesSupport,
+              executable,
+              starlarkTargetArgs,
+              null,
+              shards,
+              runsPerTest);
     }
 
     extraTestEnv.putAll(extraEnv);

src/main/java/com/google/devtools/build/lib/analysis/test/TestTargetExecutionSettings.java

@@ -53,6 +53,7 @@ public final class TestTargetExecutionSettings {
       RuleContext ruleContext,
       RunfilesSupport runfilesSupport,
       Artifact executable,
+      @Nullable CommandLine starlarkTargetArgs,
       Artifact instrumentedFileManifest,
       int shards,
       int runs)
@@ -62,7 +63,8 @@ public final class TestTargetExecutionSettings {
     BuildConfigurationValue config = ruleContext.getConfiguration();
     TestConfiguration testConfig = config.getFragment(TestConfiguration.class);
 
-    CommandLine targetArgs = runfilesSupport.getArgs();
+    CommandLine targetArgs =
+        starlarkTargetArgs != null ? starlarkTargetArgs : runfilesSupport.getArgs();
     testArguments =
         CommandLine.concat(targetArgs, ImmutableList.copyOf(testConfig.getTestArguments()));
 

src/main/java/com/google/devtools/build/lib/rules/test/StarlarkTestingModule.java

@@ -49,6 +49,7 @@ public RunEnvironmentInfo testEnvironment(
         Dict.cast(environment, String.class, String.class, "environment"),
         StarlarkList.immutableCopyOf(
             Sequence.cast(inheritedEnvironment, String.class, "inherited_environment")),
+        /* arguments= */null,
         /* shouldErrorOnNonExecutableRule= */ false);
   }
 

src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java

@@ -176,19 +176,28 @@ protected BuildResult processRequest(final CommandEnvironment env, BuildRequest
   public void editOptions(OptionsParser optionsParser) { }
 
   /**
-   * Compute the arguments the binary should be run with by concatenating the arguments in its
-   * {@code args} attribute and the arguments on the Blaze command line.
+   * Compute the arguments the binary should be run with by concatenating the arguments provided by
+   * its {@link RunEnvironmentInfo} or, if not set, in its {@code args} attribute, and the arguments
+   * on the Blaze command line.
    */
-  @Nullable
   private List<String> computeArgs(ConfiguredTarget targetToRun, List<String> commandLineArgs)
       throws InterruptedException, CommandLineExpansionException {
     List<String> args = Lists.newArrayList();
 
-    FilesToRunProvider provider = targetToRun.getProvider(FilesToRunProvider.class);
-    RunfilesSupport runfilesSupport = provider == null ? null : provider.getRunfilesSupport();
-    if (runfilesSupport != null && runfilesSupport.getArgs() != null) {
-      CommandLine targetArgs = runfilesSupport.getArgs();
-      Iterables.addAll(args, targetArgs.arguments());
+    // If a Starlark rule explicitly sets arguments on RunEnvironmentInfo, these arguments take
+    // precedence over those obtained from the implicit "args" attribute on all rules so that
+    // Starlark rules can implement their own logic for this attribute.
+    RunEnvironmentInfo runEnvironmentInfo =
+        (RunEnvironmentInfo) targetToRun.get(RunEnvironmentInfo.PROVIDER.getKey());
+    if (runEnvironmentInfo != null && runEnvironmentInfo.getArguments() != null) {
+      Iterables.addAll(args, runEnvironmentInfo.getArguments().arguments());
+    } else {
+      FilesToRunProvider provider = targetToRun.getProvider(FilesToRunProvider.class);
+      RunfilesSupport runfilesSupport = provider == null ? null : provider.getRunfilesSupport();
+      if (runfilesSupport != null && runfilesSupport.getArgs() != null) {
+        CommandLine targetArgs = runfilesSupport.getArgs();
+        Iterables.addAll(args, targetArgs.arguments());
+      }
     }
     args.addAll(commandLineArgs);
     return args;

src/main/java/com/google/devtools/build/lib/starlarkbuildapi/RunEnvironmentInfoApi.java

@@ -26,11 +26,12 @@
 import net.starlark.java.annot.StarlarkMethod;
 import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.EvalException;
+import net.starlark.java.eval.NoneType;
 import net.starlark.java.eval.Sequence;
 
 /**
- * Provider containing any additional environment variables for use when running executables, either
- * in test actions or when executed via the run command.
+ * Provider containing any additional environment variables and arguments for use when running
+ * executables, either in test actions or when executed via the run command.
  */
 @StarlarkBuiltin(
     name = "RunEnvironmentInfo",
@@ -78,7 +79,9 @@ interface RunEnvironmentInfoApiProvider extends ProviderApi {
               doc =
                   "A map of string keys and values that represent environment variables and their"
                       + " values. These will be made available when the target that returns this"
-                      + " provider is executed, either as a test or via the run command."),
+                      + " provider is executed, either as a test or via the run command. In the"
+                      + " case of a test, environment variables specified on the command line via"
+                      + " <code>--test_env</code> take precedence over values specified here."),
           @Param(
               name = "inherited_environment",
               allowedTypes = {@ParamType(type = Sequence.class, generic1 = String.class)},
@@ -91,13 +94,29 @@ interface RunEnvironmentInfoApiProvider extends ProviderApi {
                       + " when the target that returns this provider is executed, either as a"
                       + " test or via the run command. If a variable is contained in both <code>"
                       + "environment</code> and <code>inherited_environment</code>, the value"
-                      + " inherited from the shell environment will take precedence if set.")
+                      + " inherited from the shell environment will take precedence if set."),
+          @Param(
+              name = "arguments",
+              allowedTypes = {
+                  @ParamType(type = Sequence.class, generic1 = String.class),
+                  @ParamType(type = NoneType.class)
+              },
+              defaultValue = "None",
+              named = true,
+              positional = false,
+              doc = "A list of arguments. If set to a value other than <code>None</code>, this"
+                        + " list will be appended to the command line when the target that returns"
+                        + " this provider is executed, either as a test or via the run command. In"
+                        + " this case, the arguments specified in the <code>args</code> attribute"
+                        + " implicitly defined for all rules are <strong>not</code> appended"
+                        + " automatically.")
         },
         selfCall = true)
     @StarlarkConstructor
     RunEnvironmentInfoApi constructor(
         Dict<?, ?> environment, // <String, String> expected
-        Sequence<?> inheritedEnvironment /* <String> expected */)
+        Sequence<?> inheritedEnvironment, /* <String> expected */
+        Object arguments /* Sequence<String> or NoneType expected */)
         throws EvalException;
   }
 }