From b1fb70e11fb01504af814e15c2f6db426ff75450 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Tue, 21 May 2024 19:56:15 +0200 Subject: [PATCH] [7.2.0] Let `Label#debugPrint` emit label strings in display form (#22460) This is a reland of 30b95e337bdd871e9f9616f993b4670f5d827da6 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 9d3a8b02b95ed9b51e5d8902463df2b261cd8d15. * 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 --- .../build/lib/actions/AbstractAction.java | 10 +- .../BazelRuleAnalysisThreadContext.java | 7 +- .../build/lib/analysis/RuleContext.java | 2 +- .../StarlarkDefinedConfigTransition.java | 9 +- .../RuleConfiguredTarget.java | 4 +- .../build/lib/analysis/starlark/Args.java | 2 +- .../starlark/StarlarkRuleClassFunctions.java | 2 +- .../bzlmod/BazelModuleResolutionValue.java | 2 +- .../lib/bazel/bzlmod/ModuleFileValue.java | 2 +- .../lib/bazel/bzlmod/NonRegistryOverride.java | 9 ++ .../bzlmod/SingleExtensionEvalFunction.java | 31 ++++++- .../lib/bazel/bzlmod/TypeCheckedTag.java | 4 +- .../build/lib/bazel/repository/starlark/BUILD | 1 + .../starlark/StarlarkRepositoryFunction.java | 32 ++++++- .../starlark/StarlarkRepositoryModule.java | 2 +- .../google/devtools/build/lib/cmdline/BUILD | 2 + .../BazelStarlarkContext.java | 22 ++++- .../devtools/build/lib/cmdline/Label.java | 25 ++++- .../build/lib/cmdline/PackageIdentifier.java | 4 +- .../build/lib/cmdline/RepositoryName.java | 11 ++- .../SymbolGenerator.java | 2 +- .../build/lib/packages/BuildGlobals.java | 1 + .../lib/packages/BzlInitThreadContext.java | 10 +- .../build/lib/packages/PackageFactory.java | 19 +++- .../build/lib/packages/RuleFactory.java | 1 + .../lib/packages/StarlarkCallbackHelper.java | 5 +- .../lib/packages/StarlarkNativeModule.java | 1 + .../lib/packages/TargetDefinitionContext.java | 0 .../build/lib/packages/WorkspaceFactory.java | 6 +- .../lib/rules/android/AndroidCommon.java | 2 +- .../build/lib/rules/cpp/CcLinkingContext.java | 14 +-- .../build/lib/rules/cpp/CcLinkingHelper.java | 2 +- .../build/lib/rules/cpp/CcModule.java | 2 +- .../cpp/FeatureConfigurationForStarlark.java | 3 +- .../build/lib/rules/cpp/LibraryToLink.java | 3 +- .../google/devtools/build/lib/skyframe/BUILD | 1 + .../build/lib/skyframe/BzlLoadFunction.java | 36 +++++++- .../lib/skyframe/BzlmodRepoCycleReporter.java | 28 +++++- .../lib/skyframe/BzlmodRepoRuleFunction.java | 13 +-- .../build/lib/skyframe/PackageFunction.java | 4 +- .../net/starlark/java/eval/MethodLibrary.java | 6 +- .../java/net/starlark/java/eval/Printer.java | 6 +- .../net/starlark/java/eval/StarlarkValue.java | 4 +- .../devtools/build/lib/cmdline/LabelTest.java | 6 ++ .../lib/cmdline/PackageIdentifierTest.java | 2 + .../build/lib/cmdline/RepositoryNameTest.java | 17 ++++ .../lib/starlark/StarlarkIntegrationTest.java | 2 +- ...arlarkRuleImplementationFunctionsTest.java | 7 +- .../util/BazelEvaluationTestCase.java | 5 +- .../java/eval/StarlarkEvaluationTest.java | 14 +-- src/test/py/bazel/bzlmod/bazel_module_test.py | 92 +++++++++++++++++++ src/test/shell/bazel/platforms_test.sh | 2 +- .../shell/bazel/starlark_repository_test.sh | 41 +++++++++ 53 files changed, 437 insertions(+), 103 deletions(-) rename src/main/java/com/google/devtools/build/lib/{packages => cmdline}/BazelStarlarkContext.java (90%) rename src/main/java/com/google/devtools/build/lib/{packages => cmdline}/SymbolGenerator.java (98%) create mode 100644 src/main/java/com/google/devtools/build/lib/packages/TargetDefinitionContext.java diff --git a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java b/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java index 25536393042ef4..06abef0486f8f8 100644 --- a/src/main/java/com/google/devtools/build/lib/actions/AbstractAction.java +++ b/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 = diff --git a/src/main/java/com/google/devtools/build/lib/analysis/BazelRuleAnalysisThreadContext.java b/src/main/java/com/google/devtools/build/lib/analysis/BazelRuleAnalysisThreadContext.java index abf90641c9e00c..c2b0b9e1800191 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/BazelRuleAnalysisThreadContext.java +++ b/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; } diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java index d20c04c48de6fd..0f5f804fa62def 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java +++ b/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; diff --git a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java b/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java index 482f3145f162b7..a23d05686722c7 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/config/StarlarkDefinedConfigTransition.java +++ b/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> 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( diff --git a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java b/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java index d78f147c411862..43b9762492d263 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/configuredtargets/RuleConfiguredTarget.java +++ b/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. diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java index fd2395a1482d1f..7eca38baaff38a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/Args.java +++ b/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())); diff --git a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java b/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java index 2bd751015fdeae..b950d60a4c4a6a 100644 --- a/src/main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java +++ b/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; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java index 5b1d45e5abd450..3dc6849db48f4b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java +++ b/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; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileValue.java index da64efb9ff5e97..c6a8d08bea9ad1 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileValue.java +++ b/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 interner = SkyKey.newInterner(); abstract ModuleKey getModuleKey(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/NonRegistryOverride.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/NonRegistryOverride.java index 8ec22314e9d0c0..a5464d828a2e50 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/NonRegistryOverride.java +++ b/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 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(); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index 61bb7b7b081024..46184ec7837fc3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/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.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); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTag.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTag.java index b9601be57ff8b9..e6d5c4eb21d16f 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/TypeCheckedTag.java +++ b/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)); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD index 880fb396010ae8..87638f45b0b801 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/BUILD +++ b/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", 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 6a59243c063ccd..ba5a1684054ebd 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 @@ -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 = diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java index 1d5d897456d88e..1ee487b99a9d12 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java +++ b/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; diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/BUILD b/src/main/java/com/google/devtools/build/lib/cmdline/BUILD index d8f88e66cec312..c7da581a407f02 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/BUILD +++ b/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", diff --git a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java b/src/main/java/com/google/devtools/build/lib/cmdline/BazelStarlarkContext.java similarity index 90% rename from src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java rename to src/main/java/com/google/devtools/build/lib/cmdline/BazelStarlarkContext.java index 2246e53c45350b..9543684f7aa68b 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BazelStarlarkContext.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/BazelStarlarkContext.java @@ -12,10 +12,11 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.packages; - +package com.google.devtools.build.lib.cmdline; import com.google.common.base.Preconditions; +import com.google.devtools.build.lib.supplier.InterruptibleSupplier; +import javax.annotation.Nullable; import net.starlark.java.eval.EvalException; import net.starlark.java.eval.Starlark; import net.starlark.java.eval.StarlarkThread; @@ -103,6 +104,8 @@ public void storeInThread(StarlarkThread thread) { // TODO(b/236456122): Eliminate Phase, migrate analysisRuleLabel to a separate context class. private final Phase phase; + @Nullable private final InterruptibleSupplier mainRepoMappingSupplier; + /** * @param phase the phase to which this Starlark thread belongs * @param symbolGenerator a {@link SymbolGenerator} to be used when creating objects to be @@ -110,9 +113,13 @@ public void storeInThread(StarlarkThread thread) { */ // TODO(b/236456122): Consider taking an owner in place of a SymbolGenerator, and constructing // the latter ourselves. Seems like we don't want to tempt anyone into sharing a SymbolGenerator. - public BazelStarlarkContext(Phase phase, SymbolGenerator symbolGenerator) { + public BazelStarlarkContext( + Phase phase, + SymbolGenerator symbolGenerator, + @Nullable InterruptibleSupplier mainRepoMappingSupplier) { this.phase = Preconditions.checkNotNull(phase); this.symbolGenerator = Preconditions.checkNotNull(symbolGenerator); + this.mainRepoMappingSupplier = mainRepoMappingSupplier; } /** Returns the phase associated with this context. */ @@ -124,6 +131,15 @@ public SymbolGenerator getSymbolGenerator() { return symbolGenerator; } + /** + * The repository mapping applicable to the main repository. This is purely meant to support + * {@link Label#debugPrint}. + */ + @Nullable + public RepositoryMapping getMainRepoMapping() throws InterruptedException { + return mainRepoMappingSupplier == null ? null : mainRepoMappingSupplier.get(); + } + @Override public String getContextForUncheckedException() { return phase.toString(); diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java index a2421a278eeb04..2cafcfbdd0034a 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/Label.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/Label.java @@ -64,10 +64,14 @@ name = "Label", category = DocCategory.BUILTIN, doc = - "A BUILD target identifier." - + "

For every Label instance l, the string representation" - + " str(l) has the property that Label(str(l)) == l," - + " regardless of where the Label() call occurs.") + "A BUILD target identifier.

For every Label instance l, the" + + " string representation str(l) has the property that Label(str(l))" + + " == l, regardless of where the Label() call occurs.

When" + + " passed as positional arguments to print() or fail()," + + " Label use a string representation optimized for human readability" + + " instead. This representation uses an apparent repository name from" + + " the perspective of the main repository if possible.") @AutoCodec @Immutable @ThreadSafe @@ -437,7 +441,7 @@ public String getUnambiguousCanonicalForm() { * @param mainRepositoryMapping the {@link RepositoryMapping} of the main repository * @return analogous to {@link PackageIdentifier#getDisplayForm(RepositoryMapping)} */ - public String getDisplayForm(RepositoryMapping mainRepositoryMapping) { + public String getDisplayForm(@Nullable RepositoryMapping mainRepositoryMapping) { return packageIdentifier.getDisplayForm(mainRepositoryMapping) + ":" + name; } @@ -650,6 +654,17 @@ public void repr(Printer printer) { printer.append(")"); } + @Override + public void debugPrint(Printer printer, StarlarkThread thread) { + RepositoryMapping mainRepoMapping; + try { + mainRepoMapping = BazelStarlarkContext.fromOrFail(thread).getMainRepoMapping(); + } catch (EvalException | InterruptedException e) { + mainRepoMapping = null; + } + printer.append(getDisplayForm(mainRepoMapping)); + } + @Override public void str(Printer printer, StarlarkSemantics semantics) { if (getRepository().isMain() diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java index a848ba5b560a6e..6e814e1f357188 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/PackageIdentifier.java @@ -22,6 +22,8 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.skyframe.SkyFunctionName; import com.google.devtools.build.skyframe.SkyKey; +import com.google.devtools.build.skyframe.SkyKey.SkyKeyInterner; +import javax.annotation.Nullable; import javax.annotation.concurrent.Immutable; /** @@ -215,7 +217,7 @@ public String getUnambiguousCanonicalForm() { *

