Add --incompatible_compact_repo_mapping

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

@@ -24,6 +24,7 @@
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
 import com.google.common.collect.ImmutableSortedMap;
+import com.google.common.collect.Iterables;
 import com.google.devtools.build.lib.actions.ActionExecutionContext;
 import com.google.devtools.build.lib.actions.ActionKeyContext;
 import com.google.devtools.build.lib.actions.ActionOwner;
@@ -45,9 +46,11 @@
 import java.io.ByteArrayOutputStream;
 import java.io.IOException;
 import java.io.PrintWriter;
+import java.util.AbstractMap;
 import java.util.IdentityHashMap;
 import java.util.Map.Entry;
 import java.util.UUID;
+import java.util.stream.Stream;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.EvalException;
 
@@ -95,6 +98,7 @@ public final class RepoMappingManifestAction extends AbstractFileWriteAction
   private final boolean hasRunfilesSymlinks;
   private final NestedSet<SymlinkEntry> runfilesRootSymlinks;
   private final String workspaceName;
+  private final boolean emitCompactRepoMapping;
 
   public RepoMappingManifestAction(
       ActionOwner owner,
@@ -103,13 +107,15 @@ public RepoMappingManifestAction(
       NestedSet<Artifact> runfilesArtifacts,
       NestedSet<SymlinkEntry> runfilesSymlinks,
       NestedSet<SymlinkEntry> runfilesRootSymlinks,
-      String workspaceName) {
+      String workspaceName,
+      boolean emitCompactRepoMapping) {
     super(owner, NestedSetBuilder.emptySet(Order.STABLE_ORDER), output);
     this.transitivePackages = transitivePackages;
     this.runfilesArtifacts = runfilesArtifacts;
     this.hasRunfilesSymlinks = !runfilesSymlinks.isEmpty();
     this.runfilesRootSymlinks = runfilesRootSymlinks;
     this.workspaceName = workspaceName;
+    this.emitCompactRepoMapping = emitCompactRepoMapping;
   }
 
   @Override
@@ -134,6 +140,7 @@ protected void computeKey(
     fp.addBoolean(hasRunfilesSymlinks);
     actionKeyContext.addNestedSetToFingerprint(FIRST_SEGMENT_FN, fp, runfilesRootSymlinks);
     fp.addString(workspaceName);
+    fp.addBoolean(emitCompactRepoMapping);
   }
 
   /**
@@ -189,41 +196,68 @@ public DeterministicWriter newDeterministicWriter() {
                       // All packages in a given repository have the same repository mapping, so the
                       // particular way of resolving duplicates does not matter.
                       (first, second) -> first));
-      // All repositories generated by a module extension have the same Map instance as the entries
-      // of their RepositoryMapping, with every repo appearing as an entry. If a module extension
-      // generates N repos and all of them are in transitivePackages, iterating over the packages
-      // and then over each mapping's entries would thus require time quadratic in N. We prevent
-      // this by caching the relevant (target apparent name, target canonical name) pairs per entry
-      // map instance.
-      IdentityHashMap<
-              ImmutableMap<String, RepositoryName>, ImmutableList<Entry<String, RepositoryName>>>
-          cachedRelevantEntries = new IdentityHashMap<>();
-      for (var repoAndMapping : sortedRepoMappings.entrySet()) {
-        cachedRelevantEntries
-            .computeIfAbsent(
-                repoAndMapping.getValue().entries(),
-                entries -> computeRelevantEntries(reposInRunfilesPaths, entries))
-            .forEach(
-                mappingEntry -> {
-                  // The canonical name of the main repo is the empty string, which is not a valid
-                  // name for a directory, so the "workspace name" is used the name of the directory
-                  // under the runfiles tree for it.
-                  String targetRepoDirectoryName =
-                      mappingEntry.getValue().isMain()
-                          ? workspaceName
-                          : mappingEntry.getValue().getName();
-                  writer.format(
-                      "%s,%s,%s\n",
-                      repoAndMapping.getKey().getName(),
-                      mappingEntry.getKey(),
-                      targetRepoDirectoryName);
-                });
+      if (emitCompactRepoMapping) {
+        Entry<RepositoryName, RepositoryMapping> lastEntry = null;
+        boolean reused = false;
+        ImmutableSet<Entry<RepositoryName, RepositoryMapping>> sentinel =
+            ImmutableSet.of(
+                new AbstractMap.SimpleImmutableEntry<>(
+                    RepositoryName.MAIN,
+                    RepositoryMapping.create(ImmutableMap.of(), RepositoryName.MAIN)));
+        for (var repoAndMapping : Iterables.concat(sortedRepoMappings.entrySet(), sentinel)) {
+          if (lastEntry != null
+              && repoAndMapping.getValue().entries() == lastEntry.getValue().entries()) {
+            reused = true;
+          } else {
+            if (lastEntry != null) {
+              String sourceRepo = lastEntry.getKey().getName();
+              String prefix;
+              if (reused) {
+                prefix = sourceRepo.substring(0, sourceRepo.lastIndexOf('+') + 1) + "*";
+              } else {
+                prefix = sourceRepo;
+              }
+              computeRelevantEntries(reposInRunfilesPaths, lastEntry.getValue().entries())
+                  .forEach(
+                      mappingEntry ->
+                          writeEntry(
+                              writer, prefix, mappingEntry.getKey(), mappingEntry.getValue()));
+            }
+            reused = false;
+            lastEntry = repoAndMapping;
+          }
+        }
+      } else {
+        // All repositories generated by a module extension have the same Map instance as the
+        // entries of their RepositoryMapping, with every repo appearing as an entry. If a module
+        // extension generates N repos and all of them are in transitivePackages, iterating over the
+        // packages and then over each mapping's entries would thus require time quadratic in N. We
+        // prevent this by caching the relevant (target apparent name, target canonical name) pairs
+        // per entry map instance.
+        IdentityHashMap<
+                ImmutableMap<String, RepositoryName>, ImmutableList<Entry<String, RepositoryName>>>
+            cachedRelevantEntries = new IdentityHashMap<>();
+        for (var repoAndMapping : sortedRepoMappings.entrySet()) {
+          cachedRelevantEntries
+              .computeIfAbsent(
+                  repoAndMapping.getValue().entries(),
+                  entries ->
+                      computeRelevantEntries(reposInRunfilesPaths, entries)
+                          .collect(toImmutableList()))
+              .forEach(
+                  mappingEntry ->
+                      writeEntry(
+                          writer,
+                          repoAndMapping.getKey().getName(),
+                          mappingEntry.getKey(),
+                          mappingEntry.getValue()));
+        }
       }
       writer.flush();
     };
   }
 
-  private ImmutableList<Entry<String, RepositoryName>> computeRelevantEntries(
+  private static Stream<Entry<String, RepositoryName>> computeRelevantEntries(
       ImmutableSet<String> reposInRunfilesPaths,
       ImmutableMap<String, RepositoryName> mappingEntries) {
     // TODO: If this becomes a hotspot, consider iterating over reposInRunfilesPaths and looking
@@ -238,7 +272,19 @@ private ImmutableList<Entry<String, RepositoryName>> computeRelevantEntries(
         .filter(mappingEntry -> !mappingEntry.getKey().isEmpty())
         // We only write entries for repos whose canonical names appear in runfiles paths.
         .filter(entry -> reposInRunfilesPaths.contains(entry.getValue().getName()))
-        .sorted(Entry.comparingByKey())
-        .collect(toImmutableList());
+        .sorted(Entry.comparingByKey());
+  }
+
+  private void writeEntry(
+      PrintWriter writer,
+      String source,
+      String targetApparentName,
+      RepositoryName targetCanonicalName) {
+    // The canonical name of the main repo is the empty string, which is not a valid
+    // name for a directory, so the "workspace name" is used the name of the
+    // directory under the runfiles tree for it.
+    String targetRepoDirectoryName =
+        targetCanonicalName.isMain() ? workspaceName : targetCanonicalName.getName();
+    writer.format("%s,%s,%s\n", source, targetApparentName, targetRepoDirectoryName);
   }
 }

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

@@ -32,6 +32,7 @@
 import com.google.devtools.build.lib.analysis.actions.SymlinkTreeAction;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue;
 import com.google.devtools.build.lib.analysis.config.BuildConfigurationValue.RunfileSymlinksMode;
+import com.google.devtools.build.lib.analysis.config.CoreOptions;
 import com.google.devtools.build.lib.analysis.config.RunUnder;
 import com.google.devtools.build.lib.analysis.config.RunUnder.LabelRunUnder;
 import com.google.devtools.build.lib.analysis.test.TestActionBuilder;
@@ -713,7 +714,12 @@ private static Artifact createRepoMappingManifestAction(
                 runfiles.getArtifacts(),
                 runfiles.getSymlinks(),
                 runfiles.getRootSymlinks(),
-                ruleContext.getWorkspaceName()));
+                ruleContext.getWorkspaceName(),
+                ruleContext
+                    .getConfiguration()
+                    .getOptions()
+                    .get(CoreOptions.class)
+                    .compactRepoMapping));
     return repoMappingManifest;
   }
 

src/main/java/com/google/devtools/build/lib/analysis/config/CoreOptions.java

@@ -538,6 +538,15 @@ public ExecConfigurationDistinguisherSchemeConverter() {
               + "https://bazel.build/extending/rules#runfiles_features_to_avoid).")
   public boolean alwaysIncludeFilesToBuildInData;
 
+  @Option(
+      name = "incompatible_compact_repo_mapping",
+      defaultValue = "false",
+      documentationCategory = OptionDocumentationCategory.OUTPUT_SELECTION,
+      effectTags = {OptionEffectTag.AFFECTS_OUTPUTS},
+      metadataTags = {OptionMetadataTag.INCOMPATIBLE_CHANGE},
+      help = "foobar")
+  public boolean compactRepoMapping;
+
   @Option(
       name = "run_under",
       defaultValue = "null",

src/main/java/com/google/devtools/build/lib/rules/python/BUILD

@@ -26,6 +26,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/analysis:analysis_cluster",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/build_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/core_option_converters",
+        "//src/main/java/com/google/devtools/build/lib/analysis:config/core_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment",
         "//src/main/java/com/google/devtools/build/lib/analysis:config/fragment_options",
         "//src/main/java/com/google/devtools/build/lib/analysis:configured_target",

src/main/java/com/google/devtools/build/lib/rules/python/PyBuiltins.java

@@ -34,6 +34,7 @@
 import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
 import com.google.devtools.build.lib.analysis.actions.AbstractFileWriteAction;
 import com.google.devtools.build.lib.analysis.actions.DeterministicWriter;
+import com.google.devtools.build.lib.analysis.config.CoreOptions;
 import com.google.devtools.build.lib.analysis.starlark.StarlarkRuleContext;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.collect.nestedset.Depset;
@@ -232,7 +233,12 @@ public void repoMappingAction(
                 runfiles.getArtifacts(),
                 runfiles.getSymlinks(),
                 runfiles.getRootSymlinks(),
-                ruleContext.getWorkspaceName()));
+                ruleContext.getWorkspaceName(),
+                ruleContext
+                    .getConfiguration()
+                    .getOptions()
+                    .get(CoreOptions.class)
+                    .compactRepoMapping));
   }
 
   @StarlarkMethod(

src/main/starlark/builtins_bzl/common/builtin_exec_platforms.bzl

@@ -274,6 +274,7 @@ bazel_fragments["CoreOptions"] = fragment(
         "//command_line_option:experimental_inprocess_symlink_creation",
         "//command_line_option:experimental_throttle_action_cache_check",
         "//command_line_option:experimental_use_platforms_in_output_dir_legacy_heuristic",
+        "//command_line_option:incompatible_compact_repo_mapping",
     ],
     inputs = ["//command_line_option:features"],
     outputs = [

src/test/java/com/google/devtools/build/lib/analysis/BUILD

@@ -353,19 +353,23 @@ java_test(
         "//src/main/java/com/google/devtools/build/lib/analysis:repo_mapping_manifest_action",
         "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl",
         "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
+        "//src/main/java/com/google/devtools/build/lib/bazel/repository/starlark",
         "//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_executor_repository_helpers_holder",
+        "//src/main/java/com/google/devtools/build/lib/starlarkbuildapi/repository",
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/vfs",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//src/main/java/net/starlark/java/eval",
         "//src/main/java/net/starlark/java/syntax",
         "//src/test/java/com/google/devtools/build/lib/analysis/util",
         "//src/test/java/com/google/devtools/build/lib/bazel/bzlmod:util",
+        "//src/test/java/com/google/devtools/build/lib/testutil",
         "//third_party:guava",
         "//third_party:junit4",
         "//third_party:truth",
+        "@maven//:com_google_testparameterinjector_test_parameter_injector",
     ],
 )