Generate repo mapping manifest when it's possible (not only when Bzlm…

…od is enabled).

Related #18551

PiperOrigin-RevId: 540213563
Change-Id: I37051f3ded24e3b7073401b8549a959e392e0dca

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

@@ -33,7 +33,6 @@
 import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.packages.Type;
-import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -534,11 +533,8 @@ public static Path outputManifestPath(Path runfilesDir) {
   @Nullable
   private static Artifact createRepoMappingManifestAction(
       RuleContext ruleContext, Runfiles runfiles, Artifact owningExecutable) {
-    if (!ruleContext
-        .getAnalysisEnvironment()
-        .getStarlarkSemantics()
-        .getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
-      // If Bzlmod is not enabled, we don't need a repo mapping manifest.
+    if (ruleContext.getTransitivePackagesForRunfileRepoMappingManifest() == null) {
+      // If transitive packages are not tracked for repo mapping manifest, we don't need the action.
       return null;
     }
 

src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java

@@ -455,6 +455,7 @@ public abstract class SkyframeExecutor implements WalkableGraphFactory, Configur
   @Nullable protected final DiffAwarenessManager diffAwarenessManager;
   // If this is null then workspace header pre-calculation won't happen.
   @Nullable private final SkyframeExecutorRepositoryHelpersHolder repositoryHelpersHolder;
+  private boolean repositoryHelpersHolderIgnored = false;
   @Nullable private final WorkspaceInfoFromDiffReceiver workspaceInfoFromDiffReceiver;
   private Set<String> previousClientEnvironment = ImmutableSet.of();
 
@@ -1084,7 +1085,12 @@ private boolean shouldStoreTransitivePackagesInLoadingAndAnalysis() {
     // external repository support. They are never needed if external repositories are disabled. To
     // avoid complexity from toggling this, just choose a setting for the lifetime of the server.
     // TODO(b/283125139): Can we support external repositories without tracking transitive packages?
-    return repositoryHelpersHolder != null;
+    return repositoryHelpersHolder != null && !repositoryHelpersHolderIgnored;
+  }
+
+  @VisibleForTesting
+  public void ignoreRepositoryHelpersHolderForTesting() {
+    this.repositoryHelpersHolderIgnored = true;
   }
 
   @VisibleForTesting

src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java

@@ -57,6 +57,9 @@ protected void setupOptions() throws Exception {
         "--experimental_remote_include_extraction_size_threshold=0",
         "--keep_going=" + keepGoing);
     runtimeWrapper.registerSubscriber(actionEventRecorder);
+    // Tell Skyframe to ignore RepositoryHelpersHolder so that we don't trigger
+    // RepoMappingManifestAction to preserve the expected order of Actions.
+    this.getSkyframeExecutor().ignoreRepositoryHelpersHolderForTesting();
   }
 
   /**

src/test/shell/bazel/remote/remote_execution_test.sh

@@ -299,7 +299,7 @@ EOF
       --remote_executor=grpc://localhost:${worker_port} \
       //a:test >& $TEST_log \
       || fail "Failed to build //a:test with remote execution"
-  expect_log "6 processes: 4 internal, 2 remote"
+  expect_log "7 processes: 5 internal, 2 remote"
   diff bazel-bin/a/test ${TEST_TMPDIR}/test_expected \
       || fail "Remote execution generated different result"
 }
@@ -2579,15 +2579,15 @@ EOF
     --disk_cache=$CACHEDIR \
     //a:test >& $TEST_log || fail "Failed to build //a:test"
 
-  expect_log "5 processes: 3 internal, 2 .*-sandbox"
+  expect_log "6 processes: 4 internal, 2 .*-sandbox"
 
   bazel clean
 
   bazel test \
     --disk_cache=$CACHEDIR \
     //a:test >& $TEST_log || fail "Failed to build //a:test"
 
-  expect_log "5 processes: 2 disk cache hit, 3 internal"
+  expect_log "6 processes: 2 disk cache hit, 4 internal"
 }
 
 # Bazel assumes that non-ASCII characters in file contents (and, in

src/test/shell/integration/runfiles_test.sh

@@ -236,6 +236,7 @@ EOF
   assert_equals "$expected" "$actual"
 
   # The manifest only records files and symlinks, not real directories
+  expected="$expected$(get_repo_mapping_manifest_file)"
   expected_manifest_size=$(echo "$expected" | grep -v ' regular dir' | wc -l)
   actual_manifest_size=$(wc -l < ../MANIFEST)
   assert_equals $expected_manifest_size $actual_manifest_size
@@ -255,6 +256,17 @@ EOF
       fi
     fi
   done
+
+  # Add the repo mapping manifest entry for Bazel.
+  if [[ "$PRODUCT_NAME" == "bazel" ]]; then
+    repo_mapping="_repo_mapping"
+    repo_mapping_target="$(readlink "$repo_mapping")"
+    if "$is_windows"; then
+      repo_mapping_target="$(cygpath -m $repo_mapping_target)"
+    fi
+    echo "$repo_mapping $repo_mapping_target" >> ${TEST_TMPDIR}/MANIFEST2
+  fi
+
   sort MANIFEST > ${TEST_TMPDIR}/MANIFEST_sorted
   sort ${TEST_TMPDIR}/MANIFEST2 > ${TEST_TMPDIR}/MANIFEST2_sorted
   diff -u ${TEST_TMPDIR}/MANIFEST_sorted ${TEST_TMPDIR}/MANIFEST2_sorted
@@ -310,13 +322,21 @@ EOF
     assert_equals  0 $(find ${WORKSPACE_NAME} -type f | wc -l)
     assert_equals  5 $(find ${WORKSPACE_NAME} -type d | wc -l)
     assert_equals  9 $(find ${WORKSPACE_NAME} | wc -l)
-    assert_equals  4 $(wc -l < MANIFEST)
+    if [[ "$PRODUCT_NAME" == "bazel" ]]; then
+      assert_equals  5 $(wc -l < MANIFEST)
+    else
+      assert_equals  4 $(wc -l < MANIFEST)
+    fi
   else
     assert_equals  3 $(find ${WORKSPACE_NAME} -type l | wc -l)
     assert_equals  0 $(find ${WORKSPACE_NAME} -type f | wc -l)
     assert_equals  5 $(find ${WORKSPACE_NAME} -type d | wc -l)
     assert_equals  8 $(find ${WORKSPACE_NAME} | wc -l)
-    assert_equals  3 $(wc -l < MANIFEST)
+    if [[ "$PRODUCT_NAME" == "bazel" ]]; then
+      assert_equals  4 $(wc -l < MANIFEST)
+    else
+      assert_equals  3 $(wc -l < MANIFEST)
+    fi
   fi
 
   rm -f ${TEST_TMPDIR}/MANIFEST
@@ -333,6 +353,17 @@ EOF
       fi
     fi
   done
+
+  # Add the repo mapping manifest entry for Bazel.
+  if [[ "$PRODUCT_NAME" == "bazel" ]]; then
+    repo_mapping="_repo_mapping"
+    repo_mapping_target="$(readlink "$repo_mapping")"
+    if "$is_windows"; then
+      repo_mapping_target="$(cygpath -m $repo_mapping_target)"
+    fi
+    echo "$repo_mapping $repo_mapping_target" >> ${TEST_TMPDIR}/MANIFEST2
+  fi
+
   sort MANIFEST > ${TEST_TMPDIR}/MANIFEST_sorted
   sort ${TEST_TMPDIR}/MANIFEST2 > ${TEST_TMPDIR}/MANIFEST2_sorted
   diff -u ${TEST_TMPDIR}/MANIFEST_sorted ${TEST_TMPDIR}/MANIFEST2_sorted

src/test/shell/integration/runfiles_test_utils.sh

@@ -20,3 +20,9 @@
 function get_python_runtime_runfiles() {
   :;
 }
+
+# Additional repo mapping manifest file.
+function get_repo_mapping_manifest_file() {
+  echo ""
+  echo "../repo_mapping file"
+}

src/test/shell/integration/ui_test.sh

@@ -724,7 +724,7 @@ EOF
   expect_log "ERROR: 'run' only works with tests with one shard"
   # If we would print this again after the run failed, we would overwrite the
   # error message above.
-  expect_log_n "INFO: Build completed successfully, [45] total actions" 1
+  expect_log_n "INFO: Build completed successfully, [456] total actions" 1
 }
 
 run_suite "Integration tests for ${PRODUCT_NAME}'s UI"