[7.2.0] Let Label#debugPrint emit label strings in display form (#2…

…2460)

This is a reland of 30b95e3 with a
different approach to emitting display form labels that avoids adding a
new `to_display_form()` method to `Label`:
* In action command lines, which are the most frequent use of labels in
rule implementation functions, labels are automatically emitted in
display form since 9d3a8b0.
* In module extensions and repository rules, if labels can be turned
into display form, the inverse of the repository mapping would need to
be tracked in lockfiles and marker files for correct incrementality.
Furthermore, allowing implementation functions to access apparent names
would allow them to "discriminate" against them, thus possibly breaking
the user's capability to map repos at will via `use_repo` and
`repo_name`. Similar to how providers on a target can't be enumerated,
it is thus safer to not provide this information to the implementation
functions directly.

This change thus implements `StarlarkValue#debugPrint` for `Label` to
allow ruleset authors to emit labels in display form in warnings and
error messages while ensuring that Starlark logic doesn't have access to
this information. `print("My message", label)` degrades gracefully with
older Bazel versions (it just prints a canonical label literal) and can
thus be adopted immediately without a need for feature detection.

This requires changing the signature of `StarlarkValue#debugPrint` to
receive the `StarlarkThread` instead of just the `StarlarkSemantics`.
Since `debugPrint` is meant for emitting potentially non-deterministic
information, it is safe to give it access to `StarlarkThread`.

Also improves the Bzlmod cycle reporter so that it prints helpful
information on a cycle encountered in a previous iteration of this PR.

Fixes #20486

RELNOTES: `Label` instances passed to `print` or `fail` as positional
arguments are now formatted with apparent repository names (optimized
for human readability).

Closes #21963.

PiperOrigin-RevId: 635589078
Change-Id: If60fdc632a59f19dad0cb02312690c15a0540c8e

Closes #22136

src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java

