Refactor CommandHelper.java
Extract language specific functions to an interface.
Working towards: https://github.com/bazelbuild/bazel/issues/7503
Closes #9022.
PiperOrigin-RevId: 260910470
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BashCommandConstructor.java b/src/main/java/com/google/devtools/build/lib/analysis/BashCommandConstructor.java
new file mode 100644
index 0000000..41b2488
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/BashCommandConstructor.java
@@ -0,0 +1,50 @@
+// Copyright 2019 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.analysis;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
+import com.google.devtools.build.lib.vfs.PathFragment;
+
+/** The class for constructing command line for Bash. */
+public final class BashCommandConstructor implements CommandConstructor {
+
+ private final PathFragment shellPath;
+ private final String scriptNameSuffix;
+
+ BashCommandConstructor(PathFragment shellPath, String scriptNameSuffix) {
+ this.shellPath = shellPath;
+ this.scriptNameSuffix = scriptNameSuffix;
+ }
+
+ @Override
+ public ImmutableList<String> asExecArgv(Artifact scriptFileArtifact) {
+ return ImmutableList.of(shellPath.getPathString(), scriptFileArtifact.getExecPathString());
+ }
+
+ @Override
+ public ImmutableList<String> asExecArgv(String command) {
+ return ImmutableList.of(shellPath.getPathString(), "-c", command);
+ }
+
+ @Override
+ public Artifact commandAsScript(RuleContext ruleContext, String command) {
+ String scriptFileName = ruleContext.getTarget().getName() + scriptNameSuffix;
+ String scriptFileContents = "#!/bin/bash\n" + command;
+ return FileWriteAction.createFile(
+ ruleContext, scriptFileName, scriptFileContents, /*executable=*/ true);
+ }
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommandConstructor.java b/src/main/java/com/google/devtools/build/lib/analysis/CommandConstructor.java
new file mode 100644
index 0000000..6fd2023
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CommandConstructor.java
@@ -0,0 +1,37 @@
+// Copyright 2019 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.analysis;
+
+import com.google.common.collect.ImmutableList;
+import com.google.devtools.build.lib.actions.Artifact;
+
+/**
+ * The interface to construct command line for different shells (Bash, Batch, Powershell). Used in
+ * {@link com.google.devtools.build.lib.analysis.CommandHelper}
+ */
+public interface CommandConstructor {
+
+ /**
+ * Given a string of command, return the arguments to run the command. eg. For Bash command,
+ * asExecArgv("foo bar") -> ["/bin/bash", "-c", "foo bar"]
+ */
+ ImmutableList<String> asExecArgv(String command);
+
+ /** Given an artifact of a script, return the arguments to run this command. */
+ ImmutableList<String> asExecArgv(Artifact scriptFileArtifact);
+
+ /** Write the command to a script and return the artifact of the script. */
+ Artifact commandAsScript(RuleContext ruleContext, String command);
+}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
index 2cd8184..4572e11 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/CommandHelper.java
@@ -23,7 +23,6 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.ExecutionRequirements;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
-import com.google.devtools.build.lib.analysis.actions.FileWriteAction;
import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.collect.nestedset.NestedSet;
@@ -283,38 +282,19 @@
}
private static Pair<List<String>, Artifact> buildCommandLineMaybeWithScriptFile(
- RuleContext ruleContext, String command, String scriptPostFix, PathFragment shellPath) {
+ RuleContext ruleContext, String command, CommandConstructor constructor) {
List<String> argv;
Artifact scriptFileArtifact = null;
if (command.length() <= maxCommandLength) {
- argv = buildCommandLineSimpleArgv(command, shellPath);
+ argv = constructor.asExecArgv(command);
} else {
// Use script file.
- scriptFileArtifact = buildCommandLineArtifact(ruleContext, command, scriptPostFix);
- argv = buildCommandLineArgvWithArtifact(scriptFileArtifact, shellPath);
+ scriptFileArtifact = constructor.commandAsScript(ruleContext, command);
+ argv = constructor.asExecArgv(scriptFileArtifact);
}
return Pair.of(argv, scriptFileArtifact);
}
- private static ImmutableList<String> buildCommandLineArgvWithArtifact(Artifact scriptFileArtifact,
- PathFragment shellPath) {
- return ImmutableList.of(shellPath.getPathString(), scriptFileArtifact.getExecPathString());
- }
-
- private static Artifact buildCommandLineArtifact(RuleContext ruleContext, String command,
- String scriptPostFix) {
- String scriptFileName = ruleContext.getTarget().getName() + scriptPostFix;
- String scriptFileContents = "#!/bin/bash\n" + command;
- Artifact scriptFileArtifact = FileWriteAction.createFile(
- ruleContext, scriptFileName, scriptFileContents, /*executable=*/true);
- return scriptFileArtifact;
- }
-
- private static ImmutableList<String> buildCommandLineSimpleArgv(String command,
- PathFragment shellPath) {
- return ImmutableList.of(shellPath.getPathString(), "-c", command);
- }
-
/**
* If {@code command} is too long, creates a helper shell script that runs that command.
*
@@ -324,49 +304,24 @@
* this method does nothing and returns null.
*/
@Nullable
- public static Artifact shellCommandHelperScriptMaybe(
- RuleContext ruleCtx,
- String command,
- String scriptPostFix,
- Map<String, String> executionInfo) {
+ public static Artifact commandHelperScriptMaybe(
+ RuleContext ruleCtx, String command, CommandConstructor constructor) {
if (command.length() <= maxCommandLength) {
return null;
} else {
- return buildCommandLineArtifact(ruleCtx, command, scriptPostFix);
+ return constructor.commandAsScript(ruleCtx, command);
}
}
/**
- * Builds the set of command-line arguments. Creates a bash script if the command line is longer
- * than the allowed maximum {@link #maxCommandLength}. Fixes up the input artifact list with the
- * created bash script when required.
- */
- public List<String> buildCommandLine(
- PathFragment shExecutable,
- String command,
- NestedSetBuilder<Artifact> inputs,
- String scriptPostFix) {
- return buildCommandLine(
- shExecutable, command, inputs, scriptPostFix, ImmutableMap.<String, String>of());
- }
-
- /**
* Builds the set of command-line arguments using the specified shell path. Creates a bash script
* if the command line is longer than the allowed maximum {@link #maxCommandLength}. Fixes up the
* input artifact list with the created bash script when required.
- *
- * @param executionInfo an execution info map of the action associated with the command line to be
- * built.
*/
public List<String> buildCommandLine(
- PathFragment shExecutable,
- String command,
- NestedSetBuilder<Artifact> inputs,
- String scriptPostFix,
- Map<String, String> executionInfo) {
+ String command, NestedSetBuilder<Artifact> inputs, CommandConstructor constructor) {
Pair<List<String>, Artifact> argvAndScriptFile =
- buildCommandLineMaybeWithScriptFile(
- ruleContext, command, scriptPostFix, shellPath(executionInfo, shExecutable));
+ buildCommandLineMaybeWithScriptFile(ruleContext, command, constructor);
if (argvAndScriptFile.second != null) {
inputs.add(argvAndScriptFile.second);
}
@@ -379,14 +334,9 @@
* created bash script when required.
*/
public List<String> buildCommandLine(
- PathFragment shExecutable,
- String command,
- List<Artifact> inputs,
- String scriptPostFix,
- Map<String, String> executionInfo) {
+ String command, List<Artifact> inputs, CommandConstructor constructor) {
Pair<List<String>, Artifact> argvAndScriptFile =
- buildCommandLineMaybeWithScriptFile(
- ruleContext, command, scriptPostFix, shellPath(executionInfo, shExecutable));
+ buildCommandLineMaybeWithScriptFile(ruleContext, command, constructor);
if (argvAndScriptFile.second != null) {
inputs.add(argvAndScriptFile.second);
}
@@ -394,10 +344,16 @@
}
/** Returns the path to the shell for an action with the given execution requirements. */
- private PathFragment shellPath(Map<String, String> executionInfo, PathFragment shExecutable) {
+ private static PathFragment shellPath(
+ Map<String, String> executionInfo, PathFragment shExecutable) {
// Use vanilla /bin/bash for actions running on mac machines.
return executionInfo.containsKey(ExecutionRequirements.REQUIRES_DARWIN)
? PathFragment.create("/bin/bash")
: shExecutable;
}
+
+ public static BashCommandConstructor buildBashCommandConstructor(
+ Map<String, String> executionInfo, PathFragment shExecutable, String scriptPostFix) {
+ return new BashCommandConstructor(shellPath(executionInfo, shExecutable), scriptPostFix);
+ }
}
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionSpec.java b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionSpec.java
index 5c17eff..56b1786 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionSpec.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/extra/ExtraActionSpec.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.actions.Artifact;
import com.google.devtools.build.lib.actions.CommandLine;
import com.google.devtools.build.lib.actions.RunfilesSupplier;
+import com.google.devtools.build.lib.analysis.BashCommandConstructor;
import com.google.devtools.build.lib.analysis.CommandHelper;
import com.google.devtools.build.lib.analysis.RuleContext;
import com.google.devtools.build.lib.analysis.TransitiveInfoProvider;
@@ -129,13 +130,10 @@
actionToShadow.getPrimaryOutput().getExecPath().getBaseName()
+ "."
+ actionToShadow.getKey(owningRule.getActionKeyContext());
- List<String> argv =
- commandHelper.buildCommandLine(
- shExecutable,
- command,
- extraActionInputs,
- "." + actionUniquifier + ".extra_action_script.sh",
- executionInfo);
+ BashCommandConstructor constructor =
+ CommandHelper.buildBashCommandConstructor(
+ executionInfo, shExecutable, "." + actionUniquifier + ".extra_action_script.sh");
+ List<String> argv = commandHelper.buildCommandLine(command, extraActionInputs, constructor);
String commandMessage = String.format("Executing extra_action %s on %s", label, ownerLabel);
owningRule.registerAction(
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
index 16dea75..9cfd02e 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkActionFactory.java
@@ -34,6 +34,7 @@
import com.google.devtools.build.lib.actions.SingleStringArgFormatter;
import com.google.devtools.build.lib.actions.extra.ExtraActionInfo;
import com.google.devtools.build.lib.actions.extra.SpawnInfo;
+import com.google.devtools.build.lib.analysis.BashCommandConstructor;
import com.google.devtools.build.lib.analysis.CommandHelper;
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.PseudoAction;
@@ -324,10 +325,12 @@
ImmutableMap.copyOf(TargetUtils.getExecutionInfo(ruleContext.getRule()));
String helperScriptSuffix = String.format(".run_shell_%d.sh", runShellOutputCounter++);
String command = (String) commandUnchecked;
- Artifact helperScript =
- CommandHelper.shellCommandHelperScriptMaybe(
- ruleContext, command, helperScriptSuffix, executionInfo);
PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext);
+ BashCommandConstructor constructor =
+ CommandHelper.buildBashCommandConstructor(
+ executionInfo, shExecutable, helperScriptSuffix);
+ Artifact helperScript =
+ CommandHelper.commandHelperScriptMaybe(ruleContext, command, constructor);
if (helperScript == null) {
builder.setShellCommand(shExecutable, command);
} else {
diff --git a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
index d9c821e..51e1980 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/skylark/SkylarkRuleContext.java
@@ -30,6 +30,7 @@
import com.google.devtools.build.lib.actions.ArtifactRoot;
import com.google.devtools.build.lib.analysis.ActionsProvider;
import com.google.devtools.build.lib.analysis.AliasProvider;
+import com.google.devtools.build.lib.analysis.BashCommandConstructor;
import com.google.devtools.build.lib.analysis.CommandHelper;
import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext;
import com.google.devtools.build.lib.analysis.DefaultInfo;
@@ -1067,15 +1068,15 @@
String.class,
"execution_requirements"));
PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext);
- List<String> argv =
- helper.buildCommandLine(
+
+ BashCommandConstructor constructor =
+ CommandHelper.buildBashCommandConstructor(
+ executionRequirements,
shExecutable,
- command,
- inputs,
// Hash the command-line to prevent multiple actions from the same rule invocation
// conflicting with each other.
- "." + Hashing.murmur3_32().hashUnencodedChars(command).toString() + SCRIPT_SUFFIX,
- executionRequirements);
+ "." + Hashing.murmur3_32().hashUnencodedChars(command).toString() + SCRIPT_SUFFIX);
+ List<String> argv = helper.buildCommandLine(command, inputs, constructor);
return Tuple.<Object>of(
MutableList.copyOf(env, inputs),
MutableList.copyOf(env, argv),
diff --git a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
index 37f048c..1557657 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/genrule/GenRuleBase.java
@@ -23,6 +23,7 @@
import com.google.devtools.build.lib.actions.CompositeRunfilesSupplier;
import com.google.devtools.build.lib.actions.MutableActionGraph.ActionConflictException;
import com.google.devtools.build.lib.analysis.AliasProvider;
+import com.google.devtools.build.lib.analysis.CommandConstructor;
import com.google.devtools.build.lib.analysis.CommandHelper;
import com.google.devtools.build.lib.analysis.ConfigurationMakeVariableContext;
import com.google.devtools.build.lib.analysis.ConfiguredTarget;
@@ -167,13 +168,12 @@
if (ruleContext.hasErrors()) {
return null;
}
- List<String> argv =
- commandHelper.buildCommandLine(
- shExecutable,
- command,
- inputs,
- ".genrule_script.sh",
- ImmutableMap.copyOf(executionInfo));
+
+ CommandConstructor constructor =
+ CommandHelper.buildBashCommandConstructor(
+ executionInfo, shExecutable, ".genrule_script.sh");
+
+ List<String> argv = commandHelper.buildCommandLine(command, inputs, constructor);
if (isStampingEnabled(ruleContext)) {
inputs.add(ruleContext.getAnalysisEnvironment().getStableWorkspaceStatusArtifact());
