diff --git a/src/main/java/com/google/devtools/build/lib/worker/BUILD b/src/main/java/com/google/devtools/build/lib/worker/BUILD index 301c21fb9a29ae..1080013342f595 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/BUILD +++ b/src/main/java/com/google/devtools/build/lib/worker/BUILD @@ -17,6 +17,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:execution_requirements", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", + "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/events", "//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy", "//src/main/java/com/google/devtools/build/lib/exec:bin_tools", diff --git a/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java b/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java index 15533d7fa07295..32f696f1928088 100644 --- a/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java +++ b/src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java @@ -14,6 +14,7 @@ package com.google.devtools.build.lib.worker; import com.google.common.collect.Iterables; +import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.sandbox.SandboxHelpers; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs; import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs; @@ -46,10 +47,12 @@ public void createFileSystem( LinkedHashSet dirsToCreate = new LinkedHashSet<>(); populateInputsAndDirsToCreate(inputs, workerFiles, outputs, inputsToCreate, dirsToCreate); - // Then do a full traversal of the `workDir`. This will use what we computed above, delete - // anything unnecessary and update `inputsToCreate`/`dirsToCreate` if something is can be left - // without changes (e.g., a symlink that already points to the right destination). - cleanExisting(workDir, inputs, inputsToCreate, dirsToCreate); + // Then do a full traversal of the parent directory of `workDir`. This will use what we computed + // above, delete anything unnecessary and update `inputsToCreate`/`dirsToCreate` if something is + // can be left without changes (e.g., a symlink that already points to the right destination). + // We're traversing from workDir's parent directory because external repositories can now be + // symlinked as siblings of workDir when --experimental_sibling_repository_layout is in effect. + cleanExisting(workDir.getParentDirectory(), inputs, inputsToCreate, dirsToCreate); // Finally, create anything that is still missing. createDirectories(dirsToCreate); @@ -105,9 +108,20 @@ private void cleanExisting( Set inputsToCreate, Set dirsToCreate) throws IOException { + Path execroot = workDir.getParentDirectory(); for (Path path : root.getDirectoryEntries()) { FileStatus stat = path.stat(Symlinks.NOFOLLOW); - PathFragment pathRelativeToWorkDir = path.relativeTo(workDir); + PathFragment pathRelativeToWorkDir; + if (path.startsWith(workDir)) { + // path is under workDir, i.e. execroot/. Simply get the relative path. + pathRelativeToWorkDir = path.relativeTo(workDir); + } else { + // path is not under workDir, which means it belongs to one of external repositories + // symlinked directly under execroot. Get the relative path based on there and prepend it + // with the designated prefix, '../', so that it's still a valid relative path to workDir. + pathRelativeToWorkDir = + LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX.getRelative(path.relativeTo(execroot)); + } Optional destination = getExpectedSymlinkDestination(pathRelativeToWorkDir, inputs); if (destination.isPresent()) { diff --git a/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java b/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java index c8cbbcf3a186e1..1ee67be78f0760 100644 --- a/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java +++ b/src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java @@ -53,8 +53,8 @@ public final void setupTestDirs() throws IOException { workspaceDir.createDirectory(); sandboxDir = testRoot.getRelative("sandbox"); sandboxDir.createDirectory(); - execRoot = sandboxDir.getRelative("execroot"); - execRoot.createDirectory(); + execRoot = sandboxDir.getRelative("execroot/__main__"); + execRoot.createDirectoryAndParents(); } @Test @@ -212,4 +212,45 @@ public void recreatesEmptyFiles() throws Exception { execRoot.getRelative("some_file"), Charset.defaultCharset())) .isEmpty(); } + + @Test + public void createsAndDeletesSiblingExternalRepoFiles() throws Exception { + Path fooRepoDir = testRoot.getRelative("external_dir/foo"); + fooRepoDir.createDirectoryAndParents(); + + fooRepoDir.getRelative("bar").createDirectory(); + Path input1 = fooRepoDir.getRelative("bar/input1"); + FileSystemUtils.writeContentAsLatin1(input1, "This is input1."); + Path input2 = fooRepoDir.getRelative("input2"); + FileSystemUtils.writeContentAsLatin1(input1, "This is input2."); + Path random = fooRepoDir.getRelative("bar/random"); + FileSystemUtils.writeContentAsLatin1(input1, "This is random."); + + // With the sibling repository layout, external repository source files are no longer symlinked + // under /external//. Instead, they become *siblings* of the main + // workspace files in that they're placed at /..//. Simulate this + // layout and check if inputs are correctly created and irrelevant symlinks are deleted. + FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("../foo/bar/input1"), input1); + FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("../foo/bar/random"), random); + + SandboxInputs inputs = + new SandboxInputs( + ImmutableMap.of( + PathFragment.create("../foo/bar/input1"), + input1, + PathFragment.create("../foo/input2"), + input2), + ImmutableSet.of(), + ImmutableMap.of()); + SandboxOutputs outputs = SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of()); + ImmutableSet workerFiles = ImmutableSet.of(); + WorkerExecRoot workerExecRoot = new WorkerExecRoot(execRoot); + workerExecRoot.createFileSystem(workerFiles, inputs, outputs); + + assertThat(execRoot.getRelative("../foo/bar/input1").readSymbolicLink()) + .isEqualTo(input1.asFragment()); + assertThat(execRoot.getRelative("../foo/input2").readSymbolicLink()) + .isEqualTo(input2.asFragment()); + assertThat(execRoot.getRelative("bar/random").exists()).isFalse(); + } } diff --git a/src/test/shell/integration/bazel_worker_test.sh b/src/test/shell/integration/bazel_worker_test.sh index 438fb992259909..bf4c0454bb7e5a 100755 --- a/src/test/shell/integration/bazel_worker_test.sh +++ b/src/test/shell/integration/bazel_worker_test.sh @@ -95,6 +95,18 @@ function test_compiles_hello_library_using_persistent_javac() { || fail "comparison failed" } +function test_compiles_hello_library_using_persistent_javac_sibling_layout() { + write_hello_library_files + + bazel build \ + --experimental_sibling_repository_layout java/main:main \ + --worker_max_instances=Javac=1 \ + &> $TEST_log || fail "build failed" + expect_log "Created new ${WORKER_TYPE_LOG_STRING} Javac worker (id [0-9]\+)" + $BINS/java/main/main | grep -q "Hello, Library!;Hello, World!" \ + || fail "comparison failed" +} + function prepare_example_worker() { cp ${example_worker} worker_lib.jar chmod +w worker_lib.jar