Start the file existence check traversal from the execroot base instead of execroot so that external repo files at "<execroot>/../<path>" are correctly handled when the sibling repository layout is enabled.

PiperOrigin-RevId: 357965029

Author: Googler

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",

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<PathFragment> 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<PathFragment> inputsToCreate,
       Set<PathFragment> 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/<workspace name>. 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<PathFragment> destination =
           getExpectedSymlinkDestination(pathRelativeToWorkDir, inputs);
       if (destination.isPresent()) {

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 <execroot>/external/<repo name>/<path>. Instead, they become *siblings* of the main
+    // workspace files in that they're placed at <execroot>/../<repo name>/<path>. 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<PathFragment> 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();
+  }
 }

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