Do not record any repo mapping entries in the RepoMappingRecorder for…

… WORKSPACE repo rules

Follow-up to 1cbf09d; beyond not recording those entries in the marker file, we shouldn't even record them in memory in the first place. This avoids nasty problems with unexported repo rules (gah) and should be an extremely tiny performance win as a bonus...

Fixes #21451

PiperOrigin-RevId: 609010175
Change-Id: I90eb921b09068f327b42886ea0a1875374a94049

src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java

@@ -230,9 +230,14 @@ private RepositoryDirectoryValue.Builder fetchInternal(
       StarlarkThread thread = new StarlarkThread(mu, starlarkSemantics);
       thread.setPrintHandler(Event.makeDebugPrintHandler(env.getListener()));
       var repoMappingRecorder = new Label.RepoMappingRecorder();
-      repoMappingRecorder.mergeEntries(
-          rule.getRuleClassObject().getRuleDefinitionEnvironmentRepoMappingEntries());
-      thread.setThreadLocal(Label.RepoMappingRecorder.class, repoMappingRecorder);
+      // For repos defined in Bzlmod, record any used repo mappings in the marker file.
+      // Repos defined in WORKSPACE are impossible to verify given the chunked loading (we'd have to
+      // record which chunk the repo mapping was used in, and ain't nobody got time for that).
+      if (!isWorkspaceRepo(rule)) {
+        repoMappingRecorder.mergeEntries(
+            rule.getRuleClassObject().getRuleDefinitionEnvironmentRepoMappingEntries());
+        thread.setThreadLocal(Label.RepoMappingRecorder.class, repoMappingRecorder);
+      }
 
       new BazelStarlarkContext(
               BazelStarlarkContext.Phase.LOADING, // ("fetch")
@@ -320,17 +325,12 @@ private RepositoryDirectoryValue.Builder fetchInternal(
             new RepoRecordedInput.EnvVar(envKey), clientEnvironment.get(envKey));
       }
 
-      // For repos defined in Bzlmod, record any used repo mappings in the marker file.
-      // Repos defined in WORKSPACE are impossible to verify given the chunked loading (we'd have to
-      // record which chunk the repo mapping was used in, and ain't nobody got time for that).
-      if (!isWorkspaceRepo(rule)) {
-        for (Table.Cell<RepositoryName, String, RepositoryName> repoMappings :
-            repoMappingRecorder.recordedEntries().cellSet()) {
-          recordedInputValues.put(
-              new RepoRecordedInput.RecordedRepoMapping(
-                  repoMappings.getRowKey(), repoMappings.getColumnKey()),
-              repoMappings.getValue().getName());
-        }
+      for (Table.Cell<RepositoryName, String, RepositoryName> repoMappings :
+          repoMappingRecorder.recordedEntries().cellSet()) {
+        recordedInputValues.put(
+            new RepoRecordedInput.RecordedRepoMapping(
+                repoMappings.getRowKey(), repoMappings.getColumnKey()),
+            repoMappings.getValue().getName());
       }
 
       env.getListener().post(resolved);

src/test/shell/bazel/starlark_repository_test.sh

@@ -3137,4 +3137,23 @@ EOF
   expect_log "I'm running!"
 }
 
+function test_unexported_rule() {
+  # alas, we still need to support this while WORKSPACE is around...
+  create_new_workspace
+  touch MODULE.bazel
+  touch BUILD
+  cat > r.bzl <<EOF
+def _impl(rctx):
+  rctx.file('BUILD', 'filegroup(name="r")')
+def r():
+  repository_rule(_impl)(name = "r")
+EOF
+  cat > WORKSPACE.bzlmod <<EOF
+load('//:r.bzl', 'r')
+r()
+EOF
+
+  bazel build @r >& $TEST_log || fail "expected bazel to succeed"
+}
+
 run_suite "local repository tests"