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 8d2f32392efb7d..827ddde92e85e4 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 @@ -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; } diff --git a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java b/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java index 366a992aa5dfe0..4570efe130b117 100644 --- a/src/main/java/com/google/devtools/build/lib/skyframe/SkyframeExecutor.java +++ b/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 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 diff --git a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java b/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java index d3459d5e6a77a3..96cd8a3fe3b1db 100644 --- a/src/test/java/com/google/devtools/build/lib/skyframe/rewinding/RewindingTest.java +++ b/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(); } /** diff --git a/src/test/shell/bazel/remote/remote_execution_test.sh b/src/test/shell/bazel/remote/remote_execution_test.sh index e3e4188d5132bd..7bdcab96020deb 100755 --- a/src/test/shell/bazel/remote/remote_execution_test.sh +++ b/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,7 +2579,7 @@ 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 @@ -2587,7 +2587,7 @@ EOF --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 diff --git a/src/test/shell/integration/runfiles_test.sh b/src/test/shell/integration/runfiles_test.sh index f4481ce9f15aec..2cdc1848aa8e59 100755 --- a/src/test/shell/integration/runfiles_test.sh +++ b/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 diff --git a/src/test/shell/integration/runfiles_test_utils.sh b/src/test/shell/integration/runfiles_test_utils.sh index 2a0ccf3798aafb..fe8d05e9d967fd 100644 --- a/src/test/shell/integration/runfiles_test_utils.sh +++ b/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" +} diff --git a/src/test/shell/integration/ui_test.sh b/src/test/shell/integration/ui_test.sh index 6432dfe1d0ca4f..c9b28d1d3942ab 100755 --- a/src/test/shell/integration/ui_test.sh +++ b/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"