@@ -373,13 +373,9 @@ private String getProgressMessageChecked(@Nullable RepositoryMapping mainReposit
   private String replaceProgressMessagePlaceholders(
       String progressMessage, @Nullable RepositoryMapping mainRepositoryMapping) {
     if (progressMessage.contains("%{label}") && owner.getLabel() != null) {
-      String labelString;
-      if (mainRepositoryMapping != null) {
-        labelString = owner.getLabel().getDisplayForm(mainRepositoryMapping);
-      } else {
-        labelString = owner.getLabel().toString();
-      }
-      progressMessage = progressMessage.replace("%{label}", labelString);
+      progressMessage =
+          progressMessage.replace(
+              "%{label}", owner.getLabel().getDisplayForm(mainRepositoryMapping));
     }
     if (progressMessage.contains("%{output}") && getPrimaryOutput() != null) {
       progressMessage =

src/main/java/com/google/devtools/build/lib/analysis/BazelRuleAnalysisThreadContext.java

@@ -14,9 +14,9 @@
 
 package com.google.devtools.build.lib.analysis;
 
+import com.google.devtools.build.lib.cmdline.BazelStarlarkContext;
 import com.google.devtools.build.lib.cmdline.Label;
-import com.google.devtools.build.lib.packages.BazelStarlarkContext;
-import com.google.devtools.build.lib.packages.SymbolGenerator;
+import com.google.devtools.build.lib.cmdline.SymbolGenerator;
 import com.google.errorprone.annotations.CanIgnoreReturnValue;
 import javax.annotation.Nullable;
 import net.starlark.java.eval.EvalException;
@@ -37,7 +37,8 @@ public class BazelRuleAnalysisThreadContext extends BazelStarlarkContext {
    */
   public BazelRuleAnalysisThreadContext(
       SymbolGenerator<?> symbolGenerator, RuleContext ruleContext) {
-    super(Phase.ANALYSIS, symbolGenerator);
+    super(
+        Phase.ANALYSIS, symbolGenerator, ruleContext.getAnalysisEnvironment()::getMainRepoMapping);
     this.ruleContext = ruleContext;
   }
 

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

@@ -82,7 +82,7 @@
 import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException;
 import com.google.devtools.build.lib.packages.StarlarkAspectClass;
 import com.google.devtools.build.lib.packages.StarlarkProviderWrapper;
-import com.google.devtools.build.lib.packages.SymbolGenerator;
+import com.google.devtools.build.lib.cmdline.SymbolGenerator;
 import com.google.devtools.build.lib.packages.Target;
 import com.google.devtools.build.lib.packages.TargetUtils;
 import com.google.devtools.build.lib.packages.Type;

src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java

@@ -31,18 +31,18 @@
 import com.google.devtools.build.lib.analysis.config.CoreOptions.OutputDirectoryNamingScheme;
 import com.google.devtools.build.lib.analysis.config.CoreOptions.OutputPathsMode;
 import com.google.devtools.build.lib.analysis.config.transitions.PatchTransition;
+import com.google.devtools.build.lib.cmdline.BazelStarlarkContext;
+import com.google.devtools.build.lib.cmdline.BazelStarlarkContext.Phase;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.Label.PackageContext;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.events.EventHandler;
-import com.google.devtools.build.lib.packages.BazelStarlarkContext;
-import com.google.devtools.build.lib.packages.BazelStarlarkContext.Phase;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.RuleTransitionData;
 import com.google.devtools.build.lib.packages.StructImpl;
-import com.google.devtools.build.lib.packages.SymbolGenerator;
+import com.google.devtools.build.lib.cmdline.SymbolGenerator;
 import com.google.devtools.build.lib.starlarkbuildapi.config.ConfigurationTransitionApi;
 import com.google.devtools.build.lib.util.RegexFilter;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -556,7 +556,8 @@ public ImmutableMap<String, Map<String, Object>> evaluate(
         // Create a new {@link BazelStarlarkContext} for the new thread. We need to
         // create a new context every time because {@link BazelStarlarkContext}s
         // should be confined to a single thread.
-        new BazelStarlarkContext(Phase.ANALYSIS, dummySymbolGenerator).storeInThread(thread);
+        new BazelStarlarkContext(Phase.ANALYSIS, dummySymbolGenerator, /* mainRepoMapping= */ null)
+            .storeInThread(thread);
 
         result =
             Starlark.fastcall(

src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java

@@ -54,7 +54,7 @@
 import net.starlark.java.eval.Dict;
 import net.starlark.java.eval.Printer;
 import net.starlark.java.eval.Starlark;
-import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
 
 /**
  * A {@link com.google.devtools.build.lib.analysis.ConfiguredTarget} that is produced by a rule.
@@ -249,7 +249,7 @@ public void repr(Printer printer) {
   }
 
   @Override
-  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public void debugPrint(Printer printer, StarlarkThread thread) {
     // Show the names of the provider keys that this target propagates.
     // Provider key names might potentially be *private* information, and thus a comprehensive
     // list of provider keys should not be exposed in any way other than for debug information.

src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java

@@ -72,7 +72,7 @@ public void repr(Printer printer) {
   }
 
   @Override
-  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public void debugPrint(Printer printer, StarlarkThread thread) {
     try {
       printer.append(
           Joiner.on(" ").join(build(/* mainRepoMappingSupplier= */ () -> null).arguments()));

src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java

@@ -52,6 +52,7 @@
 import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule.Descriptor;
 import com.google.devtools.build.lib.analysis.test.TestConfiguration;
 import com.google.devtools.build.lib.cmdline.BazelModuleContext;
+import com.google.devtools.build.lib.cmdline.BazelStarlarkContext;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
@@ -63,7 +64,6 @@
 import com.google.devtools.build.lib.packages.Attribute.StarlarkComputedDefaultTemplate;
 import com.google.devtools.build.lib.packages.AttributeTransitionData;
 import com.google.devtools.build.lib.packages.AttributeValueSource;
-import com.google.devtools.build.lib.packages.BazelStarlarkContext;
 import com.google.devtools.build.lib.packages.BuildSetting;
 import com.google.devtools.build.lib.packages.BuildType;
 import com.google.devtools.build.lib.packages.BuiltinRestriction;

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java

@@ -29,7 +29,7 @@
  * graphs.
  */
 @AutoValue
-abstract class BazelModuleResolutionValue implements SkyValue {
+public abstract class BazelModuleResolutionValue implements SkyValue {
 
   @SerializationConstant
   public static final SkyKey KEY = () -> SkyFunctions.BAZEL_MODULE_RESOLUTION;

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileValue.java

@@ -117,7 +117,7 @@ public static Key key(ModuleKey moduleKey, @Nullable ModuleOverride override) {
   /** {@link SkyKey} for {@link ModuleFileValue} computation. */
   @AutoCodec
   @AutoValue
-  abstract static class Key implements SkyKey {
+  public abstract static class Key implements SkyKey {
     private static final SkyKeyInterner<Key> interner = SkyKey.newInterner();
 
     abstract ModuleKey getModuleKey();

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/NonRegistryOverride.java

@@ -15,6 +15,7 @@
 
 package com.google.devtools.build.lib.bazel.bzlmod;
 
+import com.google.common.collect.ImmutableSet;
 import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule.ResolutionReason;
 
 /**
@@ -25,6 +26,14 @@
  */
 public interface NonRegistryOverride extends ModuleOverride {
 
+  // Starlark rules loaded from bazel_tools that may define Bazel module repositories with
+  // non-registry overrides and thus must be loaded without relying on any other modules or the main
+  // repo mapping.
+  ImmutableSet<String> BOOTSTRAP_RULE_CLASSES =
+      ImmutableSet.of(
+          ArchiveRepoSpecBuilder.HTTP_ARCHIVE_PATH + "%http_archive",
+          GitRepoSpecBuilder.GIT_REPO_PATH + "%git_repository");
+
   /** Returns the {@link RepoSpec} that defines this repository. */
   RepoSpec getRepoSpec();
 

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java

@@ -34,11 +34,14 @@
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
 import com.google.devtools.build.lib.bazel.repository.starlark.StarlarkRepositoryModule.RepositoryRuleFunction;
 import com.google.devtools.build.lib.cmdline.BazelModuleContext;
+import com.google.devtools.build.lib.cmdline.BazelStarlarkContext;
+import com.google.devtools.build.lib.cmdline.BazelStarlarkContext.Phase;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
+import com.google.devtools.build.lib.cmdline.SymbolGenerator;
 import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.packages.NoSuchPackageException;
 import com.google.devtools.build.lib.packages.Rule;
@@ -131,6 +134,12 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     if (starlarkSemantics == null) {
       return null;
     }
+    RepositoryMappingValue mainRepoMappingValue =
+        (RepositoryMappingValue)
+            env.getValue(RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS);
+    if (mainRepoMappingValue == null) {
+      return null;
+    }
 
     ModuleExtensionId extensionId = (ModuleExtensionId) skyKey.argument();
     SingleExtensionUsagesValue usagesValue =
@@ -181,7 +190,12 @@ public SkyValue compute(SkyKey skyKey, Environment env)
     // Run that extension!
     env.getListener().post(ModuleExtensionEvaluationProgress.ongoing(extensionId, "starting"));
     RunModuleExtensionResult moduleExtensionResult =
-        extension.run(env, usagesValue, starlarkSemantics, extensionId);
+        extension.run(
+            env,
+            usagesValue,
+            starlarkSemantics,
+            extensionId,
+            mainRepoMappingValue.getRepositoryMapping());
     if (moduleExtensionResult == null) {
       return null;
     }
@@ -523,7 +537,8 @@ RunModuleExtensionResult run(
         Environment env,
         SingleExtensionUsagesValue usagesValue,
         StarlarkSemantics starlarkSemantics,
-        ModuleExtensionId extensionId)
+        ModuleExtensionId extensionId,
+        RepositoryMapping repositoryMapping)
         throws InterruptedException, SingleExtensionEvalFunctionException;
   }
 
@@ -675,7 +690,8 @@ public RunModuleExtensionResult run(
         Environment env,
         SingleExtensionUsagesValue usagesValue,
         StarlarkSemantics starlarkSemantics,
-        ModuleExtensionId extensionId)
+        ModuleExtensionId extensionId,
+        RepositoryMapping mainRepositoryMapping)
         throws InterruptedException, SingleExtensionEvalFunctionException {
       var generatedRepoSpecs = ImmutableMap.<String, RepoSpec>builderWithExpectedSize(repos.size());
       // Instantiate the repos one by one.
@@ -845,7 +861,8 @@ public RunModuleExtensionResult run(
         Environment env,
         SingleExtensionUsagesValue usagesValue,
         StarlarkSemantics starlarkSemantics,
-        ModuleExtensionId extensionId)
+        ModuleExtensionId extensionId,
+        RepositoryMapping mainRepositoryMapping)
         throws InterruptedException, SingleExtensionEvalFunctionException {
       ModuleExtensionEvalStarlarkThreadContext threadContext =
           new ModuleExtensionEvalStarlarkThreadContext(
@@ -864,6 +881,12 @@ public RunModuleExtensionResult run(
         thread.setPrintHandler(Event.makeDebugPrintHandler(env.getListener()));
         moduleContext = createContext(env, usagesValue, starlarkSemantics, extensionId);
         threadContext.storeInThread(thread);
+        new BazelStarlarkContext(
+                Phase.WORKSPACE,
+                // Doesn't create retained objects.
+                new SymbolGenerator<>(new Object()),
+                () -> mainRepositoryMapping)
+            .storeInThread(thread);
         // This is used by the `Label()` constructor in Starlark, to record any attempts to resolve
         // apparent repo names to canonical repo names. See #20721 for why this is necessary.
         thread.setThreadLocal(Label.RepoMappingRecorder.class, repoMappingRecorder);

src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTag.java

@@ -24,7 +24,7 @@
 import net.starlark.java.annot.StarlarkBuiltin;
 import net.starlark.java.eval.EvalException;
 import net.starlark.java.eval.Printer;
-import net.starlark.java.eval.StarlarkSemantics;
+import net.starlark.java.eval.StarlarkThread;
 import net.starlark.java.eval.Structure;
 import net.starlark.java.spelling.SpellChecker;
 import net.starlark.java.syntax.Location;
@@ -150,7 +150,7 @@ public String getErrorMessageForUnknownField(String field) {
   }
 
   @Override
-  public void debugPrint(Printer printer, StarlarkSemantics semantics) {
+  public void debugPrint(Printer printer, StarlarkThread thread) {
     printer.append(String.format("'%s' tag at %s", tagClassName, location));
   }
 }

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

@@ -46,6 +46,7 @@ java_library(
         "//src/main/java/com/google/devtools/build/lib/skyframe:directory_tree_digest_value",
         "//src/main/java/com/google/devtools/build/lib/skyframe:ignored_package_prefixes_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/starlarkbuildapi/repository",
         "//src/main/java/com/google/devtools/build/lib/util",
         "//src/main/java/com/google/devtools/build/lib/util:string",

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

@@ -23,15 +23,17 @@
 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.bzlmod.NonRegistryOverride;
 import com.google.devtools.build.lib.bazel.repository.RepositoryResolvedEvent;
 import com.google.devtools.build.lib.bazel.repository.downloader.DownloadManager;
+import com.google.devtools.build.lib.cmdline.BazelStarlarkContext;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelConstants;
+import com.google.devtools.build.lib.cmdline.RepositoryMapping;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.packages.BazelStarlarkContext;
 import com.google.devtools.build.lib.packages.Rule;
-import com.google.devtools.build.lib.packages.SymbolGenerator;
+import com.google.devtools.build.lib.cmdline.SymbolGenerator;
 import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
 import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
 import com.google.devtools.build.lib.profiler.Profiler;
@@ -47,7 +49,7 @@
 import com.google.devtools.build.lib.runtime.RepositoryRemoteExecutor;
 import com.google.devtools.build.lib.skyframe.IgnoredPackagePrefixesValue;
 import com.google.devtools.build.lib.skyframe.PrecomputedValue;
-import com.google.devtools.build.lib.util.CPU;
+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;
@@ -243,6 +245,27 @@ private RepositoryDirectoryValue.Builder fetchInternal(
       return null;
     }
 
+    boolean enableBzlmod = starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD);
+    @Nullable RepositoryMapping mainRepoMapping;
+    String ruleClass =
+        rule.getRuleClassObject().getRuleDefinitionEnvironmentLabel().getUnambiguousCanonicalForm()
+            + "%"
+            + rule.getRuleClass();
+    if (NonRegistryOverride.BOOTSTRAP_RULE_CLASSES.contains(ruleClass)) {
+      // Avoid a cycle.
+      mainRepoMapping = null;
+    } else if (enableBzlmod || !isWorkspaceRepo(rule)) {
+      var mainRepoMappingValue =
+          (RepositoryMappingValue)
+              env.getValue(RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS);
+      if (mainRepoMappingValue == null) {
+        return null;
+      }
+      mainRepoMapping = mainRepoMappingValue.getRepositoryMapping();
+    } else {
+      mainRepoMapping = rule.getPackage().getRepositoryMapping();
+    }
+
     IgnoredPackagePrefixesValue ignoredPackagesValue =
         (IgnoredPackagePrefixesValue) env.getValue(IgnoredPackagePrefixesValue.key());
     if (env.valuesMissing()) {
@@ -265,7 +288,8 @@ private RepositoryDirectoryValue.Builder fetchInternal(
 
       new BazelStarlarkContext(
               BazelStarlarkContext.Phase.LOADING, // ("fetch")
-              new SymbolGenerator<>(key))
+              new SymbolGenerator<>(key),
+              () -> mainRepoMapping)
           .storeInThread(thread);
 
       StarlarkRepositoryContext starlarkRepositoryContext =

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

@@ -28,12 +28,12 @@
 import com.google.devtools.build.lib.bazel.bzlmod.ModuleExtensionEvalStarlarkThreadContext;
 import com.google.devtools.build.lib.bazel.bzlmod.TagClass;
 import com.google.devtools.build.lib.cmdline.BazelModuleContext;
+import com.google.devtools.build.lib.cmdline.BazelStarlarkContext;
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.packages.Attribute;
 import com.google.devtools.build.lib.packages.AttributeValueSource;
-import com.google.devtools.build.lib.packages.BazelStarlarkContext;
 import com.google.devtools.build.lib.packages.BzlInitThreadContext;
 import com.google.devtools.build.lib.packages.Package;
 import com.google.devtools.build.lib.packages.Package.NameConflictException;

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

@@ -17,6 +17,7 @@ java_library(
     srcs = [
         "BazelCompileContext.java",
         "BazelModuleContext.java",
+        "BazelStarlarkContext.java",
         "Label.java",
         "LabelConstants.java",
         "LabelParser.java",
@@ -26,6 +27,7 @@ java_library(
         "RepositoryName.java",
         "ResolvedTargets.java",
         "SignedTargetPattern.java",
+        "SymbolGenerator.java",
         "TargetParsingException.java",
         "TargetPattern.java",
         "TargetPatternResolver.java",