[7.1.0] Do not record any repo mapping entries in the RepoMappingRecorder for WORKSPACE repo rules (#21457)
Follow-up to
https://github.com/bazelbuild/bazel/commit/1cbf09dd45a4bb4cc73ace58ae344bb90e709752;
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 https://github.com/bazelbuild/bazel/issues/21451
Commit
https://github.com/bazelbuild/bazel/commit/e84d0533f93889772820084627659a5324e1942c
PiperOrigin-RevId: 609010175
Change-Id: I90eb921b09068f327b42886ea0a1875374a94049
Co-authored-by: Googler <wyv@google.com>
diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
index ba2bb1b..3f98b7b 100644
--- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
+++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryFunction.java
@@ -231,9 +231,14 @@
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")
@@ -321,17 +326,12 @@
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);
diff --git a/src/test/shell/bazel/starlark_repository_test.sh b/src/test/shell/bazel/starlark_repository_test.sh
index f042329..b3feef4 100755
--- a/src/test/shell/bazel/starlark_repository_test.sh
+++ b/src/test/shell/bazel/starlark_repository_test.sh
@@ -3137,4 +3137,23 @@
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"
