Skip to content

Commit

Permalink
Fix .runfile logic
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Aug 30, 2024
1 parent 9703421 commit 94f75fc
Show file tree
Hide file tree
Showing 2 changed files with 16 additions and 7 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -160,13 +160,16 @@ private SpawnExec reconstructSpawnExec(ExecLogEntry.Spawn entry) throws IOExcept
private record FlattenedInputSet<T extends Map<String, File>>(
T inputs, boolean hasWorkspaceRunfilesDirectory) {}

// Matches paths of source files and generated files under an external repository.
private static final Pattern EXTERNAL_PREFIX_PATTERN =
Pattern.compile("(external/|(blaze|bazel)-out/[^/]+/[^/]+/external/).*");

private <T extends Map<String, File>> FlattenedInputSet<T> reconstructInputs(
int setId, Supplier<T> newMap) throws IOException {
T inputs = newMap.get();
ArrayDeque<Integer> setsToVisit = new ArrayDeque<>();
HashSet<Integer> visited = new HashSet<>();
boolean hasWorkspaceRunfilesDirectory = false;
String externalPrefix = LabelConstants.EXTERNAL_PATH_PREFIX.getPathString() + "/";
if (setId != 0) {
setsToVisit.addLast(setId);
visited.add(setId);
Expand All @@ -177,7 +180,8 @@ private <T extends Map<String, File>> FlattenedInputSet<T> reconstructInputs(
if (visited.add(fileId)) {
File file = getFromMap(fileMap, fileId);
inputs.put(file.getPath(), file);
if (!hasWorkspaceRunfilesDirectory && !file.getPath().startsWith(externalPrefix)) {
if (!hasWorkspaceRunfilesDirectory
&& !EXTERNAL_PREFIX_PATTERN.matcher(file.getPath()).matches()) {
hasWorkspaceRunfilesDirectory = true;
}
}
Expand All @@ -191,7 +195,8 @@ private <T extends Map<String, File>> FlattenedInputSet<T> reconstructInputs(
// This is bug-for-bug compatible with the implementation in Runfiles by considering
// an empty non-external directory as a runfiles entry under the workspace runfiles
// directory even though it won't be materialized as one.
if (!hasWorkspaceRunfilesDirectory && !dir.getFirst().startsWith(externalPrefix)) {
if (!hasWorkspaceRunfilesDirectory
&& !EXTERNAL_PREFIX_PATTERN.matcher(dir.getFirst()).matches()) {
hasWorkspaceRunfilesDirectory = true;
}
}
Expand All @@ -200,7 +205,8 @@ private <T extends Map<String, File>> FlattenedInputSet<T> reconstructInputs(
if (visited.add(symlinkId)) {
File symlink = getFromMap(symlinkMap, symlinkId);
inputs.put(symlink.getPath(), symlink);
if (!hasWorkspaceRunfilesDirectory && !symlink.getPath().startsWith(externalPrefix)) {
if (!hasWorkspaceRunfilesDirectory
&& !EXTERNAL_PREFIX_PATTERN.matcher(symlink.getPath()).matches()) {
hasWorkspaceRunfilesDirectory = true;
}
}
Expand Down Expand Up @@ -278,6 +284,7 @@ private Pair<String, Collection<File>> reconstructRunfilesDir(
return Pair.of(runfilesTree.getPath(), ImmutableList.copyOf(builder.values()));
}

// Matches the prefix of a generated file path.
private static final Pattern BAZEL_OUT_PREFIX_PATTERN =
Pattern.compile("^(blaze|bazel)-out/[^/]+/[^/]+/");
private static final String EXTERNAL_PREFIX =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -647,7 +647,6 @@ public void testRunfilesExternalOnly(
defaultTimeout(),
defaultSpawnResult());

var builder = defaultSpawnExecBuilder();
List<File> files =
new ArrayList<>(
List.of(
Expand Down Expand Up @@ -700,8 +699,11 @@ public void testRunfilesExternalOnly(
.setPath("bazel-out/k8-fastbuild/bin/tools/foo.runfiles/_main/.runfile")
.build());
}
files.stream().sorted(comparing(File::getPath)).forEach(builder::addInputs);
closeAndAssertLog(context, builder.build());
closeAndAssertLog(
context,
defaultSpawnExecBuilder()
.addAllInputs(files.stream().sorted(comparing(File::getPath)).toList())
.build());
}

@Test
Expand Down

0 comments on commit 94f75fc

Please sign in to comment.