only with Bzlmod if the current package belongs to a repository that is not visible * from the main module */ - public String getDisplayForm(RepositoryMapping mainRepositoryMapping) { + public String getDisplayForm(@Nullable RepositoryMapping mainRepositoryMapping) { return repository.getDisplayForm(mainRepositoryMapping) + "//" + pkgName; } diff --git a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java index d0b064512dd98d..984ba4ece2ac75 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/RepositoryName.java @@ -247,13 +247,15 @@ public String getCanonicalForm() { *
@protobuf *
if this repository is a WORKSPACE dependency and its name is "protobuf", * or if this repository is a Bzlmod dependency of the main module and its apparent name - * is "protobuf" + * is "protobuf" (in both cases only if mainRepositoryMapping is not null) *
@@protobuf~3.19.2 *
only with Bzlmod, if this a repository that is not visible from the main module */ - public String getDisplayForm(RepositoryMapping mainRepositoryMapping) { + public String getDisplayForm(@Nullable RepositoryMapping mainRepositoryMapping) { Preconditions.checkArgument( - mainRepositoryMapping.ownerRepo() == null || mainRepositoryMapping.ownerRepo().isMain()); + mainRepositoryMapping == null + || mainRepositoryMapping.ownerRepo() == null + || mainRepositoryMapping.ownerRepo().isMain()); if (!isVisible()) { return getNameWithAt(); } @@ -261,6 +263,9 @@ public String getDisplayForm(RepositoryMapping mainRepositoryMapping) { // Packages in the main repository can always use repo-relative form. return ""; } + if (mainRepositoryMapping == null) { + return getNameWithAt(); + } if (!mainRepositoryMapping.usesStrictDeps()) { // If the main repository mapping is not using strict visibility, then Bzlmod is certainly // disabled, which means that canonical and apparent names can be used interchangeably from diff --git a/src/main/java/com/google/devtools/build/lib/packages/SymbolGenerator.java b/src/main/java/com/google/devtools/build/lib/cmdline/SymbolGenerator.java similarity index 98% rename from src/main/java/com/google/devtools/build/lib/packages/SymbolGenerator.java rename to src/main/java/com/google/devtools/build/lib/cmdline/SymbolGenerator.java index eaacceb6e5efbf..37e46452d77586 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/SymbolGenerator.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/SymbolGenerator.java @@ -12,7 +12,7 @@ // See the License for the specific language governing permissions and // limitations under the License. -package com.google.devtools.build.lib.packages; +package com.google.devtools.build.lib.cmdline; import com.google.devtools.build.lib.concurrent.ThreadSafety; diff --git a/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java b/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java index 4fbad576ca6f8b..8b55b1df77681d 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BuildGlobals.java @@ -15,6 +15,7 @@ import com.google.devtools.build.docgen.annot.GlobalMethods; import com.google.devtools.build.docgen.annot.GlobalMethods.Environment; +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.packages.License.DistributionType; diff --git a/src/main/java/com/google/devtools/build/lib/packages/BzlInitThreadContext.java b/src/main/java/com/google/devtools/build/lib/packages/BzlInitThreadContext.java index 65b62931b41356..8ff9b094b409c3 100644 --- a/src/main/java/com/google/devtools/build/lib/packages/BzlInitThreadContext.java +++ b/src/main/java/com/google/devtools/build/lib/packages/BzlInitThreadContext.java @@ -16,8 +16,11 @@ import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.RuleDefinitionEnvironment; +import com.google.devtools.build.lib.cmdline.BazelStarlarkContext; import com.google.devtools.build.lib.cmdline.Label; +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.errorprone.annotations.CanIgnoreReturnValue; import java.util.Optional; import javax.annotation.Nullable; @@ -57,7 +60,7 @@ public final class BzlInitThreadContext extends BazelStarlarkContext * @param networkAllowlistForTests an allowlist for rule classes created by this thread * @param fragmentNameToClass a map from configuration fragment name to configuration fragment * class, such as "apple" to AppleConfiguration.class - * @param symbolGenerator symbol generator for this context + * @param mainRepoMapping the repository mapping of the main repository */ public BzlInitThreadContext( Label bzlFile, @@ -65,8 +68,9 @@ public BzlInitThreadContext( RepositoryName toolsRepository, Optional