From e76406654cd11171ee09fb83bc396e19d4577442 Mon Sep 17 00:00:00 2001
From: Googler <wyv@google.com>
Date: Fri, 30 May 2025 01:45:56 -0700
Subject: [PATCH] [8.7.0] Fix crash when repo contents cache is under the main
 repo

We try to release the shared lock in `afterCommand`, but that crashes if we never acquired the shared lock in `beforeCommand` because the repo content cache is under the main repo.

Also fix wording to be more accurate (s/workspace/main repo)

PiperOrigin-RevId: 765087159
Change-Id: Iee2e6f25e361fcd67c8231b7fb1f99db7336027b
(cherry picked from commit 7b792b65f7a4a48c26733b5657f8be89a1761d69)
---
 .../build/lib/bazel/BazelRepositoryModule.java  | 10 +++++-----
 .../shell/bazel/bazel_repository_cache_test.sh  | 17 +++++++++++++++++
 2 files changed, 22 insertions(+), 5 deletions(-)

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
index 45c4e7f0abbdcb..dea60fd31e4562 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/BazelRepositoryModule.java
@@ -397,9 +397,9 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
       if (repoContentsCachePath != null
           && env.getWorkspace() != null
           && repoContentsCachePath.startsWith(env.getWorkspace())) {
-        // Having the repo contents cache inside the workspace is very dangerous. During the
-        // lifetime of a Bazel invocation, we treat files inside the workspace as immutable. This
-        // can cause mysterious failures if we write files inside the workspace during the
+        // Having the repo contents cache inside the main repo is very dangerous. During the
+        // lifetime of a Bazel invocation, we treat files inside the main repo as immutable. This
+        // can cause mysterious failures if we write files inside the main repo during the
         // invocation, as is often the case with the repo contents cache.
         // TODO: wyv@ - This is a crude check that disables some use cases (such as when the output
         //   base itself is inside the main repo). Investigate a better check.
@@ -407,9 +407,9 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
         throw new AbruptExitException(
             detailedExitCode(
                 """
-                The repo contents cache [%s] is inside the workspace [%s]. This can cause spurious \
+                The repo contents cache [%s] is inside the main repo [%s]. This can cause spurious \
                 failures. Disable the repo contents cache with `--repo_contents_cache=`, or \
-                specify `--repo_contents_cache=<path outside the workspace>`.
+                specify `--repo_contents_cache=<path outside the main repo>`.
                 """
                     .formatted(repoContentsCachePath, env.getWorkspace()),
                 Code.BAD_REPO_CONTENTS_CACHE));
diff --git a/src/test/shell/bazel/bazel_repository_cache_test.sh b/src/test/shell/bazel/bazel_repository_cache_test.sh
index b698b9580420b5..c55799a73c3d17 100755
--- a/src/test/shell/bazel/bazel_repository_cache_test.sh
+++ b/src/test/shell/bazel/bazel_repository_cache_test.sh
@@ -541,4 +541,21 @@ EOF
     && fail "expected failure" || :
 }
 
+function test_contents_cache_not_allowed_in_main_repo() {
+  # For some reason, this same test written in Python causes it to hang forever
+  # in the regression case, instead of failing immediately. So we write it in
+  # shell.
+  echo 'filegroup(name="foo")' > BUILD
+  if (bazel build --repo_contents_cache=inside foo >& $TEST_log); then
+    fail "expected exit code 2, got 0"
+  else
+    exit_code=$?
+    if [ $exit_code -ne 2 ]; then
+      fail "expected exit code 2, got $exit_code"
+    else
+      :
+    fi
+  fi
+}
+
 run_suite "repository cache tests"