Skip to content

Commit

Permalink
Give StarlarkValue#debugPrint access to the StarlarkThread
Browse files Browse the repository at this point in the history
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 bazelbuild#20486
  • Loading branch information
fmeum committed Apr 25, 2024
1 parent 7ffa325 commit 6586341
Show file tree
Hide file tree
Showing 30 changed files with 222 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -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 =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down Expand Up @@ -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.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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()));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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<Key> interner = SkyKey.newInterner();

abstract ModuleKey getModuleKey();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -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();

Expand Down Expand Up @@ -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<Module> loads,
byte[] bzlTransitiveDigest) {
return new AutoValue_BazelModuleContext(
label, repoMapping, filename, loads, bzlTransitiveDigest);
label, repoMapping, mainRepoMapping, filename, loads, bzlTransitiveDigest);
}

public final Label.PackageContext packageContext() {
Expand Down
13 changes: 11 additions & 2 deletions src/main/java/com/google/devtools/build/lib/cmdline/Label.java
Original file line number Diff line number Diff line change
Expand Up @@ -66,7 +66,11 @@
"A BUILD target identifier."
+ "<p>For every <code>Label</code> instance <code>l</code>, the string representation"
+ " <code>str(l)</code> has the property that <code>Label(str(l)) == l</code>,"
+ " regardless of where the <code>Label()</code> call occurs.")
+ " regardless of where the <code>Label()</code> call occurs."
+ "<p>When passed as positional arguments to <code>print()</code> or <code>fail()"
+ "</code>, <code>Label</code> use a string representation optimized for human readability"
+ " instead. This representation uses an <a href=\"/external/overview#apparent-repo-name\">"
+ "apparent repository name</a> from the perspective of the main repository if possible.")
@Immutable
@ThreadSafe
public final class Label implements Comparable<Label>, StarlarkValue, SkyKey, CommandLineItem {
Expand Down Expand Up @@ -442,7 +446,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;
}

Expand Down Expand Up @@ -655,6 +659,11 @@ public void repr(Printer printer) {
printer.append(")");
}

@Override
public void debugPrint(Printer printer, StarlarkThread thread) {
printer.append(getDisplayForm(BazelModuleContext.ofInnermostBzlOrThrow(thread).mainRepoMapping()));
}

