diff --git a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java index 4a5e435413f51a..8ee30a9d8fcc7f 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java +++ b/src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java @@ -98,6 +98,16 @@ public NestedSet getArtifactsAtCanonicalLocationsForLogging() { public Map getAllSymlinksForLogging() { return wrapped.getAllSymlinksForLogging(); } + + @Override + public NestedSet getEmptyFilenamesForLogging() { + return wrapped.getEmptyFilenamesForLogging(); + } + + @Override + public boolean isLegacyExternalRunfiles() { + return wrapped.isLegacyExternalRunfiles(); + } } /** diff --git a/src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java b/src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java index dbd91a1a0c8920..70a3d5cb41c0b1 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java +++ b/src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java @@ -49,4 +49,8 @@ public interface RunfilesTree { NestedSet getArtifactsAtCanonicalLocationsForLogging(); Map getAllSymlinksForLogging(); + + NestedSet getEmptyFilenamesForLogging(); + + boolean isLegacyExternalRunfiles(); } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java index 6fa1ab50c7e81b..c7ba64e29bae43 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java @@ -251,6 +251,9 @@ public NestedSet getSymlinks() { } public NestedSet getEmptyFilenames() { + if (emptyFilesSupplier == DUMMY_EMPTY_FILES_SUPPLIER) { + return NestedSetBuilder.emptySet(Order.STABLE_ORDER); + } Set manifestKeys = Streams.concat( symlinks.toList().stream().map(SymlinkEntry::getPath), @@ -399,6 +402,10 @@ public Map getAllSymlinksForLogging( return builder.build(); } + public boolean isLegacyExternalRunfiles() { + return legacyExternalRunfiles; + } + /** * Helper class to handle munging the paths of external artifacts. */ diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java index c1d90d4e44c9bb..678ad9d4744e47 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java +++ b/src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java @@ -185,6 +185,16 @@ public Map getAllSymlinksForLogging() { return runfiles.getAllSymlinksForLogging(repoMappingManifest); } + @Override + public NestedSet getEmptyFilenamesForLogging() { + return runfiles.getEmptyFilenames(); + } + + @Override + public boolean isLegacyExternalRunfiles() { + return runfiles.isLegacyExternalRunfiles(); + } + @Override public RunfileSymlinksMode getSymlinksMode() { return runfileSymlinksMode; diff --git a/src/main/java/com/google/devtools/build/lib/exec/BUILD b/src/main/java/com/google/devtools/build/lib/exec/BUILD index d9f0be54cebb95..ec4b1f822cd82a 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/main/java/com/google/devtools/build/lib/exec/BUILD @@ -272,7 +272,7 @@ java_library( "SpawnLogContext.java", ], deps = [ - ":stable_sort", + ":spawn_log_context_utils", "//src/main/java/com/google/devtools/build/lib/actions", "//src/main/java/com/google/devtools/build/lib/actions:artifacts", "//src/main/java/com/google/devtools/build/lib/actions:file_metadata", @@ -298,13 +298,12 @@ java_library( ) java_library( - name = "spawn_log_reconstructor", + name = "spawn_log_context_utils", srcs = [ "SpawnLogReconstructor.java", + "StableSort.java", ], deps = [ - ":stable_sort", - "//src/main/java/com/google/devtools/build/lib/bazel/rules/python", "//src/main/java/com/google/devtools/build/lib/cmdline", "//src/main/java/com/google/devtools/build/lib/profiler", "//src/main/java/com/google/devtools/build/lib/util:pair", @@ -317,21 +316,6 @@ java_library( ], ) -java_library( - name = "stable_sort", - srcs = ["StableSort.java"], - deps = [ - "//src/main/java/com/google/devtools/build/lib/cmdline", - "//src/main/java/com/google/devtools/build/lib/profiler", - "//src/main/java/com/google/devtools/build/lib/util:pair", - "//src/main/java/com/google/devtools/build/lib/util/io:io-proto", - "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", - "//src/main/protobuf:spawn_java_proto", - "//third_party:guava", - "//third_party:jsr305", - ], -) - java_library( name = "spawn_runner", srcs = [ diff --git a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java index 746e26d3aabe6e..054c62990215e1 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java +++ b/src/main/java/com/google/devtools/build/lib/exec/CompactSpawnLogContext.java @@ -50,6 +50,7 @@ import com.google.devtools.build.lib.vfs.Symlinks; import com.google.devtools.build.lib.vfs.XattrProvider; import com.google.errorprone.annotations.CanIgnoreReturnValue; +import com.google.protobuf.Empty; import it.unimi.dsi.fastutil.objects.Object2IntOpenHashMap; import java.io.BufferedOutputStream; import java.io.IOException; @@ -60,6 +61,7 @@ import java.util.List; import java.util.Map; import java.util.SortedMap; +import java.util.TreeMap; import java.util.concurrent.ForkJoinPool; import javax.annotation.Nullable; import javax.annotation.concurrent.GuardedBy; @@ -454,7 +456,8 @@ private int logRunfilesTree( ExecLogEntry.RunfilesTree.Builder builder = ExecLogEntry.RunfilesTree.newBuilder() - .setPath(runfilesTree.getExecPath().getPathString()); + .setPath(runfilesTree.getExecPath().getPathString()) + .setLegacyExternalRunfiles(runfilesTree.isLegacyExternalRunfiles()); builder.setArtifactsId( logNestedSet( @@ -483,6 +486,24 @@ private int logRunfilesTree( } else { symlinkTarget.setUnresolvedSymlinkId(logUnresolvedSymlink(artifact, path)); } + builder.putSymlinks(relativePath.getPathString(), symlinkTarget.build()); + } + + var emptyFilenames = runfilesTree.getEmptyFilenamesForLogging(); + if (!emptyFilenames.isEmpty()) { + // This case is rare: it only applies to Python runfiles and only with + // --incompatible_default_to_explicit_init_py. We optimize for the common case and thus + // have to copy and sort the symlinks map here. + var symlinks = new TreeMap<>(builder.getSymlinksMap()); + for (String emptyFilename : emptyFilenames.toList()) { + symlinks.put( + PathFragment.create(workspaceName).getRelative(emptyFilename).getPathString(), + ExecLogEntry.RunfilesTree.SymlinkTarget.newBuilder() + .setEmptyFile(Empty.getDefaultInstance()) + .build()); + } + builder.clearSymlinks(); + builder.putAllSymlinks(symlinks); } return ExecLogEntry.newBuilder().setRunfilesTree(builder); diff --git a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogReconstructor.java b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogReconstructor.java index 22efbafe8639d9..85cecc7307ee4a 100644 --- a/src/main/java/com/google/devtools/build/lib/exec/SpawnLogReconstructor.java +++ b/src/main/java/com/google/devtools/build/lib/exec/SpawnLogReconstructor.java @@ -14,19 +14,16 @@ package com.google.devtools.build.lib.exec; import static com.google.common.collect.ImmutableList.toImmutableList; -import static com.google.common.collect.ImmutableSet.toImmutableSet; import com.github.luben.zstd.ZstdInputStream; import com.google.common.collect.ImmutableList; import com.google.common.collect.Iterables; -import com.google.devtools.build.lib.bazel.rules.python.BazelPyBuiltins; import com.google.devtools.build.lib.cmdline.LabelConstants; import com.google.devtools.build.lib.exec.Protos.ExecLogEntry; import com.google.devtools.build.lib.exec.Protos.File; import com.google.devtools.build.lib.exec.Protos.SpawnExec; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.util.io.MessageInputStream; -import com.google.devtools.build.lib.vfs.PathFragment; import java.io.IOException; import java.io.InputStream; import java.util.ArrayDeque; @@ -35,7 +32,6 @@ import java.util.HashSet; import java.util.LinkedHashMap; import java.util.Map; -import java.util.Set; import java.util.SortedMap; import java.util.SortedSet; import java.util.TreeMap; @@ -240,25 +236,13 @@ private Pair> reconstructRunfilesDir( inputs.stream() .flatMap( file -> - getRunfilesPaths(file.getPath()) + getRunfilesPaths(file.getPath(), runfilesTree.getLegacyExternalRunfiles()) .map( relativePath -> file.toBuilder() .setPath(runfilesTree.getPath() + "/" + relativePath) .build())) .forEach(file -> builder.put(file.getPath(), file)); - if (runfilesTree.getCreateInitPy()) { - Set manifest = - Stream.concat( - runfilesTree.getSymlinksMap().keySet().stream(), - inputs.stream().flatMap(file -> getRunfilesPaths(file.getPath()))) - .map(PathFragment::create) - .collect(toImmutableSet()); - for (PathFragment emptyFilePath : BazelPyBuiltins.GET_INIT_PY_FILES.getExtraPaths(manifest)) { - String path = runfilesTree.getPath() + "/" + emptyFilePath.getPathString(); - builder.put(path, File.newBuilder().setPath(path).build()); - } - } String workspaceDir = runfilesTree.getPath() + "/" + workspaceRunfilesDirectory + "/"; if (builder.keySet().stream().noneMatch(p -> p.startsWith(workspaceDir))) { String emptyRunfile = workspaceDir + ".runfile"; @@ -267,7 +251,7 @@ private Pair> reconstructRunfilesDir( return Pair.of(runfilesTree.getPath(), ImmutableList.copyOf(builder.values())); } - private Stream getRunfilesPaths(String originalPath) { + private Stream getRunfilesPaths(String originalPath, boolean legacyExternalRunfiles) { String path = originalPath; if (path.startsWith("bazel-out/") || path.startsWith("blaze-out/")) { // Trim the first three segments ("bazel-out//bin/") from the path. @@ -275,10 +259,12 @@ private Stream getRunfilesPaths(String originalPath) { path = path.substring(thirdSlash + 1); } if (path.startsWith(LabelConstants.EXTERNAL_PATH_PREFIX.getPathString())) { - return Stream.of( - path.substring(LabelConstants.EXTERNAL_PATH_PREFIX.getPathString().length()), - // --legacy_external_runfiles - workspaceRunfilesDirectory + "/" + path); + Stream.Builder paths = Stream.builder(); + paths.add(path.substring(LabelConstants.EXTERNAL_PATH_PREFIX.getPathString().length())); + if (legacyExternalRunfiles) { + paths.add(workspaceRunfilesDirectory + "/" + path); + } + return paths.build(); } return Stream.of(workspaceRunfilesDirectory + "/" + path); } @@ -286,6 +272,7 @@ private Stream getRunfilesPaths(String originalPath) { private Collection reconstructRunfilesSymlinkTarget( String newPath, ExecLogEntry.RunfilesTree.SymlinkTarget target) throws IOException { return switch (target.getTargetCase()) { + case EMPTY_FILE -> ImmutableList.of(File.newBuilder().setPath(newPath).build()); case FILE_ID -> { var file = getFromMap(fileMap, target.getFileId()); yield ImmutableList.of(file.toBuilder().setPath(newPath).build()); diff --git a/src/main/protobuf/BUILD b/src/main/protobuf/BUILD index 177d778bde7322..e96a6428287cb9 100644 --- a/src/main/protobuf/BUILD +++ b/src/main/protobuf/BUILD @@ -316,6 +316,7 @@ proto_library( srcs = ["spawn.proto"], deps = [ "@com_google_protobuf//:duration_proto", + "@com_google_protobuf//:empty_proto", "@com_google_protobuf//:timestamp_proto", ], ) diff --git a/src/main/protobuf/spawn.proto b/src/main/protobuf/spawn.proto index d24661a0eb73f3..969a78013f1f0b 100644 --- a/src/main/protobuf/spawn.proto +++ b/src/main/protobuf/spawn.proto @@ -17,6 +17,7 @@ syntax = "proto3"; package tools.protos; import "google/protobuf/duration.proto"; +import "google/protobuf/empty.proto"; import "google/protobuf/timestamp.proto"; option java_package = "com.google.devtools.build.lib.exec"; @@ -263,6 +264,7 @@ message ExecLogEntry { int32 file_id = 1; int32 directory_id = 2; int32 unresolved_symlink_id = 3; + google.protobuf.Empty empty_file = 4; } } @@ -287,7 +289,7 @@ message ExecLogEntry { // The artifacts in the runfiles tree that are symlinked at custom // locations. map symlinks = 3; - bool create_init_py = 4; + bool legacy_external_runfiles = 4; } // A set of spawn inputs. diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/FakeRunfilesTree.java b/src/test/java/com/google/devtools/build/lib/analysis/util/FakeRunfilesTree.java index 084cbbd9b4704a..f063f0fa02b697 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/FakeRunfilesTree.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/FakeRunfilesTree.java @@ -100,4 +100,14 @@ public NestedSet getArtifactsAtCanonicalLocationsForLogging() { public Map getAllSymlinksForLogging() { return runfiles.getAllSymlinksForLogging(repoMappingManifest); } + + @Override + public NestedSet getEmptyFilenamesForLogging() { + return runfiles.getEmptyFilenames(); + } + + @Override + public boolean isLegacyExternalRunfiles() { + return runfiles.isLegacyExternalRunfiles(); + } } diff --git a/src/test/java/com/google/devtools/build/lib/exec/BUILD b/src/test/java/com/google/devtools/build/lib/exec/BUILD index cb7d1b1ac63210..783982388a3a35 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/BUILD +++ b/src/test/java/com/google/devtools/build/lib/exec/BUILD @@ -52,11 +52,10 @@ java_library( "//src/main/java/com/google/devtools/build/lib/exec:spawn_exec_exception", "//src/main/java/com/google/devtools/build/lib/exec:spawn_input_expander", "//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context", - "//src/main/java/com/google/devtools/build/lib/exec:spawn_log_reconstructor", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context_utils", "//src/main/java/com/google/devtools/build/lib/exec:spawn_runner", "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_policy", "//src/main/java/com/google/devtools/build/lib/exec:spawn_strategy_registry", - "//src/main/java/com/google/devtools/build/lib/exec:stable_sort", "//src/main/java/com/google/devtools/build/lib/exec:standalone_test_result", "//src/main/java/com/google/devtools/build/lib/exec:standalone_test_strategy", "//src/main/java/com/google/devtools/build/lib/exec:streamed_test_output", diff --git a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java index 65a834308e1ecc..314bbf83fc6b40 100644 --- a/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java +++ b/src/test/java/com/google/devtools/build/lib/exec/SpawnLogContextTestBase.java @@ -419,13 +419,7 @@ public void testRunfilesEmptyInput() throws Exception { Artifact runfilesMiddleman = ActionsTestUtil.createArtifact(middlemanDir, "runfiles"); Artifact runfilesInput = ActionsTestUtil.createArtifact(rootDir, "sub/dir/script.py"); PathFragment runfilesRoot = outputDir.getExecPath().getRelative("foo.runfiles"); - RunfilesTree runfilesTree = - createRunfilesTree( - runfilesRoot, - ImmutableMap.of(), - ImmutableMap.of(), - /* createEmptyFiles= */ true, - runfilesInput); + RunfilesTree runfilesTree = createRunfilesTree(runfilesRoot, runfilesInput); writeFile(runfilesInput, "abc"); @@ -1004,17 +998,18 @@ protected static SpawnExec.Builder defaultSpawnExecBuilder() { } protected static RunfilesTree createRunfilesTree(PathFragment root, Artifact... artifacts) { - return createRunfilesTree(root, ImmutableMap.of(), ImmutableMap.of(), false, artifacts); + return createRunfilesTree( + root, ImmutableMap.of(), ImmutableMap.of(), /* legacyExternalRunfiles= */ false, artifacts); } protected static RunfilesTree createRunfilesTree( PathFragment root, Map symlinks, Map rootSymlinks, - boolean createEmptyFiles, + boolean legacyExternalRunfiles, Artifact... artifacts) { Runfiles.Builder runfiles = - new Runfiles.Builder(TestConstants.WORKSPACE_NAME, /* legacyExternalRunfiles= */ true); + new Runfiles.Builder(TestConstants.WORKSPACE_NAME, legacyExternalRunfiles); runfiles.addArtifacts(Arrays.asList(artifacts)); for (Map.Entry entry : symlinks.entrySet()) { runfiles.addSymlink(PathFragment.create(entry.getKey()), entry.getValue()); @@ -1022,9 +1017,7 @@ protected static RunfilesTree createRunfilesTree( for (Map.Entry entry : rootSymlinks.entrySet()) { runfiles.addRootSymlink(PathFragment.create(entry.getKey()), entry.getValue()); } - if (createEmptyFiles) { - runfiles.setEmptyFilesSupplier(BazelPyBuiltins.GET_INIT_PY_FILES); - } + runfiles.setEmptyFilesSupplier(BazelPyBuiltins.GET_INIT_PY_FILES); return new RunfilesSupport.RunfilesTreeImpl(root, runfiles.build()); } diff --git a/src/tools/execlog/src/main/java/com/google/devtools/build/execlog/BUILD b/src/tools/execlog/src/main/java/com/google/devtools/build/execlog/BUILD index 8bbec35fd044bb..e87e67acb563ee 100644 --- a/src/tools/execlog/src/main/java/com/google/devtools/build/execlog/BUILD +++ b/src/tools/execlog/src/main/java/com/google/devtools/build/execlog/BUILD @@ -31,7 +31,7 @@ java_library( "ExecLogConverter.java", ], deps = [ - "//src/main/java/com/google/devtools/build/lib/exec:spawn_log_reconstructor", + "//src/main/java/com/google/devtools/build/lib/exec:spawn_log_context_utils", "//src/main/java/com/google/devtools/build/lib/util/io:io-proto", "//src/main/java/com/google/devtools/common/options", "//src/main/protobuf:spawn_java_proto",