diff options
3 files changed, 134 insertions, 5 deletions
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 70afd0f1b3..330c017a1d 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 @@ -246,6 +246,27 @@ public final class CommandHelper { } /** + * If {@code command} is too long, creates a helper shell script that runs that command. + * + * <p>Returns the {@link Artifact} corresponding to that script. + * + * <p>Otherwise, when {@code command} is shorter than the platform's shell's command length limit, + * this method does nothing and returns null. + */ + @Nullable + public static Artifact shellCommandHelperScriptMaybe( + RuleContext ruleCtx, + String command, + String scriptPostFix, + Map<String, String> executionInfo) { + if (command.length() <= maxCommandLength) { + return null; + } else { + return buildCommandLineArtifact(ruleCtx, command, scriptPostFix); + } + } + + /** * 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. 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 88080e05eb..2433cdb6c7 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 @@ -20,6 +20,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.actions.Root; import com.google.devtools.build.lib.actions.RunfilesSupplier; import com.google.devtools.build.lib.actions.extra.SpawnInfo; +import com.google.devtools.build.lib.analysis.CommandHelper; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.PseudoAction; import com.google.devtools.build.lib.analysis.RuleContext; @@ -34,6 +35,7 @@ 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.EventHandler; import com.google.devtools.build.lib.events.Location; +import com.google.devtools.build.lib.packages.TargetUtils; import com.google.devtools.build.lib.skylarkinterface.Param; import com.google.devtools.build.lib.skylarkinterface.ParamType; import com.google.devtools.build.lib.skylarkinterface.SkylarkCallable; @@ -72,6 +74,8 @@ public class SkylarkActionFactory implements SkylarkValue { private final SkylarkRuleContext context; private final SkylarkSemanticsOptions skylarkSemanticsOptions; private RuleContext ruleContext; + /** Counter for actions.run_shell helper scripts. Every script must have a unique name. */ + private int runShellOutputCounter = 0; public SkylarkActionFactory( SkylarkRuleContext context, @@ -573,7 +577,23 @@ public class SkylarkActionFactory implements SkylarkValue { } if (commandUnchecked instanceof String) { - builder.setShellCommand((String) commandUnchecked); + Map<String, String> executionInfo = + 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); + if (helperScript == null) { + builder.setShellCommand(command); + } else { + builder.setShellCommand(helperScript.getExecPathString()); + builder.addInput(helperScript); + FilesToRunProvider provider = context.getExecutableRunfiles(helperScript); + if (provider != null) { + builder.addTool(provider); + } + } } else if (commandUnchecked instanceof SkylarkList) { SkylarkList commandList = (SkylarkList) commandUnchecked; if (commandList.size() < 3) { @@ -601,18 +621,20 @@ public class SkylarkActionFactory implements SkylarkValue { } /** - * Stup for spawn actions common between {@link #run} and {@link #runShell}. + * Setup for spawn actions common between {@link #run} and {@link #runShell}. * - * {@code builder} should have either executable or a command set. + * <p>{@code builder} should have either executable or a command set. */ private void registerSpawnAction( SkylarkList outputs, Object inputs, Object mnemonicUnchecked, Object progressMessage, - Boolean useDefaultShellEnv, Object envUnchecked, + Boolean useDefaultShellEnv, + Object envUnchecked, Object executionRequirementsUnchecked, - Object inputManifestsUnchecked, SpawnAction.Builder builder) + Object inputManifestsUnchecked, + SpawnAction.Builder builder) throws EvalException { // TODO(bazel-team): builder still makes unnecessary copies of inputs, outputs and args. Iterable<Artifact> inputArtifacts; diff --git a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java index b5bf2b8698..10402c25ca 100644 --- a/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java +++ b/src/test/java/com/google/devtools/build/lib/skylark/SkylarkRuleContextTest.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.ActionsProvider; import com.google.devtools.build.lib.analysis.FileConfiguredTarget; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; +import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.skylark.SkylarkRuleContext; import com.google.devtools.build.lib.cmdline.Label; @@ -1676,6 +1677,91 @@ public class SkylarkRuleContextTest extends SkylarkTestCase { } @Test + public void testRunShellUsesHelperScriptForLongCommand() throws Exception { + // createRuleContext() gives us the context for a rule upon entry into its analysis function. + // But we need to inspect the result of calling created_actions() after the rule context has + // been modified by creating actions. So we'll call created_actions() from within the analysis + // function and pass it along as a provider. + scratch.file( + "test/rules.bzl", + "def _undertest_impl(ctx):", + " out1 = ctx.outputs.out1", + " out2 = ctx.outputs.out2", + " out3 = ctx.outputs.out3", + " ctx.actions.run_shell(outputs=[out1],", + " command='( %s ; ) > $1' % (", + " ' ; '.join(['echo xxx%d' % i for i in range(0, 7000)])),", + " mnemonic='mnemonic1',", + " arguments=[out1.path])", + " ctx.actions.run_shell(outputs=[out2],", + " command='echo foo > ' + out2.path,", + " mnemonic='mnemonic2')", + " ctx.actions.run_shell(outputs=[out3],", + " command='( %s ; ) > $1' % (", + " ' ; '.join(['echo yyy%d' % i for i in range(0, 7000)])),", + " mnemonic='mnemonic3',", + " arguments=[out3.path])", + " v = ctx.created_actions().by_file", + " return struct(v=v, out1=out1, out2=out2, out3=out3)", + "", + "undertest_rule = rule(", + " implementation=_undertest_impl,", + " outputs={'out1': '%{name}1.txt',", + " 'out2': '%{name}2.txt',", + " 'out3': '%{name}3.txt'},", + " _skylark_testable = True,", + ")", + testingRuleDefinition); + scratch.file("test/BUILD", simpleBuildDefinition); + SkylarkRuleContext ruleContext = createRuleContext("//test:testing"); + + Object mapUnchecked = evalRuleContextCode(ruleContext, "ruleContext.attr.dep.v"); + assertThat(mapUnchecked).isInstanceOf(SkylarkDict.class); + SkylarkDict<?, ?> map = (SkylarkDict<?, ?>) mapUnchecked; + Object out1 = eval("ruleContext.attr.dep.out1"); + Object out2 = eval("ruleContext.attr.dep.out2"); + Object out3 = eval("ruleContext.attr.dep.out3"); + // 5 actions in total: 3 SpawnActions and 2 FileWriteActions for the two long commands. + assertThat(map).hasSize(5); + assertThat(map).containsKey(out1); + assertThat(map).containsKey(out2); + assertThat(map).containsKey(out3); + Object action1Unchecked = map.get(out1); + Object action2Unchecked = map.get(out2); + Object action3Unchecked = map.get(out3); + assertThat(action1Unchecked).isInstanceOf(ActionAnalysisMetadata.class); + assertThat(action2Unchecked).isInstanceOf(ActionAnalysisMetadata.class); + assertThat(action3Unchecked).isInstanceOf(ActionAnalysisMetadata.class); + ActionAnalysisMetadata spawnAction1 = (ActionAnalysisMetadata) action1Unchecked; + ActionAnalysisMetadata spawnAction2 = (ActionAnalysisMetadata) action2Unchecked; + ActionAnalysisMetadata spawnAction3 = (ActionAnalysisMetadata) action3Unchecked; + assertThat(spawnAction1.getMnemonic()).isEqualTo("mnemonic1"); + assertThat(spawnAction2.getMnemonic()).isEqualTo("mnemonic2"); + assertThat(spawnAction3.getMnemonic()).isEqualTo("mnemonic3"); + Artifact helper1 = + Iterables.getOnlyElement( + Iterables.filter( + spawnAction1.getInputs(), a -> a.getFilename().equals("undertest.run_shell_0.sh"))); + assertThat( + Iterables.filter(spawnAction2.getInputs(), a -> a.getFilename().contains("run_shell_"))) + .isEmpty(); + Artifact helper3 = + Iterables.getOnlyElement( + Iterables.filter( + spawnAction3.getInputs(), a -> a.getFilename().equals("undertest.run_shell_2.sh"))); + assertThat(map).containsKey(helper1); + assertThat(map).containsKey(helper3); + Object action4Unchecked = map.get(helper1); + Object action5Unchecked = map.get(helper3); + assertThat(action4Unchecked).isInstanceOf(FileWriteAction.class); + assertThat(action5Unchecked).isInstanceOf(FileWriteAction.class); + FileWriteAction fileWriteAction1 = (FileWriteAction) action4Unchecked; + FileWriteAction fileWriteAction2 = (FileWriteAction) action5Unchecked; + assertThat(fileWriteAction1.getFileContents()).contains("echo xxx6999 ;"); + assertThat(fileWriteAction2.getFileContents()).contains("echo yyy6999 ;"); + } + + @Test public void testFileWriteActionInterface() throws Exception { scratch.file("test/rules.bzl", getSimpleUnderTestDefinition( |