From 19befaf4c8623bf9671e0d8d5f4e713cd8a2c7a1 Mon Sep 17 00:00:00 2001 From: ulfjack Date: Mon, 24 Jul 2017 11:09:40 +0200 Subject: Extract a common AbstractSpawnStrategy parent class This removes a bunch of code duplication that I previously introduced. PiperOrigin-RevId: 162909430 --- .../build/lib/exec/AbstractSpawnStrategy.java | 168 +++++++++++++++++++++ .../lib/remote/RemoteActionContextProvider.java | 4 +- .../build/lib/remote/RemoteSpawnStrategy.java | 149 +----------------- .../build/lib/sandbox/DarwinSandboxedStrategy.java | 27 ++-- .../build/lib/sandbox/LinuxSandboxedStrategy.java | 36 ++--- .../sandbox/ProcessWrapperSandboxedStrategy.java | 25 ++- .../lib/sandbox/SandboxActionContextProvider.java | 81 +++++++--- .../build/lib/sandbox/SandboxStrategy.java | 166 -------------------- .../StandaloneActionContextProvider.java | 29 +++- .../lib/standalone/StandaloneSpawnStrategy.java | 143 +----------------- .../standalone/StandaloneSpawnStrategyTest.java | 11 +- 11 files changed, 303 insertions(+), 536 deletions(-) create mode 100644 src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java delete mode 100644 src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java (limited to 'src') diff --git a/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java new file mode 100644 index 0000000000..f92cd8eb44 --- /dev/null +++ b/src/main/java/com/google/devtools/build/lib/exec/AbstractSpawnStrategy.java @@ -0,0 +1,168 @@ +// Copyright 2017 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.exec; + +import com.google.common.base.Throwables; +import com.google.common.eventbus.EventBus; +import com.google.devtools.build.lib.actions.ActionExecutionContext; +import com.google.devtools.build.lib.actions.ActionInput; +import com.google.devtools.build.lib.actions.ActionInputFileCache; +import com.google.devtools.build.lib.actions.ActionStatusMessage; +import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; +import com.google.devtools.build.lib.actions.EnvironmentalExecException; +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.SandboxedSpawnActionContext; +import com.google.devtools.build.lib.actions.Spawn; +import com.google.devtools.build.lib.actions.SpawnActionContext; +import com.google.devtools.build.lib.actions.Spawns; +import com.google.devtools.build.lib.events.Event; +import com.google.devtools.build.lib.exec.SpawnResult.Status; +import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; +import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; +import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; +import com.google.devtools.build.lib.util.CommandFailureUtils; +import com.google.devtools.build.lib.util.io.FileOutErr; +import com.google.devtools.build.lib.vfs.PathFragment; +import java.io.IOException; +import java.util.SortedMap; +import java.util.concurrent.TimeUnit; +import java.util.concurrent.atomic.AtomicInteger; +import java.util.concurrent.atomic.AtomicReference; + +/** Abstract common ancestor for spawn strategies implementing the common parts. */ +public abstract class AbstractSpawnStrategy implements SandboxedSpawnActionContext { + private final boolean verboseFailures; + private final SpawnInputExpander spawnInputExpander; + private final SpawnRunner spawnRunner; + private final AtomicInteger execCount = new AtomicInteger(); + + public AbstractSpawnStrategy( + boolean verboseFailures, + SpawnRunner spawnRunner) { + this.verboseFailures = verboseFailures; + this.spawnInputExpander = new SpawnInputExpander(false); + this.spawnRunner = spawnRunner; + } + + @Override + public void exec(Spawn spawn, ActionExecutionContext actionExecutionContext) + throws ExecException, InterruptedException { + exec(spawn, actionExecutionContext, null); + } + + @Override + public void exec( + Spawn spawn, + ActionExecutionContext actionExecutionContext, + AtomicReference> writeOutputFiles) + throws ExecException, InterruptedException { + if (actionExecutionContext.reportsSubcommands()) { + actionExecutionContext.reportSubcommand(spawn); + } + final int timeoutSeconds = Spawns.getTimeoutSeconds(spawn); + SpawnExecutionPolicy policy = new SpawnExecutionPolicy() { + private final int id = execCount.incrementAndGet(); + + @Override + public int getId() { + return id; + } + + @Override + public void prefetchInputs(Iterable inputs) throws IOException { + actionExecutionContext.getActionInputPrefetcher().prefetchFiles(inputs); + } + + @Override + public ActionInputFileCache getActionInputFileCache() { + return actionExecutionContext.getActionInputFileCache(); + } + + @Override + public ArtifactExpander getArtifactExpander() { + return actionExecutionContext.getArtifactExpander(); + } + + @Override + public void lockOutputFiles() throws InterruptedException { + Class token = AbstractSpawnStrategy.this.getClass(); + if (writeOutputFiles != null + && writeOutputFiles.get() != token + && !writeOutputFiles.compareAndSet(null, token)) { + throw new InterruptedException(); + } + } + + @Override + public long getTimeoutMillis() { + return TimeUnit.SECONDS.toMillis(timeoutSeconds); + } + + @Override + public FileOutErr getFileOutErr() { + return actionExecutionContext.getFileOutErr(); + } + + @Override + public SortedMap getInputMapping() throws IOException { + return spawnInputExpander.getInputMapping( + spawn, + actionExecutionContext.getArtifactExpander(), + actionExecutionContext.getActionInputFileCache(), + actionExecutionContext.getContext(FilesetActionContext.class)); + } + + @Override + public void report(ProgressStatus state, String name) { + // TODO(ulfjack): We should report more details to the UI. + EventBus eventBus = actionExecutionContext.getEventBus(); + switch (state) { + case EXECUTING: + eventBus.post(ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), name)); + break; + case SCHEDULING: + eventBus.post(ActionStatusMessage.schedulingStrategy(spawn.getResourceOwner())); + break; + default: + break; + } + } + }; + SpawnResult result; + try { + result = spawnRunner.exec(spawn, policy); + } catch (IOException e) { + if (verboseFailures) { + actionExecutionContext + .getEventHandler() + .handle( + Event.warn( + spawn.getMnemonic() + + " remote work failed:\n" + + Throwables.getStackTraceAsString(e))); + } + throw new EnvironmentalExecException("Unexpected IO error.", e); + } + + if ((result.status() != Status.SUCCESS) || (result.exitCode() != 0)) { + String cwd = actionExecutionContext.getExecRoot().getPathString(); + String message = + CommandFailureUtils.describeCommandFailure( + verboseFailures, spawn.getArguments(), spawn.getEnvironment(), cwd); + throw new SpawnExecException( + message, result, /*forciblyRunRemotely=*/false, /*catastrophe=*/false); + } + } +} diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java index 99448b8377..368af4099f 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteActionContextProvider.java @@ -59,7 +59,7 @@ final class RemoteActionContextProvider extends ActionContextProvider { spawnRunner = new RemoteSpawnRunner( env.getExecRoot(), remoteOptions, - createFallbackRunner(), + createFallbackRunner(env), cache, executor); spawnStrategy = @@ -68,7 +68,7 @@ final class RemoteActionContextProvider extends ActionContextProvider { executionOptions.verboseFailures); } - private SpawnRunner createFallbackRunner() { + private static SpawnRunner createFallbackRunner(CommandEnvironment env) { LocalExecutionOptions localExecutionOptions = env.getOptions().getOptions(LocalExecutionOptions.class); LocalEnvProvider localEnvProvider = OS.getCurrent() == OS.DARWIN diff --git a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java index aa870a75cf..1744c1c813 100644 --- a/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/remote/RemoteSpawnStrategy.java @@ -13,38 +13,10 @@ // limitations under the License. package com.google.devtools.build.lib.remote; -import com.google.common.base.Throwables; -import com.google.common.eventbus.EventBus; -import com.google.devtools.build.lib.actions.ActionExecutionContext; -import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.ActionInputFileCache; -import com.google.devtools.build.lib.actions.ActionInputPrefetcher; -import com.google.devtools.build.lib.actions.ActionStatusMessage; -import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; -import com.google.devtools.build.lib.actions.EnvironmentalExecException; -import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; -import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; -import com.google.devtools.build.lib.actions.Spawns; -import com.google.devtools.build.lib.events.Event; -import com.google.devtools.build.lib.exec.SpawnExecException; -import com.google.devtools.build.lib.exec.SpawnInputExpander; -import com.google.devtools.build.lib.exec.SpawnResult; -import com.google.devtools.build.lib.exec.SpawnResult.Status; +import com.google.devtools.build.lib.exec.AbstractSpawnStrategy; import com.google.devtools.build.lib.exec.SpawnRunner; -import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; -import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; -import com.google.devtools.build.lib.standalone.StandaloneSpawnStrategy; -import com.google.devtools.build.lib.util.CommandFailureUtils; -import com.google.devtools.build.lib.util.Preconditions; -import com.google.devtools.build.lib.util.io.FileOutErr; -import com.google.devtools.build.lib.vfs.PathFragment; -import java.io.IOException; -import java.util.SortedMap; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; /** * Strategy that uses a distributed cache for sharing action input and output files. Optionally this @@ -54,128 +26,13 @@ import java.util.concurrent.atomic.AtomicInteger; name = {"remote"}, contextType = SpawnActionContext.class ) -final class RemoteSpawnStrategy implements SpawnActionContext { - private final SpawnInputExpander spawnInputExpander = new SpawnInputExpander(/*strict=*/false); - private final SpawnRunner spawnRunner; - private final boolean verboseFailures; - private final ActionInputPrefetcher inputPrefetcher; - private final AtomicInteger execCount = new AtomicInteger(); - +final class RemoteSpawnStrategy extends AbstractSpawnStrategy { RemoteSpawnStrategy(SpawnRunner spawnRunner, boolean verboseFailures) { - this.spawnRunner = spawnRunner; - this.verboseFailures = verboseFailures; - this.inputPrefetcher = ActionInputPrefetcher.NONE; + super(verboseFailures, spawnRunner); } @Override public String toString() { return "remote"; } - - @Override - public void exec(final Spawn spawn, final ActionExecutionContext actionExecutionContext) - throws ExecException, InterruptedException { - if (!spawn.isRemotable()) { - StandaloneSpawnStrategy standaloneStrategy = - Preconditions.checkNotNull( - actionExecutionContext.getContext(StandaloneSpawnStrategy.class)); - standaloneStrategy.exec(spawn, actionExecutionContext); - return; - } - - if (actionExecutionContext.reportsSubcommands()) { - actionExecutionContext.reportSubcommand(spawn); - } - final int timeoutSeconds = Spawns.getTimeoutSeconds(spawn); - SpawnExecutionPolicy policy = new SpawnExecutionPolicy() { - private final int id = execCount.incrementAndGet(); - - @Override - public int getId() { - return id; - } - - @Override - public void prefetchInputs(Iterable inputs) throws IOException { - inputPrefetcher.prefetchFiles(inputs); - } - - @Override - public ActionInputFileCache getActionInputFileCache() { - return actionExecutionContext.getActionInputFileCache(); - } - - @Override - public ArtifactExpander getArtifactExpander() { - return actionExecutionContext.getArtifactExpander(); - } - - @Override - public void lockOutputFiles() throws InterruptedException { - // This is only needed for the dynamic spawn strategy, which we still need to actually - // implement. - } - - @Override - public long getTimeoutMillis() { - return TimeUnit.SECONDS.toMillis(timeoutSeconds); - } - - @Override - public FileOutErr getFileOutErr() { - return actionExecutionContext.getFileOutErr(); - } - - @Override - public SortedMap getInputMapping() throws IOException { - return spawnInputExpander.getInputMapping( - spawn, - actionExecutionContext.getArtifactExpander(), - actionExecutionContext.getActionInputFileCache(), - actionExecutionContext.getContext(FilesetActionContext.class)); - } - - @Override - public void report(ProgressStatus state, String name) { - EventBus eventBus = actionExecutionContext.getEventBus(); - switch (state) { - case EXECUTING: - eventBus.post( - ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), name)); - break; - case SCHEDULING: - eventBus.post(ActionStatusMessage.schedulingStrategy(spawn.getResourceOwner())); - break; - default: - break; - } - } - }; - - SpawnResult result; - try { - result = spawnRunner.exec(spawn, policy); - } catch (IOException e) { - if (verboseFailures) { - actionExecutionContext - .getEventHandler() - .handle( - Event.warn( - spawn.getMnemonic() - + " remote work failed:\n" - + Throwables.getStackTraceAsString(e))); - } - throw new EnvironmentalExecException("Unexpected IO error.", e); - } - - if ((result.status() != Status.SUCCESS) || (result.exitCode() != 0)) { - // TODO(ulfjack): Return SpawnResult from here and let the upper layers worry about error - // handling and reporting. - String cwd = actionExecutionContext.getExecRoot().getPathString(); - String message = - CommandFailureUtils.describeCommandFailure( - verboseFailures, spawn.getArguments(), spawn.getEnvironment(), cwd); - throw new SpawnExecException(message, result, /*catastrophe=*/ false); - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java index 7b132f124c..3574691243 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/DarwinSandboxedStrategy.java @@ -16,10 +16,8 @@ package com.google.devtools.build.lib.sandbox; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.SpawnActionContext; -import com.google.devtools.build.lib.buildtool.BuildRequest; -import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.vfs.Path; -import java.io.IOException; +import com.google.devtools.build.lib.exec.AbstractSpawnStrategy; +import com.google.devtools.build.lib.exec.SpawnRunner; /** Strategy that uses sandboxing to execute a process, for Darwin */ //TODO(ulfjack): This class only exists for this annotation. Find a better way to handle this! @@ -27,18 +25,13 @@ import java.io.IOException; name = {"sandboxed", "darwin-sandbox"}, contextType = SpawnActionContext.class ) -final class DarwinSandboxedStrategy extends SandboxStrategy { - DarwinSandboxedStrategy( - CommandEnvironment cmdEnv, - BuildRequest buildRequest, - Path sandboxBase, - boolean verboseFailures, - String productName, - int timeoutGraceSeconds) - throws IOException { - super( - verboseFailures, - new DarwinSandboxedSpawnRunner( - cmdEnv, buildRequest, sandboxBase, productName, timeoutGraceSeconds)); +final class DarwinSandboxedStrategy extends AbstractSpawnStrategy { + DarwinSandboxedStrategy(boolean verboseFailures, SpawnRunner spawnRunner) { + super(verboseFailures, spawnRunner); + } + + @Override + public String toString() { + return "sandboxed"; } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java index 5e4cf5dd10..f344708d90 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/LinuxSandboxedStrategy.java @@ -17,6 +17,8 @@ package com.google.devtools.build.lib.sandbox; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.SpawnActionContext; import com.google.devtools.build.lib.buildtool.BuildRequest; +import com.google.devtools.build.lib.exec.AbstractSpawnStrategy; +import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.runtime.CommandEnvironment; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; @@ -28,33 +30,22 @@ import java.io.IOException; name = {"sandboxed", "linux-sandbox"}, contextType = SpawnActionContext.class ) -public final class LinuxSandboxedStrategy extends SandboxStrategy { - private LinuxSandboxedStrategy( - CommandEnvironment cmdEnv, - BuildRequest buildRequest, - Path sandboxBase, - boolean verboseFailures, - Path inaccessibleHelperFile, - Path inaccessibleHelperDir, - int timeoutGraceSeconds) { - super( - verboseFailures, - new LinuxSandboxedSpawnRunner( - cmdEnv, - buildRequest, - sandboxBase, - inaccessibleHelperFile, - inaccessibleHelperDir, - timeoutGraceSeconds)); +public final class LinuxSandboxedStrategy extends AbstractSpawnStrategy { + LinuxSandboxedStrategy(boolean verboseFailures, SpawnRunner spawnRunner) { + super(verboseFailures, spawnRunner); + } + + @Override + public String toString() { + return "sandboxed"; } - static LinuxSandboxedStrategy create( + static LinuxSandboxedSpawnRunner create( CommandEnvironment cmdEnv, BuildRequest buildRequest, Path sandboxBase, - boolean verboseFailures, int timeoutGraceSeconds) - throws IOException { + throws IOException { Path inaccessibleHelperFile = sandboxBase.getRelative("inaccessibleHelperFile"); FileSystemUtils.touchFile(inaccessibleHelperFile); inaccessibleHelperFile.setReadable(false); @@ -67,11 +58,10 @@ public final class LinuxSandboxedStrategy extends SandboxStrategy { inaccessibleHelperDir.setWritable(false); inaccessibleHelperDir.setExecutable(false); - return new LinuxSandboxedStrategy( + return new LinuxSandboxedSpawnRunner( cmdEnv, buildRequest, sandboxBase, - verboseFailures, inaccessibleHelperFile, inaccessibleHelperDir, timeoutGraceSeconds); diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java index af4ae92e48..202f586065 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/ProcessWrapperSandboxedStrategy.java @@ -16,9 +16,8 @@ package com.google.devtools.build.lib.sandbox; import com.google.devtools.build.lib.actions.ExecutionStrategy; import com.google.devtools.build.lib.actions.SpawnActionContext; -import com.google.devtools.build.lib.buildtool.BuildRequest; -import com.google.devtools.build.lib.runtime.CommandEnvironment; -import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.exec.AbstractSpawnStrategy; +import com.google.devtools.build.lib.exec.SpawnRunner; /** Strategy that uses sandboxing to execute a process. */ //TODO(ulfjack): This class only exists for this annotation. Find a better way to handle this! @@ -26,17 +25,13 @@ import com.google.devtools.build.lib.vfs.Path; name = {"sandboxed", "processwrapper-sandbox"}, contextType = SpawnActionContext.class ) -final class ProcessWrapperSandboxedStrategy extends SandboxStrategy { - ProcessWrapperSandboxedStrategy( - CommandEnvironment cmdEnv, - BuildRequest buildRequest, - Path sandboxBase, - boolean verboseFailures, - String productName, - int timeoutGraceSeconds) { - super( - verboseFailures, - new ProcessWrapperSandboxedSpawnRunner( - cmdEnv, buildRequest, sandboxBase, productName, timeoutGraceSeconds)); +final class ProcessWrapperSandboxedStrategy extends AbstractSpawnStrategy { + ProcessWrapperSandboxedStrategy(boolean verboseFailures, SpawnRunner spawnRunner) { + super(verboseFailures, spawnRunner); + } + + @Override + public String toString() { + return "sandboxed"; } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java index 4f795f4059..55f755da4c 100644 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxActionContextProvider.java @@ -16,11 +16,20 @@ package com.google.devtools.build.lib.sandbox; import com.google.common.collect.ImmutableList; import com.google.devtools.build.lib.actions.ActionContext; +import com.google.devtools.build.lib.actions.ExecException; +import com.google.devtools.build.lib.actions.ResourceManager; +import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.buildtool.BuildRequest; import com.google.devtools.build.lib.exec.ActionContextProvider; import com.google.devtools.build.lib.exec.ExecutionOptions; +import com.google.devtools.build.lib.exec.SpawnResult; +import com.google.devtools.build.lib.exec.SpawnRunner; +import com.google.devtools.build.lib.exec.apple.XCodeLocalEnvProvider; +import com.google.devtools.build.lib.exec.local.LocalEnvProvider; import com.google.devtools.build.lib.exec.local.LocalExecutionOptions; +import com.google.devtools.build.lib.exec.local.LocalSpawnRunner; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; @@ -46,40 +55,74 @@ final class SandboxActionContextProvider extends ActionContextProvider { // This works on most platforms, but isn't the best choice, so we put it first and let later // platform-specific sandboxing strategies become the default. if (ProcessWrapperSandboxedSpawnRunner.isSupported(cmdEnv)) { - contexts.add( - new ProcessWrapperSandboxedStrategy( - cmdEnv, - buildRequest, - sandboxBase, - verboseFailures, - productName, - timeoutGraceSeconds)); + SpawnRunner spawnRunner = withFallback( + cmdEnv, + new ProcessWrapperSandboxedSpawnRunner( + cmdEnv, buildRequest, sandboxBase, productName, timeoutGraceSeconds)); + contexts.add(new ProcessWrapperSandboxedStrategy(verboseFailures, spawnRunner)); } // This is the preferred sandboxing strategy on Linux. if (LinuxSandboxedSpawnRunner.isSupported(cmdEnv)) { - contexts.add( - LinuxSandboxedStrategy.create( - cmdEnv, buildRequest, sandboxBase, verboseFailures, timeoutGraceSeconds)); + SpawnRunner spawnRunner = withFallback( + cmdEnv, + LinuxSandboxedStrategy.create(cmdEnv, buildRequest, sandboxBase, timeoutGraceSeconds)); + contexts.add(new LinuxSandboxedStrategy(verboseFailures, spawnRunner)); } // This is the preferred sandboxing strategy on macOS. if (DarwinSandboxedSpawnRunner.isSupported(cmdEnv)) { - contexts.add( - new DarwinSandboxedStrategy( - cmdEnv, - buildRequest, - sandboxBase, - verboseFailures, - productName, - timeoutGraceSeconds)); + SpawnRunner spawnRunner = withFallback( + cmdEnv, + new DarwinSandboxedSpawnRunner( + cmdEnv, buildRequest, sandboxBase, productName, timeoutGraceSeconds)); + contexts.add(new DarwinSandboxedStrategy(verboseFailures, spawnRunner)); } return new SandboxActionContextProvider(contexts.build()); } + private static SpawnRunner withFallback(CommandEnvironment env, SpawnRunner sandboxSpawnRunner) { + return new SandboxFallbackSpawnRunner(sandboxSpawnRunner, createFallbackRunner(env)); + } + + private static SpawnRunner createFallbackRunner(CommandEnvironment env) { + LocalExecutionOptions localExecutionOptions = + env.getOptions().getOptions(LocalExecutionOptions.class); + LocalEnvProvider localEnvProvider = OS.getCurrent() == OS.DARWIN + ? new XCodeLocalEnvProvider() + : LocalEnvProvider.UNMODIFIED; + return + new LocalSpawnRunner( + env.getExecRoot(), + localExecutionOptions, + ResourceManager.instance(), + env.getRuntime().getProductName(), + localEnvProvider); + } + @Override public Iterable getActionContexts() { return contexts; } + + private static final class SandboxFallbackSpawnRunner implements SpawnRunner { + private final SpawnRunner sandboxSpawnRunner; + private final SpawnRunner fallbackSpawnRunner; + + SandboxFallbackSpawnRunner(SpawnRunner sandboxSpawnRunner, SpawnRunner fallbackSpawnRunner) { + this.sandboxSpawnRunner = sandboxSpawnRunner; + this.fallbackSpawnRunner = fallbackSpawnRunner; + } + + @Override + public SpawnResult exec(Spawn spawn, SpawnExecutionPolicy policy) + throws InterruptedException, IOException, ExecException { + if (!spawn.isRemotable() || spawn.hasNoSandbox()) { + return fallbackSpawnRunner.exec(spawn, policy); + } else { + return sandboxSpawnRunner.exec(spawn, policy); + } + } + } } diff --git a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java b/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java deleted file mode 100644 index e64a795f03..0000000000 --- a/src/main/java/com/google/devtools/build/lib/sandbox/SandboxStrategy.java +++ /dev/null @@ -1,166 +0,0 @@ -// Copyright 2016 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.sandbox; - -import com.google.common.eventbus.EventBus; -import com.google.devtools.build.lib.actions.ActionExecutionContext; -import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.ActionInputFileCache; -import com.google.devtools.build.lib.actions.ActionInputPrefetcher; -import com.google.devtools.build.lib.actions.ActionStatusMessage; -import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; -import com.google.devtools.build.lib.actions.ExecException; -import com.google.devtools.build.lib.actions.SandboxedSpawnActionContext; -import com.google.devtools.build.lib.actions.Spawn; -import com.google.devtools.build.lib.actions.SpawnActionContext; -import com.google.devtools.build.lib.actions.Spawns; -import com.google.devtools.build.lib.exec.SpawnExecException; -import com.google.devtools.build.lib.exec.SpawnInputExpander; -import com.google.devtools.build.lib.exec.SpawnResult; -import com.google.devtools.build.lib.exec.SpawnResult.Status; -import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; -import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; -import com.google.devtools.build.lib.util.CommandFailureUtils; -import com.google.devtools.build.lib.util.io.FileOutErr; -import com.google.devtools.build.lib.vfs.PathFragment; -import java.io.IOException; -import java.util.SortedMap; -import java.util.concurrent.TimeUnit; -import java.util.concurrent.atomic.AtomicInteger; -import java.util.concurrent.atomic.AtomicReference; - -/** Abstract common ancestor for sandbox strategies implementing the common parts. */ -abstract class SandboxStrategy implements SandboxedSpawnActionContext { - private final boolean verboseFailures; - private final SpawnInputExpander spawnInputExpander; - private final AbstractSandboxSpawnRunner spawnRunner; - private final ActionInputPrefetcher inputPrefetcher; - private final AtomicInteger execCount = new AtomicInteger(); - - public SandboxStrategy( - boolean verboseFailures, - AbstractSandboxSpawnRunner spawnRunner) { - this.verboseFailures = verboseFailures; - this.spawnInputExpander = new SpawnInputExpander(false); - this.spawnRunner = spawnRunner; - this.inputPrefetcher = ActionInputPrefetcher.NONE; - } - - @Override - public void exec(Spawn spawn, ActionExecutionContext actionExecutionContext) - throws ExecException, InterruptedException { - exec(spawn, actionExecutionContext, null); - } - - @Override - public void exec( - Spawn spawn, - ActionExecutionContext actionExecutionContext, - AtomicReference> writeOutputFiles) - throws ExecException, InterruptedException { - // Certain actions can't run remotely or in a sandbox - pass them on to the standalone strategy. - if (!spawn.isRemotable() || spawn.hasNoSandbox()) { - SandboxHelpers.fallbackToNonSandboxedExecution(spawn, actionExecutionContext); - return; - } - - if (actionExecutionContext.reportsSubcommands()) { - actionExecutionContext.reportSubcommand(spawn); - } - final int timeoutSeconds = Spawns.getTimeoutSeconds(spawn); - SpawnExecutionPolicy policy = new SpawnExecutionPolicy() { - private final int id = execCount.incrementAndGet(); - - @Override - public int getId() { - return id; - } - - @Override - public void prefetchInputs(Iterable inputs) throws IOException { - inputPrefetcher.prefetchFiles(inputs); - } - - @Override - public ActionInputFileCache getActionInputFileCache() { - return actionExecutionContext.getActionInputFileCache(); - } - - @Override - public ArtifactExpander getArtifactExpander() { - return actionExecutionContext.getArtifactExpander(); - } - - @Override - public void lockOutputFiles() throws InterruptedException { - Class token = SandboxStrategy.this.getClass(); - if (writeOutputFiles != null - && writeOutputFiles.get() != token - && !writeOutputFiles.compareAndSet(null, token)) { - throw new InterruptedException(); - } - } - - @Override - public long getTimeoutMillis() { - return TimeUnit.SECONDS.toMillis(timeoutSeconds); - } - - @Override - public FileOutErr getFileOutErr() { - return actionExecutionContext.getFileOutErr(); - } - - @Override - public SortedMap getInputMapping() throws IOException { - return spawnInputExpander.getInputMapping( - spawn, - actionExecutionContext.getArtifactExpander(), - actionExecutionContext.getActionInputFileCache(), - actionExecutionContext.getContext(FilesetActionContext.class)); - } - - @Override - public void report(ProgressStatus state, String name) { - // TODO(ulfjack): We should report more details to the UI. - EventBus eventBus = actionExecutionContext.getEventBus(); - switch (state) { - case EXECUTING: - eventBus.post(ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), name)); - break; - case SCHEDULING: - eventBus.post(ActionStatusMessage.schedulingStrategy(spawn.getResourceOwner())); - break; - default: - break; - } - } - }; - SpawnResult result = spawnRunner.exec(spawn, policy); - if (result.status() != Status.SUCCESS || result.exitCode() != 0) { - String message = - CommandFailureUtils.describeCommandFailure( - verboseFailures, spawn.getArguments(), spawn.getEnvironment(), null); - throw new SpawnExecException( - message, result, /*forciblyRunRemotely=*/false, /*catastrophe=*/false); - } - } - - @Override - public String toString() { - return "sandboxed"; - } -} diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java index ba42ed9965..62b1a7ee3e 100644 --- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java +++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneActionContextProvider.java @@ -24,14 +24,19 @@ import com.google.devtools.build.lib.actions.ResourceManager; import com.google.devtools.build.lib.exec.ActionContextProvider; import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.FileWriteStrategy; +import com.google.devtools.build.lib.exec.SpawnRunner; import com.google.devtools.build.lib.exec.StandaloneTestStrategy; import com.google.devtools.build.lib.exec.TestStrategy; +import com.google.devtools.build.lib.exec.apple.XCodeLocalEnvProvider; +import com.google.devtools.build.lib.exec.local.LocalEnvProvider; import com.google.devtools.build.lib.exec.local.LocalExecutionOptions; +import com.google.devtools.build.lib.exec.local.LocalSpawnRunner; import com.google.devtools.build.lib.rules.cpp.IncludeScanningContext; import com.google.devtools.build.lib.rules.cpp.SpawnGccStrategy; import com.google.devtools.build.lib.rules.test.ExclusiveTestStrategy; import com.google.devtools.build.lib.rules.test.TestActionContext; import com.google.devtools.build.lib.runtime.CommandEnvironment; +import com.google.devtools.build.lib.util.OS; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import java.io.IOException; @@ -72,8 +77,6 @@ public class StandaloneActionContextProvider extends ActionContextProvider { @Override public Iterable getActionContexts() { ExecutionOptions executionOptions = env.getOptions().getOptions(ExecutionOptions.class); - LocalExecutionOptions localExecutionOptions = - env.getOptions().getOptions(LocalExecutionOptions.class); Path testTmpRoot = TestStrategy.getTmpRoot(env.getWorkspace(), env.getExecRoot(), executionOptions); @@ -86,16 +89,26 @@ public class StandaloneActionContextProvider extends ActionContextProvider { // could potentially be used and a spawnActionContext doesn't specify which one it wants, the // last one from strategies list will be used return ImmutableList.of( - new StandaloneSpawnStrategy( - env.getExecRoot(), - localExecutionOptions, - executionOptions.verboseFailures, - env.getRuntime().getProductName(), - ResourceManager.instance()), + new StandaloneSpawnStrategy(executionOptions.verboseFailures, createLocalRunner(env)), new DummyIncludeScanningContext(), new SpawnGccStrategy(), testStrategy, new ExclusiveTestStrategy(testStrategy), new FileWriteStrategy()); } + + private static SpawnRunner createLocalRunner(CommandEnvironment env) { + LocalExecutionOptions localExecutionOptions = + env.getOptions().getOptions(LocalExecutionOptions.class); + LocalEnvProvider localEnvProvider = OS.getCurrent() == OS.DARWIN + ? new XCodeLocalEnvProvider() + : LocalEnvProvider.UNMODIFIED; + return + new LocalSpawnRunner( + env.getExecRoot(), + localExecutionOptions, + ResourceManager.instance(), + env.getRuntime().getProductName(), + localEnvProvider); + } } diff --git a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java index f62e2b7bdf..495d46d362 100644 --- a/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java +++ b/src/main/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategy.java @@ -13,152 +13,19 @@ // limitations under the License. package com.google.devtools.build.lib.standalone; -import com.google.common.eventbus.EventBus; -import com.google.devtools.build.lib.actions.ActionExecutionContext; -import com.google.devtools.build.lib.actions.ActionInput; -import com.google.devtools.build.lib.actions.ActionInputFileCache; -import com.google.devtools.build.lib.actions.ActionStatusMessage; -import com.google.devtools.build.lib.actions.Artifact.ArtifactExpander; -import com.google.devtools.build.lib.actions.ExecException; import com.google.devtools.build.lib.actions.ExecutionStrategy; -import com.google.devtools.build.lib.actions.ResourceManager; -import com.google.devtools.build.lib.actions.Spawn; import com.google.devtools.build.lib.actions.SpawnActionContext; -import com.google.devtools.build.lib.actions.Spawns; -import com.google.devtools.build.lib.actions.UserExecException; -import com.google.devtools.build.lib.exec.SpawnExecException; -import com.google.devtools.build.lib.exec.SpawnInputExpander; -import com.google.devtools.build.lib.exec.SpawnResult; -import com.google.devtools.build.lib.exec.SpawnResult.Status; -import com.google.devtools.build.lib.exec.SpawnRunner.ProgressStatus; -import com.google.devtools.build.lib.exec.SpawnRunner.SpawnExecutionPolicy; -import com.google.devtools.build.lib.exec.apple.XCodeLocalEnvProvider; -import com.google.devtools.build.lib.exec.local.LocalEnvProvider; -import com.google.devtools.build.lib.exec.local.LocalExecutionOptions; -import com.google.devtools.build.lib.exec.local.LocalSpawnRunner; -import com.google.devtools.build.lib.rules.fileset.FilesetActionContext; -import com.google.devtools.build.lib.util.CommandFailureUtils; -import com.google.devtools.build.lib.util.OS; -import com.google.devtools.build.lib.util.io.FileOutErr; -import com.google.devtools.build.lib.vfs.Path; -import com.google.devtools.build.lib.vfs.PathFragment; -import java.io.IOException; -import java.util.SortedMap; -import java.util.concurrent.atomic.AtomicInteger; +import com.google.devtools.build.lib.exec.AbstractSpawnStrategy; +import com.google.devtools.build.lib.exec.SpawnRunner; /** * Strategy that uses subprocessing to execute a process. */ @ExecutionStrategy(name = { "standalone", "local" }, contextType = SpawnActionContext.class) -public class StandaloneSpawnStrategy implements SpawnActionContext { - private final boolean verboseFailures; - private final LocalSpawnRunner localSpawnRunner; - private final AtomicInteger execCount = new AtomicInteger(); - +public class StandaloneSpawnStrategy extends AbstractSpawnStrategy { public StandaloneSpawnStrategy( - Path execRoot, LocalExecutionOptions localExecutionOptions, boolean verboseFailures, - String productName, ResourceManager resourceManager) { - this.verboseFailures = verboseFailures; - LocalEnvProvider localEnvProvider = OS.getCurrent() == OS.DARWIN - ? new XCodeLocalEnvProvider() - : LocalEnvProvider.UNMODIFIED; - this.localSpawnRunner = new LocalSpawnRunner( - execRoot, - localExecutionOptions, - resourceManager, - productName, - localEnvProvider); - } - - /** - * Executes the given {@code spawn}. - */ - @Override - public void exec(final Spawn spawn, final ActionExecutionContext actionExecutionContext) - throws ExecException, InterruptedException { - final int timeoutSeconds = Spawns.getTimeoutSeconds(spawn); - final EventBus eventBus = actionExecutionContext.getEventBus(); - SpawnExecutionPolicy policy = new SpawnExecutionPolicy() { - private final int id = execCount.incrementAndGet(); - - @Override - public int getId() { - return id; - } - - @Override - public void prefetchInputs(Iterable inputs) throws IOException { - if (Spawns.shouldPrefetchInputsForLocalExecution(spawn)) { - actionExecutionContext.getActionInputPrefetcher().prefetchFiles(inputs); - } - } - - @Override - public ActionInputFileCache getActionInputFileCache() { - return actionExecutionContext.getActionInputFileCache(); - } - - @Override - public ArtifactExpander getArtifactExpander() { - return actionExecutionContext.getArtifactExpander(); - } - - @Override - public void lockOutputFiles() throws InterruptedException { - // Do nothing for now. - } - - @Override - public long getTimeoutMillis() { - return timeoutSeconds * 1000L; - } - - @Override - public FileOutErr getFileOutErr() { - return actionExecutionContext.getFileOutErr(); - } - - @Override - public SortedMap getInputMapping() throws IOException { - return new SpawnInputExpander(/*strict*/false) - .getInputMapping( - spawn, - actionExecutionContext.getArtifactExpander(), - actionExecutionContext.getActionInputFileCache(), - actionExecutionContext.getContext(FilesetActionContext.class)); - } - - @Override - public void report(ProgressStatus state, String name) { - switch (state) { - case EXECUTING: - eventBus.post(ActionStatusMessage.runningStrategy(spawn.getResourceOwner(), name)); - break; - case SCHEDULING: - eventBus.post(ActionStatusMessage.schedulingStrategy(spawn.getResourceOwner())); - break; - default: - break; - } - } - }; - - if (actionExecutionContext.reportsSubcommands()) { - actionExecutionContext.reportSubcommand(spawn); - } - - try { - SpawnResult result = localSpawnRunner.exec(spawn, policy); - if (result.status() != Status.SUCCESS || result.exitCode() != 0) { - String message = - CommandFailureUtils.describeCommandFailure( - verboseFailures, spawn.getArguments(), spawn.getEnvironment(), null); - throw new SpawnExecException( - message, result, /*forciblyRunRemotely=*/false, /*catastrophe=*/false); - } - } catch (IOException e) { - throw new UserExecException("I/O exception during local execution", e); - } + boolean verboseFailures, SpawnRunner spawnRunner) { + super(verboseFailures, spawnRunner); } @Override diff --git a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java index 0aa7695b5d..7983c669c1 100644 --- a/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java +++ b/src/test/java/com/google/devtools/build/lib/standalone/StandaloneSpawnStrategyTest.java @@ -40,7 +40,9 @@ import com.google.devtools.build.lib.exec.ActionContextProvider; import com.google.devtools.build.lib.exec.BlazeExecutor; import com.google.devtools.build.lib.exec.ExecutionOptions; import com.google.devtools.build.lib.exec.SingleBuildFileCache; +import com.google.devtools.build.lib.exec.local.LocalEnvProvider; import com.google.devtools.build.lib.exec.local.LocalExecutionOptions; +import com.google.devtools.build.lib.exec.local.LocalSpawnRunner; import com.google.devtools.build.lib.integration.util.IntegrationMock; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestUtils; @@ -128,8 +130,13 @@ public class StandaloneSpawnStrategyTest { ImmutableMap.of( "", new StandaloneSpawnStrategy( - execRoot, localExecutionOptions, /*verboseFailures=*/false, "mock-product-name", - resourceManager)), + /*verboseFailures=*/false, + new LocalSpawnRunner( + execRoot, + localExecutionOptions, + resourceManager, + "mock-product-name", + LocalEnvProvider.UNMODIFIED))), ImmutableList.of()); executor.getExecRoot().createDirectory(); -- cgit v1.2.3