Skip to content

Commit a2cc046

Browse files
Googlercopybara-github
Googler
authored and
copybara-github
committed
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
1 parent db3757b commit a2cc046

File tree

4 files changed

+75
-7
lines changed

4 files changed

+75
-7
lines changed

‎src/main/java/com/google/devtools/build/lib/worker/BUILD‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@ java_library(
1717
"//src/main/java/com/google/devtools/build/lib/actions:artifacts",
1818
"//src/main/java/com/google/devtools/build/lib/actions:execution_requirements",
1919
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
20+
"//src/main/java/com/google/devtools/build/lib/cmdline",
2021
"//src/main/java/com/google/devtools/build/lib/events",
2122
"//src/main/java/com/google/devtools/build/lib/exec:abstract_spawn_strategy",
2223
"//src/main/java/com/google/devtools/build/lib/exec:bin_tools",

‎src/main/java/com/google/devtools/build/lib/worker/WorkerExecRoot.java‎

Lines changed: 19 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,7 @@
1414
package com.google.devtools.build.lib.worker;
1515

1616
import com.google.common.collect.Iterables;
17+
import com.google.devtools.build.lib.cmdline.LabelConstants;
1718
import com.google.devtools.build.lib.sandbox.SandboxHelpers;
1819
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxInputs;
1920
import com.google.devtools.build.lib.sandbox.SandboxHelpers.SandboxOutputs;
@@ -46,10 +47,12 @@ public void createFileSystem(
4647
LinkedHashSet<PathFragment> dirsToCreate = new LinkedHashSet<>();
4748
populateInputsAndDirsToCreate(inputs, workerFiles, outputs, inputsToCreate, dirsToCreate);
4849

49-
// Then do a full traversal of the `workDir`. This will use what we computed above, delete
50-
// anything unnecessary and update `inputsToCreate`/`dirsToCreate` if something is can be left
51-
// without changes (e.g., a symlink that already points to the right destination).
52-
cleanExisting(workDir, inputs, inputsToCreate, dirsToCreate);
50+
// Then do a full traversal of the parent directory of `workDir`. This will use what we computed
51+
// above, delete anything unnecessary and update `inputsToCreate`/`dirsToCreate` if something is
52+
// can be left without changes (e.g., a symlink that already points to the right destination).
53+
// We're traversing from workDir's parent directory because external repositories can now be
54+
// symlinked as siblings of workDir when --experimental_sibling_repository_layout is in effect.
55+
cleanExisting(workDir.getParentDirectory(), inputs, inputsToCreate, dirsToCreate);
5356

5457
// Finally, create anything that is still missing.
5558
createDirectories(dirsToCreate);
@@ -105,9 +108,20 @@ private void cleanExisting(
105108
Set<PathFragment> inputsToCreate,
106109
Set<PathFragment> dirsToCreate)
107110
throws IOException {
111+
Path execroot = workDir.getParentDirectory();
108112
for (Path path : root.getDirectoryEntries()) {
109113
FileStatus stat = path.stat(Symlinks.NOFOLLOW);
110-
PathFragment pathRelativeToWorkDir = path.relativeTo(workDir);
114+
PathFragment pathRelativeToWorkDir;
115+
if (path.startsWith(workDir)) {
116+
// path is under workDir, i.e. execroot/<workspace name>. Simply get the relative path.
117+
pathRelativeToWorkDir = path.relativeTo(workDir);
118+
} else {
119+
// path is not under workDir, which means it belongs to one of external repositories
120+
// symlinked directly under execroot. Get the relative path based on there and prepend it
121+
// with the designated prefix, '../', so that it's still a valid relative path to workDir.
122+
pathRelativeToWorkDir =
123+
LabelConstants.EXPERIMENTAL_EXTERNAL_PATH_PREFIX.getRelative(path.relativeTo(execroot));
124+
}
111125
Optional<PathFragment> destination =
112126
getExpectedSymlinkDestination(pathRelativeToWorkDir, inputs);
113127
if (destination.isPresent()) {

‎src/test/java/com/google/devtools/build/lib/worker/WorkerExecRootTest.java‎

Lines changed: 43 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -53,8 +53,8 @@ public final void setupTestDirs() throws IOException {
5353
workspaceDir.createDirectory();
5454
sandboxDir = testRoot.getRelative("sandbox");
5555
sandboxDir.createDirectory();
56-
execRoot = sandboxDir.getRelative("execroot");
57-
execRoot.createDirectory();
56+
execRoot = sandboxDir.getRelative("execroot/__main__");
57+
execRoot.createDirectoryAndParents();
5858
}
5959

6060
@Test
@@ -212,4 +212,45 @@ public void recreatesEmptyFiles() throws Exception {
212212
execRoot.getRelative("some_file"), Charset.defaultCharset()))
213213
.isEmpty();
214214
}
215+
216+
@Test
217+
public void createsAndDeletesSiblingExternalRepoFiles() throws Exception {
218+
Path fooRepoDir = testRoot.getRelative("external_dir/foo");
219+
fooRepoDir.createDirectoryAndParents();
220+
221+
fooRepoDir.getRelative("bar").createDirectory();
222+
Path input1 = fooRepoDir.getRelative("bar/input1");
223+
FileSystemUtils.writeContentAsLatin1(input1, "This is input1.");
224+
Path input2 = fooRepoDir.getRelative("input2");
225+
FileSystemUtils.writeContentAsLatin1(input1, "This is input2.");
226+
Path random = fooRepoDir.getRelative("bar/random");
227+
FileSystemUtils.writeContentAsLatin1(input1, "This is random.");
228+
229+
// With the sibling repository layout, external repository source files are no longer symlinked
230+
// under <execroot>/external/<repo name>/<path>. Instead, they become *siblings* of the main
231+
// workspace files in that they're placed at <execroot>/../<repo name>/<path>. Simulate this
232+
// layout and check if inputs are correctly created and irrelevant symlinks are deleted.
233+
FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("../foo/bar/input1"), input1);
234+
FileSystemUtils.ensureSymbolicLink(execRoot.getRelative("../foo/bar/random"), random);
235+
236+
SandboxInputs inputs =
237+
new SandboxInputs(
238+
ImmutableMap.of(
239+
PathFragment.create("../foo/bar/input1"),
240+
input1,
241+
PathFragment.create("../foo/input2"),
242+
input2),
243+
ImmutableSet.of(),
244+
ImmutableMap.of());
245+
SandboxOutputs outputs = SandboxOutputs.create(ImmutableSet.of(), ImmutableSet.of());
246+
ImmutableSet<PathFragment> workerFiles = ImmutableSet.of();
247+
WorkerExecRoot workerExecRoot = new WorkerExecRoot(execRoot);
248+
workerExecRoot.createFileSystem(workerFiles, inputs, outputs);
249+
250+
assertThat(execRoot.getRelative("../foo/bar/input1").readSymbolicLink())
251+
.isEqualTo(input1.asFragment());
252+
assertThat(execRoot.getRelative("../foo/input2").readSymbolicLink())
253+
.isEqualTo(input2.asFragment());
254+
assertThat(execRoot.getRelative("bar/random").exists()).isFalse();
255+
}
215256
}

‎src/test/shell/integration/bazel_worker_test.sh‎

Lines changed: 12 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -95,6 +95,18 @@ function test_compiles_hello_library_using_persistent_javac() {
9595
|| fail "comparison failed"
9696
}
9797

98+
function test_compiles_hello_library_using_persistent_javac_sibling_layout() {
99+
write_hello_library_files
100+
101+
bazel build \
102+
--experimental_sibling_repository_layout java/main:main \
103+
--worker_max_instances=Javac=1 \
104+
&> $TEST_log || fail "build failed"
105+
expect_log "Created new ${WORKER_TYPE_LOG_STRING} Javac worker (id [0-9]\+)"
106+
$BINS/java/main/main | grep -q "Hello, Library!;Hello, World!" \
107+
|| fail "comparison failed"
108+
}
109+
98110
function prepare_example_worker() {
99111
cp ${example_worker} worker_lib.jar
100112
chmod +w worker_lib.jar

0 commit comments

Comments
 (0)