diff options
5 files changed, 86 insertions, 18 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java b/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java index a06ddc0114..fedd037d1c 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java +++ b/src/main/java/com/google/devtools/build/lib/worker/SandboxedWorker.java @@ -45,13 +45,14 @@ final class SandboxedWorker extends Worker { // used to pass information to this method and the method below. Instead, don't pass the // WorkerKey to this method but only the input and output files. new SymlinkedSandboxedSpawn( - workDir, - workDir, - ImmutableList.of("/does_not_exist"), - ImmutableMap.<String, String>of(), - key.getInputFiles(), - key.getOutputFiles(), - ImmutableSet.<Path>of()).createFileSystem(); + workDir, + workDir, + ImmutableList.of("/does_not_exist"), + ImmutableMap.of(), + key.getInputFiles(), + key.getOutputFiles(), + ImmutableSet.of()) + .createFileSystem(); } @Override @@ -59,12 +60,13 @@ final class SandboxedWorker extends Worker { // Note: the key passed in here may be different from the key passed to the constructor for // subsequent invocations of the same worker. new SymlinkedSandboxedSpawn( - workDir, - workDir, - ImmutableList.of("/does_not_exist"), - ImmutableMap.<String, String>of(), - key.getInputFiles(), - key.getOutputFiles(), - ImmutableSet.<Path>of()).copyOutputs(key.getExecRoot()); + workDir, + workDir, + ImmutableList.of("/does_not_exist"), + ImmutableMap.of(), + key.getInputFiles(), + key.getOutputFiles(), + ImmutableSet.of()) + .copyOutputs(key.getExecRoot()); } } diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java index 1e0e611cab..6f75fcf29d 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerFactory.java @@ -60,7 +60,7 @@ final class WorkerFactory extends BaseKeyedPooledObjectFactory<WorkerKey, Worker Worker worker; boolean sandboxed = workerOptions.workerSandboxing || key.mustBeSandboxed(); if (sandboxed) { - Path workDir = workerBaseDir.getRelative("worker-" + workerId + "-" + key.getMnemonic()); + Path workDir = getSandboxedWorkerPath(key, workerId); worker = new SandboxedWorker(key, workerId, workDir, logFile); } else { worker = new Worker(key, workerId, key.getExecRoot(), logFile); @@ -80,6 +80,13 @@ final class WorkerFactory extends BaseKeyedPooledObjectFactory<WorkerKey, Worker return worker; } + Path getSandboxedWorkerPath(WorkerKey key, int workerId) { + String workspaceName = key.getExecRoot().getBaseName(); + return workerBaseDir + .getRelative("worker-" + workerId + "-" + key.getMnemonic()) + .getRelative(workspaceName); + } + /** * Use the DefaultPooledObject implementation. */ diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java index 0ac6cc031a..578cc12219 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerModule.java @@ -44,8 +44,8 @@ public class WorkerModule extends BlazeModule { @Override public Iterable<Class<? extends OptionsBase>> getCommandOptions(Command command) { return "build".equals(command.name()) - ? ImmutableList.<Class<? extends OptionsBase>>of(WorkerOptions.class) - : ImmutableList.<Class<? extends OptionsBase>>of(); + ? ImmutableList.of(WorkerOptions.class) + : ImmutableList.of(); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java index 9bcc6e3d0a..c69193c00f 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerSpawnRunner.java @@ -95,7 +95,7 @@ final class WorkerSpawnRunner implements SpawnRunner { if (!spawn.getExecutionInfo().containsKey(ExecutionRequirements.SUPPORTS_WORKERS) || !spawn.getExecutionInfo().get(ExecutionRequirements.SUPPORTS_WORKERS).equals("1")) { // TODO(ulfjack): Don't circumvent SpawnExecutionPolicy. Either drop the warning here, or - // provide a mechanism in SpawnExectionPolicy to report warnings. + // provide a mechanism in SpawnExecutionPolicy to report warnings. reporter.handle( Event.warn( String.format(ERROR_MESSAGE_PREFIX + REASON_NO_EXECUTION_INFO, spawn.getMnemonic()))); diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java new file mode 100644 index 0000000000..27c0d4af1f --- /dev/null +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerFactoryTest.java @@ -0,0 +1,59 @@ +// 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.worker; + +import static com.google.common.truth.Truth.assertThat; + +import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; +import com.google.common.collect.ImmutableSet; +import com.google.common.collect.ImmutableSortedMap; +import com.google.common.hash.HashCode; +import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.Path; +import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.junit.runners.JUnit4; + +/** Tests for {@link WorkerFactory}. */ +@RunWith(JUnit4.class) +public class WorkerFactoryTest { + + final FileSystem fs = new InMemoryFileSystem(); + + /** + * Regression test for b/64689608: The execroot of the sandboxed worker process must end with the + * workspace name, just like the normal execroot does. + */ + @Test + public void sandboxedWorkerPathEndsWithWorkspaceName() throws Exception { + Path workerBaseDir = fs.getPath("/outputbase/bazel-workers"); + WorkerFactory workerFactory = new WorkerFactory(new WorkerOptions(), workerBaseDir); + WorkerKey workerKey = + new WorkerKey( + ImmutableList.of(), + ImmutableMap.of(), + fs.getPath("/outputbase/execroot/workspace"), + "dummy", + HashCode.fromInt(0), + ImmutableSortedMap.of(), + ImmutableMap.of(), + ImmutableSet.of(), + true); + Path sandboxedWorkerPath = workerFactory.getSandboxedWorkerPath(workerKey, 1); + + assertThat(sandboxedWorkerPath.getBaseName()).isEqualTo("workspace"); + } +} |