@Override
public void str(Printer printer, StarlarkSemantics semantics) {
if (getRepository().isMain()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,6 +24,7 @@
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;

/**
Expand Down Expand Up @@ -226,7 +227,7 @@ public String getUnambiguousCanonicalForm() {
* <dd>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;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -249,20 +249,25 @@ public String getCanonicalForm() {
* <dt><code>@protobuf</code>
* <dd>if this repository is a WORKSPACE dependency and its <code>name</code> 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)
* <dt><code>@@protobuf~3.19.2</code>
* <dd>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();
}
if (isMain()) {
// 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
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -276,23 +276,23 @@ public Sequence<Linkstamp> getLinkstampsForStarlark() {
}

@Override
public void debugPrint(Printer printer, StarlarkSemantics semantics) {
public void debugPrint(Printer printer, StarlarkThread thread) {
printer.append("<LinkerInput(owner=");
if (owner == null) {
printer.append("[null owner, uses old create_linking_context API]");
} else {
owner.debugPrint(printer, semantics);
owner.debugPrint(printer, thread);
}
printer.append(", libraries=[");
for (LibraryToLink libraryToLink : libraries) {
libraryToLink.debugPrint(printer, semantics);
libraryToLink.debugPrint(printer, thread);
printer.append(", ");
}
printer.append("], userLinkFlags=[");
printer.append(Joiner.on(", ").join(userLinkFlags));
printer.append("], nonCodeInputs=[");
for (Artifact nonCodeInput : nonCodeInputs) {
nonCodeInput.debugPrint(printer, semantics);
nonCodeInput.debugPrint(printer, thread);
printer.append(", ");
}
// TODO(cparsons): Add debug repesentation of linkstamps.
Expand Down Expand Up @@ -390,10 +390,10 @@ public String toString() {
@Nullable private final ExtraLinkTimeLibraries extraLinkTimeLibraries;

@Override
public void debugPrint(Printer printer, StarlarkSemantics semantics) {
public void debugPrint(Printer printer, StarlarkThread thread) {
printer.append("<CcLinkingContext([");
for (LinkerInput linkerInput : linkerInputs.toList()) {
linkerInput.debugPrint(printer, semantics);
linkerInput.debugPrint(printer, thread);
printer.append(", ");
}
printer.append("])>");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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("<FeatureConfiguration(");
printer.append(Joiner.on(", ").join(featureConfiguration.getEnabledFeatureNames()));
printer.append(")>");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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. */
Expand Down Expand Up @@ -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("<LibraryToLink(");
printer.append(
Joiner.on(", ")
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2587,6 +2587,7 @@ java_library(
":sky_functions",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value",
"//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/packages",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -65,6 +65,7 @@
import java.util.LinkedHashSet;
import java.util.List;
import java.util.Map;
import java.util.Optional;
import java.util.concurrent.atomic.AtomicBoolean;
import java.util.function.Consumer;
import javax.annotation.Nullable;
Expand Down Expand Up @@ -762,6 +763,11 @@ private BzlLoadValue computeInternalWithCompiledBzl(
if (repoMapping == null) {
return null;
}
Optional<RepositoryMapping> mainRepoMapping =
getMainRepositoryMapping(key, builtins.starlarkSemantics, env);
if (mainRepoMapping == null) {
return null;
}
Label.RepoMappingRecorder repoMappingRecorder = new Label.RepoMappingRecorder();
ImmutableList<Pair<String, Location>> programLoads = getLoadsFromProgram(prog);
ImmutableList<Label> loadLabels =
Expand Down Expand Up @@ -840,6 +846,7 @@ private BzlLoadValue computeInternalWithCompiledBzl(
BazelModuleContext.create(
label,
repoMapping,
mainRepoMapping.orElse(null),
prog.getFilename(),
ImmutableList.copyOf(loadMap.values()),
transitiveDigest);
Expand Down Expand Up @@ -972,6 +979,33 @@ private static RepositoryMapping getRepositoryMapping(
return repositoryMappingValue.getRepositoryMapping();
}

@Nullable
private static Optional<RepositoryMapping> getMainRepositoryMapping(
BzlLoadValue.Key key, StarlarkSemantics starlarkSemantics, Environment env)
throws InterruptedException {
if (!starlarkSemantics.getBool(BuildLanguageOptions.ENABLE_BZLMOD)) {
return Optional.empty();
}
RepositoryMappingValue.Key repoMappingKey;
if (key instanceof BzlLoadValue.KeyForBuild) {
repoMappingKey = RepositoryMappingValue.key(RepositoryName.MAIN);
} else if ((key instanceof BzlLoadValue.KeyForBzlmod
&& !(key instanceof BzlLoadValue.KeyForBzlmodBootstrap))
|| key instanceof BzlLoadValue.KeyForWorkspace) {
// Since the main repo mapping requires evaluating WORKSPACE, but WORKSPACE can load from
// extension repos, requesting the full main repo mapping would cause a cycle.
repoMappingKey = RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS;
} else {
// Builtins and Bzlmod bootstrap repo rules don't need the main repo mapping.
return Optional.empty();
}
var mainRepositoryMappingValue = (RepositoryMappingValue) env.getValue(repoMappingKey);
if (mainRepositoryMappingValue == null) {
return null;
}
return Optional.of(mainRepositoryMappingValue.getRepositoryMapping());
}

/**
* Validates a label appearing in a {@code load()} statement, throwing {@link
* LabelSyntaxException} on failure.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,11 @@
import com.google.common.base.Predicates;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.Iterables;
import com.google.devtools.build.lib.bazel.bzlmod.BazelDepGraphValue;
import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleResolutionValue;
import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleValue;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleExtensionId;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.events.Event;
Expand Down Expand Up @@ -69,6 +72,15 @@ public class BzlmodRepoCycleReporter implements CyclesReporter.SingleCycleReport
private static final Predicate<SkyKey> IS_WORKSPACE_FILE =
SkyFunctions.isSkyFunction(WorkspaceFileValue.WORKSPACE_FILE);

private static final Predicate<SkyKey> IS_MODULE_RESOLUTION =
SkyFunctions.isSkyFunction(SkyFunctions.BAZEL_MODULE_RESOLUTION);

private static final Predicate<SkyKey> IS_DEP_GRAPH =
SkyFunctions.isSkyFunction(SkyFunctions.BAZEL_DEP_GRAPH);

private static final Predicate<SkyKey> IS_MODULE_FILE =
SkyFunctions.isSkyFunction(SkyFunctions.MODULE_FILE);

private static void requestRepoDefinitions(
ExtendedEventHandler eventHandler, Iterable<SkyKey> repos) {
for (SkyKey repo : repos) {
Expand Down Expand Up @@ -113,7 +125,10 @@ public boolean maybeReportCycle(
IS_MODULE_EXTENSION_REPO_MAPPING_ENTRIES,
IS_PACKAGE,
IS_EXTERNAL_PACKAGE,
IS_WORKSPACE_FILE))
IS_WORKSPACE_FILE,
IS_MODULE_RESOLUTION,
IS_DEP_GRAPH,
IS_MODULE_FILE))
&& Iterables.any(cycle, Predicates.or(IS_REPO_RULE, IS_EXTENSION_IMPL))) {
StringBuilder cycleMessage =
new StringBuilder(
Expand All @@ -127,7 +142,10 @@ public boolean maybeReportCycle(
IS_EXTENSION_IMPL,
IS_BZL_LOAD,
IS_REPO_MAPPING,
IS_WORKSPACE_FILE));
IS_WORKSPACE_FILE,
IS_MODULE_RESOLUTION,
IS_DEP_GRAPH,
IS_MODULE_FILE));
Function<Object, String> printer =
rawInput -> {
SkyKey input = (SkyKey) rawInput;
Expand All @@ -144,6 +162,12 @@ public boolean maybeReportCycle(
return String.format("repository mapping of %s", key.repoName());
} else if (input.argument() instanceof WorkspaceFileValue.WorkspaceFileKey) {
return "WORKSPACE file";
} else if (input.argument() == BazelModuleResolutionValue.KEY) {
return "module resolution";
} else if (input.argument() == BazelDepGraphValue.KEY) {
return "module dependency graph";
} else if (input.argument() instanceof ModuleFileValue.Key) {
return "module file of " + input.argument();
} else {
Preconditions.checkArgument(input.argument() instanceof BzlLoadValue.Key);
return ((BzlLoadValue.Key) input.argument()).getLabel().toString();
Expand Down
Loading

0 comments on commit 6586341

Please sign in to comment.