From 6586341e4f549de2e1458d96d983c807ef51e1f2 Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 10 Apr 2024 22:35:00 +0200 Subject: [PATCH] Give `StarlarkValue#debugPrint` access to the `StarlarkThread` Since `debugPrint` is meant for emitting potentially non-deterministic, it is safe to give it access to `StarlarkThread` information. This will be used in a follow-up to let `print` and `fail` emit `Label`s in display form without having to track the reverse repo mapping for the sake of invalidation of repo rules or module extensions. Work towards #20486 --- .../build/lib/actions/AbstractAction.java | 10 +--- .../RuleConfiguredTarget.java | 4 +- .../build/lib/analysis/starlark/Args.java | 2 +- .../bzlmod/BazelModuleResolutionValue.java | 2 +- .../lib/bazel/bzlmod/ModuleFileValue.java | 2 +- .../lib/bazel/bzlmod/TypeCheckedTag.java | 4 +- .../build/lib/cmdline/BazelModuleContext.java | 10 +++- .../devtools/build/lib/cmdline/Label.java | 13 +++- .../build/lib/cmdline/PackageIdentifier.java | 3 +- .../build/lib/cmdline/RepositoryName.java | 11 +++- .../build/lib/rules/cpp/CcLinkingContext.java | 12 ++-- .../cpp/FeatureConfigurationForStarlark.java | 2 +- .../build/lib/rules/cpp/LibraryToLink.java | 3 +- .../google/devtools/build/lib/skyframe/BUILD | 1 + .../build/lib/skyframe/BzlLoadFunction.java | 34 +++++++++++ .../lib/skyframe/BzlmodRepoCycleReporter.java | 28 ++++++++- .../net/starlark/java/eval/MethodLibrary.java | 6 +- .../java/net/starlark/java/eval/Printer.java | 6 +- .../net/starlark/java/eval/StarlarkValue.java | 4 +- .../actions/BuildInfoFileWriteActionTest.java | 1 + .../devtools/build/lib/cmdline/LabelTest.java | 6 ++ .../lib/cmdline/PackageIdentifierTest.java | 2 + .../build/lib/cmdline/RepositoryNameTest.java | 17 ++++++ .../lib/starlark/StarlarkIntegrationTest.java | 2 +- .../StarlarkRuleClassFunctionsTest.java | 1 + ...arlarkRuleImplementationFunctionsTest.java | 9 ++- .../util/BazelEvaluationTestCase.java | 1 + .../java/eval/StarlarkEvaluationTest.java | 14 +++-- src/test/py/bazel/bzlmod/bazel_module_test.py | 59 +++++++++++++++++++ src/test/shell/bazel/platforms_test.sh | 2 +- 30 files changed, 222 insertions(+), 49 deletions(-) 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 60d2ccef5896a6..8694278b959743 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 @@ -368,13 +368,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/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 cade026c9edd4b..b451eb528cddc9 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 @@ -71,7 +71,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/bazel/bzlmod/BazelModuleResolutionValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionValue.java index 19c642cfa5d7f7..ca3794479605fe 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 @@ -27,7 +27,7 @@ * graphs. */ @AutoValue -abstract class BazelModuleResolutionValue implements SkyValue { +public abstract class BazelModuleResolutionValue implements SkyValue { /* TODO(andreisolo): Also load the modules overridden by {@code single_version_override} or NonRegistryOverride if we need to detect changes in the dependency graph caused by them. */ 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 7c7e38fd6c3db6..4a9d4653cdc92b 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 @@ -99,7 +99,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/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/cmdline/BazelModuleContext.java b/src/main/java/com/google/devtools/build/lib/cmdline/BazelModuleContext.java index 9bd7dd45b30ef7..bb00dc332ef7e7 100644 --- a/src/main/java/com/google/devtools/build/lib/cmdline/BazelModuleContext.java +++ b/src/main/java/com/google/devtools/build/lib/cmdline/BazelModuleContext.java @@ -44,6 +44,13 @@ public abstract class BazelModuleContext { /** The repository mapping applicable to the repo where the .bzl file is located in. */ public abstract RepositoryMapping repoMapping(); + /** + * The repository mapping applicable to the main repository. This is purely meant to support + * {@link Label#debugPrint}. + */ + @Nullable + public abstract RepositoryMapping mainRepoMapping(); + /** Returns the name of the module's .bzl file, as provided to the parser. */ public abstract String filename(); @@ -160,11 +167,12 @@ public static BazelModuleContext ofInnermostBzlOrFail(StarlarkThread thread, Str public static BazelModuleContext create( Label label, RepositoryMapping repoMapping, + @Nullable RepositoryMapping mainRepoMapping, String filename, ImmutableList loads, byte[] bzlTransitiveDigest) { return new AutoValue_BazelModuleContext( - label, repoMapping, filename, loads, bzlTransitiveDigest); + label, repoMapping, mainRepoMapping, filename, loads, bzlTransitiveDigest); } public final Label.PackageContext packageContext() { 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 3d2443802e1ff4..962f468888e8ce 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 @@ -66,7 +66,11 @@ "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.") + + " 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.") @Immutable @ThreadSafe public final class Label implements Comparable

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 3747008f2cc58a..d7435f7fb68251 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 @@ -249,13 +249,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(); } @@ -263,6 +265,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/rules/cpp/CcLinkingContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java index 1794531b75b4d8..7ffeaddebec14f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcLinkingContext.java @@ -276,23 +276,23 @@ public Sequence getLinkstampsForStarlark() { } @Override - public void debugPrint(Printer printer, StarlarkSemantics semantics) { + public void debugPrint(Printer printer, StarlarkThread thread) { printer.append(""); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java index d567e577eea472..485fd1c0b5f277 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FeatureConfigurationForStarlark.java @@ -62,7 +62,7 @@ public void str(Printer printer, StarlarkSemantics semantics) { } @Override - public void debugPrint(Printer printer, StarlarkSemantics semantics) { + public void debugPrint(Printer printer, StarlarkThread thread) { printer.append(""); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java index d97ebd2981a464..9cfa09fb5df786 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LibraryToLink.java @@ -31,7 +31,6 @@ import net.starlark.java.eval.Printer; import net.starlark.java.eval.Sequence; import net.starlark.java.eval.StarlarkList; -import net.starlark.java.eval.StarlarkSemantics; import net.starlark.java.eval.StarlarkThread; /** Encapsulates information for linking a library. */ @@ -224,7 +223,7 @@ LinkerInputs.LibraryToLink getInterfaceLibraryToLink() { abstract boolean getDisableWholeArchive(); @Override - public final void debugPrint(Printer printer, StarlarkSemantics semantics) { + public final void debugPrint(Printer printer, StarlarkThread thread) { printer.append(" mainRepoMapping = + getMainRepositoryMapping(key, builtins.starlarkSemantics, env); + if (mainRepoMapping == null) { + return null; + } Label.RepoMappingRecorder repoMappingRecorder = new Label.RepoMappingRecorder(); ImmutableList> programLoads = getLoadsFromProgram(prog); ImmutableList