Make repo marker files sensitive to repo mapping changes

Similar to the fix for #20721, we write recorded repo mapping entries into the marker file so that repo fetching is sensitive to any relevant repo mapping changes.

Fixes #20722.

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

@@ -20,6 +20,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.Table;
 import com.google.devtools.build.lib.analysis.BlazeDirectories;
 import com.google.devtools.build.lib.analysis.RuleDefinition;
 import com.google.devtools.build.lib.bazel.repository.RepositoryResolvedEvent;
@@ -240,6 +241,9 @@ private RepositoryDirectoryValue.Builder fetchInternal(
     try (Mutability mu = Mutability.create("Starlark repository")) {
       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);
 
       new BazelStarlarkContext(
               BazelStarlarkContext.Phase.LOADING, // ("fetch")
@@ -328,6 +332,10 @@ private RepositoryDirectoryValue.Builder fetchInternal(
         markerData.put("ENV:" + envKey, clientEnvironment.get(envKey));
       }
 
+      for (Table.Cell<RepositoryName, String, RepositoryName> repoMappings : repoMappingRecorder.recordedEntries().cellSet()) {
+        markerData.put("REPO_MAPPING:" + repoMappings.getRowKey().getName() + "," + repoMappings.getColumnKey(), repoMappings.getValue().getName());
+      }
+
       env.getListener().post(resolved);
     } catch (NeedsSkyframeRestartException e) {
       // A dependency is missing, cleanup and returns null

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

@@ -112,6 +112,10 @@ public StarlarkCallable repositoryRule(
     BazelModuleContext moduleContext = BazelModuleContext.ofInnermostBzlOrThrow(thread);
     builder.setRuleDefinitionEnvironmentLabelAndDigest(
         moduleContext.label(), moduleContext.bzlTransitiveDigest());
+    Label.RepoMappingRecorder repoMappingRecorder = thread.getThreadLocal(Label.RepoMappingRecorder.class);
+    if (repoMappingRecorder != null) {
+      builder.setRuleDefinitionEnvironmentRepoMappingEntries(repoMappingRecorder.recordedEntries());
+    }
     builder.setWorkspaceOnly();
     return new RepositoryRuleFunction(
         builder,

src/main/java/com/google/devtools/build/lib/packages/RuleClass.java

@@ -29,6 +29,7 @@
 import com.google.common.collect.ImmutableList;
 import com.google.common.collect.ImmutableMap;
 import com.google.common.collect.ImmutableSet;
+import com.google.common.collect.ImmutableTable;
 import com.google.common.collect.Interner;
 import com.google.common.collect.Iterables;
 import com.google.common.collect.Lists;
@@ -38,6 +39,7 @@
 import com.google.devtools.build.lib.analysis.config.transitions.TransitionFactory;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
+import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.Attribute.ComputedDefault;
 import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate;
@@ -786,8 +788,9 @@ public String toString() {
         NO_OPTION_REFERENCE;
     /** This field and the next are null iff the rule is native. */
     @Nullable private Label ruleDefinitionEnvironmentLabel;
-
     @Nullable private byte[] ruleDefinitionEnvironmentDigest = null;
+    /** This filed is non-null iff the rule is a Starlark repo rule. */
+    @Nullable private ImmutableTable<RepositoryName, String, RepositoryName> ruleDefinitionEnvironmentRepoMappingEntries;
     private final ConfigurationFragmentPolicy.Builder configurationFragmentPolicy =
         new ConfigurationFragmentPolicy.Builder();
 
@@ -984,6 +987,7 @@ public RuleClass build(String name, String key) {
           optionReferenceFunction,
           ruleDefinitionEnvironmentLabel,
           ruleDefinitionEnvironmentDigest,
+          ruleDefinitionEnvironmentRepoMappingEntries,
           configurationFragmentPolicy.build(),
           supportsConstraintChecking,
           toolchainTypes,
@@ -1410,6 +1414,13 @@ public Label getRuleDefinitionEnvironmentLabel() {
       return this.ruleDefinitionEnvironmentLabel;
     }
 
+    @CanIgnoreReturnValue
+    public Builder setRuleDefinitionEnvironmentRepoMappingEntries(
+        ImmutableTable<RepositoryName, String, RepositoryName> recordedRepoMappingEntries) {
+      this.ruleDefinitionEnvironmentRepoMappingEntries = recordedRepoMappingEntries;
+      return this;
+    }
+
     /**
      * Removes an attribute with the same name from this rule class.
      *
@@ -1762,8 +1773,8 @@ public Attribute.Builder<?> copy(String name) {
    * Starlark executable RuleClasses.
    */
   @Nullable private final Label ruleDefinitionEnvironmentLabel;
-
   @Nullable private final byte[] ruleDefinitionEnvironmentDigest;
+  @Nullable private final ImmutableTable<RepositoryName, String, RepositoryName> ruleDefinitionEnvironmentRepoMappingEntries;
   private final OutputFile.Kind outputFileKind;
 
   /**
@@ -1835,6 +1846,7 @@ public Attribute.Builder<?> copy(String name) {
       Function<? super Rule, ? extends Set<String>> optionReferenceFunction,
       @Nullable Label ruleDefinitionEnvironmentLabel,
       @Nullable byte[] ruleDefinitionEnvironmentDigest,
+      @Nullable ImmutableTable<RepositoryName, String, RepositoryName> ruleDefinitionEnvironmentRepoMappingEntries,
       ConfigurationFragmentPolicy configurationFragmentPolicy,
       boolean supportsConstraintChecking,
       Set<ToolchainTypeRequirement> toolchainTypes,
@@ -1870,6 +1882,7 @@ public Attribute.Builder<?> copy(String name) {
     this.optionReferenceFunction = optionReferenceFunction;
     this.ruleDefinitionEnvironmentLabel = ruleDefinitionEnvironmentLabel;
     this.ruleDefinitionEnvironmentDigest = ruleDefinitionEnvironmentDigest;
+    this.ruleDefinitionEnvironmentRepoMappingEntries = ruleDefinitionEnvironmentRepoMappingEntries;
     this.outputFileKind = outputFileKind;
     this.attributes = attributes;
     this.workspaceOnly = workspaceOnly;
@@ -2635,6 +2648,11 @@ public byte[] getRuleDefinitionEnvironmentDigest() {
     return ruleDefinitionEnvironmentDigest;
   }
 
+  @Nullable
+  public ImmutableTable<RepositoryName, String, RepositoryName> getRuleDefinitionEnvironmentRepoMappingEntries() {
+    return ruleDefinitionEnvironmentRepoMappingEntries;
+  }
+
   /** Returns true if this RuleClass is a Starlark-defined RuleClass. */
   @Override
   public boolean isStarlark() {

src/main/java/com/google/devtools/build/lib/rules/BUILD

@@ -415,6 +415,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
         "//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
+        "//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
         "//src/main/java/com/google/devtools/build/lib/skyframe:sky_value_dirtiness_checker",
         "//src/main/java/com/google/devtools/build/lib/util",

src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryDelegatorFunction.java

@@ -93,7 +93,7 @@ public final class RepositoryDelegatorFunction implements SkyFunction {
 
   // The marker file version is inject in the rule key digest so the rule key is always different
   // when we decide to update the format.
-  private static final int MARKER_FILE_VERSION = 3;
+  private static final int MARKER_FILE_VERSION = 4;
 
   // Mapping of rule class name to RepositoryFunction.
   private final ImmutableMap<String, RepositoryFunction> handlers;

src/main/java/com/google/devtools/build/lib/rules/repository/RepositoryFunction.java

@@ -15,6 +15,7 @@
 package com.google.devtools.build.lib.rules.repository;
 
 import static com.google.common.base.Preconditions.checkState;
+import static com.google.common.collect.ImmutableSet.toImmutableSet;
 
 import com.google.common.annotations.VisibleForTesting;
 import com.google.common.base.Preconditions;
@@ -40,6 +41,7 @@
 import com.google.devtools.build.lib.skyframe.PackageLookupFunction;
 import com.google.devtools.build.lib.skyframe.PackageLookupValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
+import com.google.devtools.build.lib.skyframe.RepositoryMappingValue;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -50,6 +52,7 @@
 import com.google.devtools.build.skyframe.SkyFunctionException;
 import com.google.devtools.build.skyframe.SkyFunctionException.Transience;
 import com.google.devtools.build.skyframe.SkyKey;
+import com.google.devtools.build.skyframe.SkyframeLookupResult;
 import java.io.IOException;
 import java.util.Collection;
 import java.util.LinkedHashMap;
@@ -205,15 +208,49 @@ private static ImmutableSet<String> getEnviron(Rule rule) {
   public boolean verifyMarkerData(Rule rule, Map<String, String> markerData, Environment env)
       throws InterruptedException {
     return verifyEnvironMarkerData(markerData, env, getEnviron(rule))
-        && verifyMarkerDataForFiles(rule, markerData, env)
-        && verifySemanticsMarkerData(markerData, env);
+        && verifySemanticsMarkerData(markerData, env)
+        && verifyRepoMappingMarkerData(markerData, env)
+        && verifyMarkerDataForFiles(rule, markerData, env);
   }
 
   protected boolean verifySemanticsMarkerData(Map<String, String> markerData, Environment env)
       throws InterruptedException {
     return true;
   }
 
+  private static boolean verifyRepoMappingMarkerData(Map<String, String> markerData,
+      Environment env)
+      throws InterruptedException {
+    ImmutableSet<SkyKey> skyKeys = markerData.keySet().stream()
+        .filter(k -> k.startsWith("REPO_MAPPING:"))
+        // grab the part after the 'REPO_MAPPING:' and before the comma
+        .map(k -> k.substring("REPO_MAPPING:".length(), k.indexOf(',')))
+        .map(k -> RepositoryMappingValue.key(RepositoryName.createUnvalidated(k)))
+        .collect(toImmutableSet());
+    SkyframeLookupResult result = env.getValuesAndExceptions(skyKeys);
+    if (env.valuesMissing()) {
+      return false;
+    }
+    for (Map.Entry<String, String> entry : markerData.entrySet()) {
+      if (!entry.getKey().startsWith("REPO_MAPPING:")) {
+        continue;
+      }
+      int commaIndex = entry.getKey().indexOf(',');
+      RepositoryName fromRepo =
+          RepositoryName.createUnvalidated(
+              entry.getKey().substring("REPO_MAPPING:".length(), commaIndex));
+      String apparentRepoName = entry.getKey().substring(commaIndex + 1);
+      RepositoryMappingValue repoMappingValue = (RepositoryMappingValue) result.get(
+          RepositoryMappingValue.key(fromRepo));
+      if (repoMappingValue == RepositoryMappingValue.NOT_FOUND_VALUE
+          || !RepositoryName.createUnvalidated(entry.getValue())
+          .equals(repoMappingValue.getRepositoryMapping().get(apparentRepoName))) {
+        return false;
+      }
+    }
+    return true;
+  }
+
   private static boolean verifyLabelMarkerData(Rule rule, String key, String value, Environment env)
       throws InterruptedException {
     Preconditions.checkArgument(key.startsWith("FILE:"));

src/test/java/com/google/devtools/build/lib/packages/RuleClassTest.java

@@ -1053,6 +1053,7 @@ private static RuleClass newRuleClass(
         /* optionReferenceFunction= */ RuleClass.NO_OPTION_REFERENCE,
         /* ruleDefinitionEnvironmentLabel= */ null,
         /* ruleDefinitionEnvironmentDigest= */ null,
+        /* ruleDefinitionEnvironmentRepoMappingEntries= */ null,
         new ConfigurationFragmentPolicy.Builder()
             .requiresConfigurationFragments(allowedConfigurationFragments)
             .build(),

src/test/shell/bazel/starlark_repository_test.sh

@@ -2615,4 +2615,93 @@ EOF
   assert_contains 'WORKSPACE' output
 }
 
+function test_repo_mapping_change_in_rule_impl() {
+  # regression test for #20722
+  create_new_workspace
+  cat > MODULE.bazel <<EOF
+r = use_repo_rule("//:r.bzl", "r")
+r(name = "r")
+bazel_dep(name="foo", repo_name="data")
+local_path_override(module_name="foo", path="foo")
+bazel_dep(name="bar")
+local_path_override(module_name="bar", path="bar")
+EOF
+  touch BUILD
+  cat > r.bzl <<EOF
+def _r(rctx):
+  print("I see: " + str(Label("@data")))
+  rctx.file("BUILD", "filegroup(name='r')")
+r=repository_rule(_r)
+EOF
+  mkdir foo
+  cat > foo/MODULE.bazel <<EOF
+module(name="foo")
+EOF
+  mkdir bar
+  cat > bar/MODULE.bazel <<EOF
+module(name="bar")
+EOF
+
+  bazel build @r >& $TEST_log || fail "expected bazel to succeed"
+  expect_log "I see: @@foo~override//:data"
+
+  # So far, so good. Now we make `@data` point to bar instead!
+  cat > MODULE.bazel <<EOF
+r = use_repo_rule("//:r.bzl", "r")
+r(name = "r")
+bazel_dep(name="foo")
+local_path_override(module_name="foo", path="foo")
+bazel_dep(name="bar", repo_name="data")
+local_path_override(module_name="bar", path="bar")
+EOF
+  # for the repo `r`, nothing except the repo mapping has changed.
+  bazel build @r >& $TEST_log || fail "expected bazel to succeed"
+  expect_log "I see: @@bar~override//:data"
+}
+
+function test_repo_mapping_change_in_bzl_init() {
+  # same as above, but tests .bzl init time repo mapping usages
+  create_new_workspace
+  cat > MODULE.bazel <<EOF
+r = use_repo_rule("//:r.bzl", "r")
+r(name = "r")
+bazel_dep(name="foo", repo_name="data")
+local_path_override(module_name="foo", path="foo")
+bazel_dep(name="bar")
+local_path_override(module_name="bar", path="bar")
+EOF
+  touch BUILD
+  cat > r.bzl <<EOF
+CONSTANT = Label("@data")
+def _r(rctx):
+  print("I see: " + str(CONSTANT))
+  rctx.file("BUILD", "filegroup(name='r')")
+r=repository_rule(_r)
+EOF
+  mkdir foo
+  cat > foo/MODULE.bazel <<EOF
+module(name="foo")
+EOF
+  mkdir bar
+  cat > bar/MODULE.bazel <<EOF
+module(name="bar")
+EOF
+
+  bazel build @r >& $TEST_log || fail "expected bazel to succeed"
+  expect_log "I see: @@foo~override//:data"
+
+  # So far, so good. Now we make `@data` point to bar instead!
+  cat > MODULE.bazel <<EOF
+r = use_repo_rule("//:r.bzl", "r")
+r(name = "r")
+bazel_dep(name="foo")
+local_path_override(module_name="foo", path="foo")
+bazel_dep(name="bar", repo_name="data")
+local_path_override(module_name="bar", path="bar")
+EOF
+  # for the repo `r`, nothing except the repo mapping has changed.
+  bazel build @r >& $TEST_log || fail "expected bazel to succeed"
+  expect_log "I see: @@bar~override//:data"
+}
+
 run_suite "local repository tests"