From 84d3097537f045a78baaebc3c6f2ffef4d814cb5 Mon Sep 17 00:00:00 2001 From: laszlocsomor Date: Mon, 23 Apr 2018 01:15:01 -0700 Subject: [PATCH] SpawnAction.setShellCommand: expect shell path SpawnAction.setShellCommand(String) now expects the shell interpreter's path as an argument. This change enables two things: - rules can report an error if the shell is missing - SpawnAction no longer has to know about a default shell The new ShToolchain class will later also be responsible for retrieving the active shell toolchain (added in https://github.com/bazelbuild/bazel/commit/81ed3add408adb20bddbc3ba1818c65806738dc5). This change brings Bazel closer to not depend on the shell unless it has to (e.g. to run shell scripts). See https://github.com/bazelbuild/bazel/issues/4319 RELNOTES: none PiperOrigin-RevId: 193885943 --- .../build/lib/analysis/CommandHelper.java | 2 +- .../build/lib/analysis/ShToolchain.java | 62 +++++++++++++++++++ .../lib/analysis/actions/SpawnAction.java | 29 +++------ .../skylark/SkylarkActionFactory.java | 6 +- .../lib/analysis/test/TestActionBuilder.java | 28 ++++++--- .../lib/analysis/test/TestRunnerAction.java | 18 +++--- .../bazel/rules/java/BazelJavaSemantics.java | 16 ++--- .../rules/python/BazelPythonSemantics.java | 3 + .../build/lib/bazel/rules/sh/ShBinary.java | 25 +++----- .../devtools/build/lib/exec/TestStrategy.java | 4 +- .../lib/rules/objc/CompilationSupport.java | 4 +- .../lib/runtime/commands/RunCommand.java | 44 ++++++++----- 12 files changed, 155 insertions(+), 86 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/analysis/ShToolchain.java 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 f364257c685d83..065d3074ad311e 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 @@ -307,6 +307,6 @@ private PathFragment shellPath(Map executionInfo) { // Use vanilla /bin/bash for actions running on mac machines. return executionInfo.containsKey(ExecutionRequirements.REQUIRES_DARWIN) ? PathFragment.create("/bin/bash") - : ruleContext.getConfiguration().getFragment(ShellConfiguration.class).getShellExecutable(); + : ShToolchain.getPathOrError(ruleContext); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/ShToolchain.java b/src/main/java/com/google/devtools/build/lib/analysis/ShToolchain.java new file mode 100644 index 00000000000000..c785da410e2bae --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/analysis/ShToolchain.java @@ -0,0 +1,62 @@ +// Copyright 2018 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.devtools.build.lib.analysis.config.BuildConfiguration; +import com.google.devtools.build.lib.vfs.PathFragment; + +/** Class to work with the shell toolchain, e.g. get the shell interpreter's path. */ +public final class ShToolchain { + + /** + * Returns the shell executable's path, or an empty path if not set. + * + *

This method checks the configuration's {@link ShellConfiguration} fragment. + */ + public static PathFragment getPath(BuildConfiguration config) { + PathFragment result = PathFragment.EMPTY_FRAGMENT; + + ShellConfiguration configFragment = + (ShellConfiguration) config.getFragment(ShellConfiguration.class); + if (configFragment != null) { + PathFragment path = configFragment.getShellExecutable(); + if (path != null) { + result = path; + } + } + + return result; + } + + /** + * Returns the shell executable's path, or reports a rule error if the path is empty. + * + *

This method checks the rule's configuration's {@link ShellConfiguration} fragment for the + * shell executable's path. If null or empty, the method reports an error against the rule. + */ + public static PathFragment getPathOrError(RuleContext ctx) { + PathFragment result = getPath(ctx.getConfiguration()); + + if (result.isEmpty()) { + ctx.ruleError( + "This rule needs a shell interpreter. Use the --shell_executable= flag to specify" + + " the interpreter's path, e.g. --shell_executable=/usr/local/bin/bash"); + } + + return result; + } + + private ShToolchain() {} +} diff --git a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java index 8f9e18aa97750b..c9a5a2dddc9cce 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/actions/SpawnAction.java @@ -56,7 +56,6 @@ import com.google.devtools.build.lib.actions.extra.SpawnInfo; import com.google.devtools.build.lib.analysis.AnalysisEnvironment; import com.google.devtools.build.lib.analysis.FilesToRunProvider; -import com.google.devtools.build.lib.analysis.ShellConfiguration; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.collect.nestedset.NestedSet; @@ -680,8 +679,7 @@ private CommandLine buildCommandLine( AnalysisEnvironment analysisEnvironment, BuildConfiguration configuration, List paramFileActions) { - ImmutableList executableArgs = buildExecutableArgs( - configuration.getFragment(ShellConfiguration.class).getShellExecutable()); + ImmutableList executableArgs = buildExecutableArgs(); boolean hasConditionalParamFile = commandLines.stream().anyMatch(c -> c.paramFileInfo != null && !c.paramFileInfo.always()); boolean spillToParamFiles = false; @@ -812,7 +810,7 @@ SpawnAction buildSpawnAction( */ CommandLine buildCommandLineWithoutParamsFiles() { SpawnActionCommandLine.Builder result = new SpawnActionCommandLine.Builder(); - ImmutableList executableArgs = buildExecutableArgs(null); + ImmutableList executableArgs = buildExecutableArgs(); result.addExecutableArguments(executableArgs); for (CommandLineAndParamFileInfo commandLineAndParamFileInfo : commandLines) { result.addCommandLine(commandLineAndParamFileInfo.commandLine); @@ -851,12 +849,7 @@ protected SpawnAction createSpawnAction( extraActionInfoSupplier); } - private ImmutableList buildExecutableArgs( - @Nullable PathFragment defaultShellExecutable) { - if (isShellCommand && executable == null) { - Preconditions.checkNotNull(defaultShellExecutable); - executable = defaultShellExecutable; - } + private ImmutableList buildExecutableArgs() { Preconditions.checkNotNull(executable); Preconditions.checkNotNull(executableArgs); @@ -1119,18 +1112,16 @@ public Builder setJarExecutable(PathFragment javaExecutable, } /** - * Sets the executable to be the shell and adds the given command as the - * command to be executed. + * Sets the executable to be the shell and adds the given command as the command to be executed. * - *

Note that this will not clear the arguments, so any arguments will - * be passed in addition to the command given here. + *

Note that this will not clear the arguments, so any arguments will be passed in addition + * to the command given here. * - *

Calling this method overrides any previous values set via calls to - * {@link #setExecutable(Artifact)}, {@link #setJavaExecutable}, or - * {@link #setShellCommand(String)}. + *

Calling this method overrides any previous values set via calls to {@link + * #setExecutable(Artifact)}, {@link #setJavaExecutable}, or {@link #setShellCommand(String)}. */ - public Builder setShellCommand(String command) { - this.executable = null; + public Builder setShellCommand(PathFragment shExecutable, String command) { + this.executable = shExecutable; // 0=shell command switch, 1=command this.executableArgs = Lists.newArrayList("-c", command); this.isShellCommand = true; 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 416a62a5a078af..4f09dde901d644 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 @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.PseudoAction; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.actions.ActionConstructionContext; import com.google.devtools.build.lib.analysis.actions.FileWriteAction; import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction; @@ -654,10 +655,11 @@ public void runShell( Artifact helperScript = CommandHelper.shellCommandHelperScriptMaybe( ruleContext, command, helperScriptSuffix, executionInfo); + PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext); if (helperScript == null) { - builder.setShellCommand(command); + builder.setShellCommand(shExecutable, command); } else { - builder.setShellCommand(helperScript.getExecPathString()); + builder.setShellCommand(shExecutable, helperScript.getExecPathString()); builder.addInput(helperScript); FilesToRunProvider provider = context.getExecutableRunfiles(helperScript); if (provider != null) { diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java index bbce3496386ea6..5299d9a8b61f1d 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestActionBuilder.java @@ -28,6 +28,7 @@ import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.RunfilesSupport; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.configuredtargets.RuleConfiguredTarget.Mode; @@ -306,14 +307,25 @@ private TestParams createTestAction(int shards) { coverageArtifacts.add(coverageArtifact); } - env.registerAction(new TestRunnerAction( - ruleContext.getActionOwner(), inputs, - testSetupScript, collectCoverageScript, - testLog, cacheStatus, - coverageArtifact, - testProperties, extraTestEnv, executionSettings, - shard, run, config, ruleContext.getWorkspaceName(), - useTestRunner)); + PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext); + env.registerAction( + new TestRunnerAction( + ruleContext.getActionOwner(), + inputs, + testSetupScript, + collectCoverageScript, + testLog, + cacheStatus, + coverageArtifact, + testProperties, + extraTestEnv, + executionSettings, + shard, + run, + config, + ruleContext.getWorkspaceName(), + shExecutable, + useTestRunner)); results.add(cacheStatus); } } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java index b8baf1838aed46..eb754bdbf4d4bc 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/test/TestRunnerAction.java @@ -33,7 +33,6 @@ import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.NotifyOnActionCacheHit; import com.google.devtools.build.lib.analysis.RunfilesSupplierImpl; -import com.google.devtools.build.lib.analysis.ShellConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.RunUnder; import com.google.devtools.build.lib.buildeventstream.TestFileNameConstants; @@ -80,6 +79,7 @@ public class TestRunnerAction extends AbstractAction implements NotifyOnActionCa private final Artifact cacheStatus; private final PathFragment testWarningsPath; private final PathFragment unusedRunfilesLogPath; + private final PathFragment shExecutable; private final PathFragment splitLogsPath; private final PathFragment splitLogsDir; private final PathFragment undeclaredOutputsDir; @@ -125,18 +125,18 @@ private static ImmutableList list(Artifact... artifacts) { } /** - * Create new TestRunnerAction instance. Should not be called directly. - * Use {@link TestActionBuilder} instead. + * Create new TestRunnerAction instance. Should not be called directly. Use {@link + * TestActionBuilder} instead. * - * @param shardNum The shard number. Must be 0 if totalShards == 0 - * (no sharding). Otherwise, must be >= 0 and < totalShards. + * @param shardNum The shard number. Must be 0 if totalShards == 0 (no sharding). Otherwise, must + * be >= 0 and < totalShards. * @param runNumber test run number */ TestRunnerAction( ActionOwner owner, Iterable inputs, - Artifact testSetupScript, // Must be in inputs - @Nullable Artifact collectCoverageScript, // Must be in inputs, if not null + Artifact testSetupScript, // Must be in inputs + @Nullable Artifact collectCoverageScript, // Must be in inputs, if not null Artifact testLog, Artifact cacheStatus, Artifact coverageArtifact, @@ -147,6 +147,7 @@ private static ImmutableList list(Artifact... artifacts) { int runNumber, BuildConfiguration configuration, String workspaceName, + PathFragment shExecutable, boolean useTestRunner) { super( owner, @@ -182,6 +183,7 @@ private static ImmutableList list(Artifact... artifacts) { this.testWarningsPath = baseDir.getChild("test.warnings"); this.unusedRunfilesLogPath = baseDir.getChild("test.unused_runfiles_log"); this.testStderr = baseDir.getChild("test.err"); + this.shExecutable = shExecutable; this.splitLogsDir = baseDir.getChild("test.raw_splitlogs"); // See note in {@link #getSplitLogsPath} on the choice of file name. this.splitLogsPath = splitLogsDir.getChild("test.splitlogs"); @@ -756,7 +758,7 @@ public Artifact getTestSetupScript() { } public PathFragment getShExecutable() { - return configuration.getFragment(ShellConfiguration.class).getShellExecutable(); + return shExecutable; } public ImmutableMap getLocalShellEnvironment() { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java index 50cdc3fed614e5..13879ef534768a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/java/BazelJavaSemantics.java @@ -25,7 +25,7 @@ import com.google.devtools.build.lib.analysis.RuleConfiguredTargetBuilder; import com.google.devtools.build.lib.analysis.RuleContext; import com.google.devtools.build.lib.analysis.Runfiles; -import com.google.devtools.build.lib.analysis.ShellConfiguration; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction; @@ -395,25 +395,17 @@ public Artifact createStubAction( if (OS.getCurrent() == OS.WINDOWS) { Artifact newExecutable = ruleContext.getImplicitOutputArtifact(ruleContext.getTarget().getName() + ".cmd"); + PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext); ruleContext.registerAction( new TemplateExpansionAction( ruleContext.getActionOwner(), newExecutable, STUB_SCRIPT_WINDOWS, ImmutableList.of( - Substitution.of( - "%bash_exe_path%", - ruleContext - .getFragment(ShellConfiguration.class) - .getShellExecutable() - .getPathString()), + Substitution.of("%bash_exe_path%", shExecutable.getPathString()), Substitution.of( "%cygpath_exe_path%", - ruleContext - .getFragment(ShellConfiguration.class) - .getShellExecutable() - .replaceName("cygpath.exe") - .getPathString())), + shExecutable.replaceName("cygpath.exe").getPathString())), true)); return newExecutable; } else { diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java index 8f9ede020b490f..55e3b957751a30 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/python/BazelPythonSemantics.java @@ -25,6 +25,7 @@ import com.google.devtools.build.lib.analysis.Runfiles.Builder; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.RunfilesSupport; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction; import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction.LaunchInfo; @@ -169,11 +170,13 @@ public Artifact createExecutable( true)); if (OS.getCurrent() != OS.WINDOWS) { + PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext); ruleContext.registerAction( new SpawnAction.Builder() .addInput(zipFile) .addOutput(executable) .setShellCommand( + shExecutable, "echo '#!/usr/bin/env python' | cat - " + zipFile.getExecPathString() + " > " diff --git a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java index 19de0229103324..94c4636dc2961e 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/rules/sh/ShBinary.java @@ -23,7 +23,7 @@ import com.google.devtools.build.lib.analysis.Runfiles; import com.google.devtools.build.lib.analysis.RunfilesProvider; import com.google.devtools.build.lib.analysis.RunfilesSupport; -import com.google.devtools.build.lib.analysis.ShellConfiguration; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.actions.ExecutableSymlinkAction; import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction; import com.google.devtools.build.lib.analysis.actions.LauncherFileWriteAction.LaunchInfo; @@ -34,6 +34,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.util.OS; +import com.google.devtools.build.lib.vfs.PathFragment; /** * Implementation for the sh_binary rule. @@ -101,8 +102,8 @@ private static boolean isWindowsExecutable(Artifact artifact) { || artifact.getExtension().equals("bat"); } - private static Artifact createWindowsExeLauncher(RuleContext ruleContext) - throws RuleErrorException { + private static Artifact createWindowsExeLauncher( + RuleContext ruleContext, PathFragment shExecutable) throws RuleErrorException { Artifact bashLauncher = ruleContext.getImplicitOutputArtifact(ruleContext.getTarget().getName() + ".exe"); @@ -110,12 +111,7 @@ private static Artifact createWindowsExeLauncher(RuleContext ruleContext) LaunchInfo.builder() .addKeyValuePair("binary_type", "Bash") .addKeyValuePair("workspace_name", ruleContext.getWorkspaceName()) - .addKeyValuePair( - "bash_bin_path", - ruleContext - .getFragment(ShellConfiguration.class) - .getShellExecutable() - .getPathString()) + .addKeyValuePair("bash_bin_path", shExecutable.getPathString()) .build(); LauncherFileWriteAction.createAndRegister(ruleContext, bashLauncher, launchInfo); @@ -138,8 +134,9 @@ private static Artifact launcherForWindows( } } + PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext); if (ruleContext.getConfiguration().enableWindowsExeLauncher()) { - return createWindowsExeLauncher(ruleContext); + return createWindowsExeLauncher(ruleContext, shExecutable); } Artifact wrapper = @@ -149,13 +146,7 @@ private static Artifact launcherForWindows( ruleContext.getActionOwner(), wrapper, STUB_SCRIPT_WINDOWS, - ImmutableList.of( - Substitution.of( - "%bash_exe_path%", - ruleContext - .getFragment(ShellConfiguration.class) - .getShellExecutable() - .getPathString())), + ImmutableList.of(Substitution.of("%bash_exe_path%", shExecutable.getPathString())), true)); return wrapper; } diff --git a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java index 87ebde4199a219..ae655519275f5b 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/exec/TestStrategy.java @@ -28,7 +28,6 @@ import com.google.devtools.build.lib.actions.SpawnResult; import com.google.devtools.build.lib.actions.TestExecException; import com.google.devtools.build.lib.actions.UserExecException; -import com.google.devtools.build.lib.analysis.ShellConfiguration; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.PerLabelOptions; import com.google.devtools.build.lib.analysis.test.TestActionContext; @@ -185,8 +184,7 @@ private static void addRunUnderArgs(TestRunnerAction testAction, List ar // the --run_under parameter and getCommand only returns the first such token. boolean needsShell = !command.contains("/"); if (needsShell) { - args.add(testAction.getConfiguration().getFragment(ShellConfiguration.class) - .getShellExecutable().getPathString()); + args.add(testAction.getShExecutable().getPathString()); args.add("-c"); args.add("\"$@\""); args.add("/bin/sh"); // Sets $0. diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index 2fd613481e744f..30b57a00f2ccfe 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java @@ -64,6 +64,7 @@ import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine; import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg; @@ -1416,10 +1417,11 @@ private CompilationSupport registerDsymActions(DsymOutputType dsymOutputType) { debugSymbolFileZipEntry, debugSymbolFile.getExecPathString())); + PathFragment shExecutable = ShToolchain.getPathOrError(ruleContext); ruleContext.registerAction( new SpawnAction.Builder() .setMnemonic("UnzipDsym") - .setShellCommand(unzipDsymCommand.toString()) + .setShellCommand(shExecutable, unzipDsymCommand.toString()) .addInput(tempDsymBundleZip) .addOutput(dsymPlist) .addOutput(debugSymbolFile) diff --git a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java index 16cbf313634f28..5670569d3d1fbb 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/commands/RunCommand.java @@ -26,7 +26,7 @@ import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.FilesToRunProvider; import com.google.devtools.build.lib.analysis.RunfilesSupport; -import com.google.devtools.build.lib.analysis.ShellConfiguration; +import com.google.devtools.build.lib.analysis.ShToolchain; import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.analysis.config.RunUnder; import com.google.devtools.build.lib.analysis.test.TestProvider; @@ -244,7 +244,7 @@ private BlazeCommandResult runTargetUnderServer(CommandEnvironment env, String unisolatedCommand = CommandFailureUtils.describeCommand( CommandDescriptionForm.COMPLETE_UNISOLATED, cmdLine, null, workingDir.getPathString()); - if (writeScript(env, runOptions.scriptPath, unisolatedCommand)) { + if (writeScript(env, shellExecutable, runOptions.scriptPath, unisolatedCommand)) { return BlazeCommandResult.exitCode(ExitCode.SUCCESS); } else { return BlazeCommandResult.exitCode(ExitCode.RUN_FAILURE); @@ -450,12 +450,21 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options) } } - PathFragment shellExecutable = - configuration.getFragment(ShellConfiguration.class).getShellExecutable(); + // TODO(laszlocsomor): change RunCommand to not require a shell and not write a shell script, + // then remove references to ShToolchain. See https://github.com/bazelbuild/bazel/issues/4319 + PathFragment shExecutable = ShToolchain.getPath(configuration); + if (shExecutable.isEmpty()) { + env.getReporter() + .handle( + Event.error( + "the \"run\" command needs a shell; use the --shell_executable= flag " + + "to specify its path, e.g. --shell_executable=/usr/local/bin/bash")); + return BlazeCommandResult.exitCode(ExitCode.COMMAND_LINE_ERROR); + } if (!runOptions.direct) { return runTargetUnderServer( - env, targetToRun, shellExecutable, runUnderTarget, runfilesDir, commandLineArgs); + env, targetToRun, shExecutable, runUnderTarget, runfilesDir, commandLineArgs); } Map runEnvironment = new TreeMap<>(); @@ -511,15 +520,15 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options) } else { workingDir = runfilesDir; List args = computeArgs(env, targetToRun, commandLineArgs); - constructCommandLine(cmdLine, prettyCmdLine, env, - shellExecutable, targetToRun, runUnderTarget, args); + constructCommandLine( + cmdLine, prettyCmdLine, env, shExecutable, targetToRun, runUnderTarget, args); } if (runOptions.scriptPath != null) { String unisolatedCommand = CommandFailureUtils.describeCommand( CommandDescriptionForm.COMPLETE_UNISOLATED, cmdLine, runEnvironment, workingDir.getPathString()); - if (writeScript(env, runOptions.scriptPath, unisolatedCommand)) { + if (writeScript(env, shExecutable, runOptions.scriptPath, unisolatedCommand)) { return BlazeCommandResult.exitCode(ExitCode.SUCCESS); } else { return BlazeCommandResult.exitCode(ExitCode.RUN_FAILURE); @@ -547,10 +556,9 @@ public BlazeCommandResult exec(CommandEnvironment env, OptionsProvider options) .setWorkingDirectory( ByteString.copyFrom(workingDir.getPathString(), StandardCharsets.ISO_8859_1)); - ImmutableList shellCmdLine = ImmutableList.of( - shellExecutable.getPathString(), - "-c", - ShellEscaper.escapeJoinAll(cmdLine)); + ImmutableList shellCmdLine = + ImmutableList.of( + shExecutable.getPathString(), "-c", ShellEscaper.escapeJoinAll(cmdLine)); for (String arg : shellCmdLine) { execDescription.addArgv(ByteString.copyFrom(arg, StandardCharsets.ISO_8859_1)); @@ -612,11 +620,17 @@ private Path ensureRunfilesBuilt(CommandEnvironment env, RunfilesSupport runfile return workingDir; } - private boolean writeScript(CommandEnvironment env, PathFragment scriptPathFrag, String cmd) { + private boolean writeScript( + CommandEnvironment env, + PathFragment shellExecutable, + PathFragment scriptPathFrag, + String cmd) { Path scriptPath = env.getWorkingDirectory().getRelative(scriptPathFrag); try { - FileSystemUtils.writeContent(scriptPath, StandardCharsets.ISO_8859_1, - "#!/bin/sh\n" + cmd + " \"$@\""); + FileSystemUtils.writeContent( + scriptPath, + StandardCharsets.ISO_8859_1, + "#!" + shellExecutable.getPathString() + "\n" + cmd + " \"$@\""); scriptPath.setExecutable(true); } catch (IOException e) { env.getReporter().handle(Event.error("Error writing run script:" + e.getMessage()));