Simplify and support legacyExternalRunfiles

src/main/java/com/google/devtools/build/lib/actions/ActionExecutionContext.java

@@ -98,6 +98,16 @@ public NestedSet<Artifact> getArtifactsAtCanonicalLocationsForLogging() {
  public Map<PathFragment, Artifact> getAllSymlinksForLogging() {
  return wrapped.getAllSymlinksForLogging();
  }
+
+ @Override
+ public NestedSet<String> getEmptyFilenamesForLogging() {
+ return wrapped.getEmptyFilenamesForLogging();
+ }
+
+ @Override
+ public boolean isLegacyExternalRunfiles() {
+ return wrapped.isLegacyExternalRunfiles();
+ }
  }
 
  /**

src/main/java/com/google/devtools/build/lib/actions/RunfilesTree.java

@@ -49,4 +49,8 @@ public interface RunfilesTree {
  NestedSet<Artifact> getArtifactsAtCanonicalLocationsForLogging();
 
  Map<PathFragment, Artifact> getAllSymlinksForLogging();
+
+ NestedSet<String> getEmptyFilenamesForLogging();
+
+ boolean isLegacyExternalRunfiles();
 }

src/main/java/com/google/devtools/build/lib/analysis/Runfiles.java

@@ -251,6 +251,9 @@ public NestedSet<SymlinkEntry> getSymlinks() {
  }
 
  public NestedSet<String> getEmptyFilenames() {
+ if (emptyFilesSupplier == DUMMY_EMPTY_FILES_SUPPLIER) {
+ return NestedSetBuilder.emptySet(Order.STABLE_ORDER);
+ }
  Set<PathFragment> manifestKeys =
  Streams.concat(
  symlinks.toList().stream().map(SymlinkEntry::getPath),
@@ -399,6 +402,10 @@ public Map<PathFragment, Artifact> getAllSymlinksForLogging(
  return builder.build();
  }
 
+ public boolean isLegacyExternalRunfiles() {
+ return legacyExternalRunfiles;
+ }
+
  /**
  * Helper class to handle munging the paths of external artifacts.
  */

src/main/java/com/google/devtools/build/lib/analysis/RunfilesSupport.java

@@ -185,6 +185,16 @@ public Map<PathFragment, Artifact> getAllSymlinksForLogging() {
  return runfiles.getAllSymlinksForLogging(repoMappingManifest);
  }
 
+ @Override
+ public NestedSet<String> getEmptyFilenamesForLogging() {
+ return runfiles.getEmptyFilenames();
+ }
+
+ @Override
+ public boolean isLegacyExternalRunfiles() {
+ return runfiles.isLegacyExternalRunfiles();
+ }
+
  @Override
  public RunfileSymlinksMode getSymlinksMode() {
  return runfileSymlinksMode;

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 = [

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);

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<String, Collection<File>> 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<PathFragment> 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,25 +251,28 @@ private Pair<String, Collection<File>> reconstructRunfilesDir(
  return Pair.of(runfilesTree.getPath(), ImmutableList.copyOf(builder.values()));
  }
 
- private Stream<String> getRunfilesPaths(String originalPath) {
+ private Stream<String> getRunfilesPaths(String originalPath, boolean legacyExternalRunfiles) {
  String path = originalPath;
  if (path.startsWith("bazel-out/") || path.startsWith("blaze-out/")) {
  // Trim the first three segments ("bazel-out/<config>/bin/") from the path.
  int thirdSlash = path.indexOf('/', path.indexOf('/', "bazel-out/".length()) + 1);
  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<String> 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);
  }
 
  private Collection<File> 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());

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

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<string, SymlinkTarget> symlinks = 3;
- bool create_init_py = 4;
+ bool legacy_external_runfiles = 4;
  }
 
  // A set of spawn inputs.

src/test/java/com/google/devtools/build/lib/analysis/util/FakeRunfilesTree.java

@@ -100,4 +100,14 @@ public NestedSet<Artifact> getArtifactsAtCanonicalLocationsForLogging() {
  public Map<PathFragment, Artifact> getAllSymlinksForLogging() {
  return runfiles.getAllSymlinksForLogging(repoMappingManifest);
  }
+
+ @Override
+ public NestedSet<String> getEmptyFilenamesForLogging() {
+ return runfiles.getEmptyFilenames();
+ }
+
+ @Override
+ public boolean isLegacyExternalRunfiles() {
+ return runfiles.isLegacyExternalRunfiles();
+ }
 }

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

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,27 +998,26 @@ 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<String, Artifact> symlinks,
  Map<String, Artifact> 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<String, Artifact> entry : symlinks.entrySet()) {
  runfiles.addSymlink(PathFragment.create(entry.getKey()), entry.getValue());
  }
  for (Map.Entry<String, Artifact> 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());
  }
 

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