Skip to content

Commit

Permalink
Start the file existence check traversal from the execroot base inste…
Browse files Browse the repository at this point in the history
…ad of execroot so that external repo files at "<execroot>/../<path>" are correctly handled when the sibling repository layout is enabled.

PiperOrigin-RevId: 357965029
  • Loading branch information
Googler authored and copybara-github committed Feb 17, 2021
1 parent db3757b commit a2cc046
Show file tree
Hide file tree
Showing 4 changed files with 75 additions and 7 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/worker/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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);
Expand Down Expand Up @@ -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()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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();
}
}
12 changes: 12 additions & 0 deletions src/test/shell/integration/bazel_worker_test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down

0 comments on commit a2cc046

Please sign in to comment.