Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Let Label#debugPrint emit label strings in display form #21963

Closed
wants to merge 4 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -14,8 +14,8 @@

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.errorprone.annotations.CanIgnoreReturnValue;
import javax.annotation.Nullable;
import net.starlark.java.eval.EvalException;
Expand All @@ -33,7 +33,7 @@ public class BazelRuleAnalysisThreadContext extends BazelStarlarkContext {
* @param ruleContext is the {@link RuleContext} of the rule for analysis of a rule or aspect
*/
public BazelRuleAnalysisThreadContext(RuleContext ruleContext) {
super(Phase.ANALYSIS);
super(Phase.ANALYSIS, ruleContext.getAnalysisEnvironment()::getMainRepoMapping);
this.ruleContext = ruleContext;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@
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.cmdline.BazelStarlarkContext;
import com.google.devtools.build.lib.cmdline.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;
Expand Down Expand Up @@ -552,7 +552,7 @@ 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).storeInThread(thread);
new BazelStarlarkContext(Phase.ANALYSIS, /* mainRepoMapping= */ null).storeInThread(thread);

result =
Starlark.fastcall(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -55,7 +55,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 @@ -254,7 +254,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 @@ -66,7 +66,7 @@
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.cmdline.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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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();

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,8 @@
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;
Expand Down Expand Up @@ -132,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 =
Expand Down Expand Up @@ -182,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;
}
Expand Down Expand Up @@ -524,7 +537,8 @@ RunModuleExtensionResult run(
Environment env,
SingleExtensionUsagesValue usagesValue,
StarlarkSemantics starlarkSemantics,
ModuleExtensionId extensionId)
ModuleExtensionId extensionId,
RepositoryMapping repositoryMapping)
throws InterruptedException, SingleExtensionEvalFunctionException;
}

Expand Down Expand Up @@ -676,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.
Expand Down Expand Up @@ -845,7 +860,8 @@ public RunModuleExtensionResult run(
Environment env,
SingleExtensionUsagesValue usagesValue,
StarlarkSemantics starlarkSemantics,
ModuleExtensionId extensionId)
ModuleExtensionId extensionId,
RepositoryMapping mainRepositoryMapping)
throws InterruptedException, SingleExtensionEvalFunctionException {
ModuleExtensionEvalStarlarkThreadContext threadContext =
new ModuleExtensionEvalStarlarkThreadContext(
Expand All @@ -869,6 +885,8 @@ public RunModuleExtensionResult run(
thread.setPrintHandler(Event.makeDebugPrintHandler(env.getListener()));
moduleContext = createContext(env, usagesValue, starlarkSemantics, extensionId);
threadContext.storeInThread(thread);
new BazelStarlarkContext(Phase.WORKSPACE, () -> 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);
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 @@ -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",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,15 @@
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.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.pkgcache.PathPackageLocator;
Expand All @@ -46,6 +48,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.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;
Expand Down Expand Up @@ -242,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()) {
Expand All @@ -264,7 +288,8 @@ private RepositoryDirectoryValue.Builder fetchInternal(
thread.setThreadLocal(Label.RepoMappingRecorder.class, repoMappingRecorder);
}

new BazelStarlarkContext(BazelStarlarkContext.Phase.LOADING).storeInThread(thread); // "fetch"
new BazelStarlarkContext(BazelStarlarkContext.Phase.LOADING, () -> mainRepoMapping)
.storeInThread(thread); // "fetch"

StarlarkRepositoryContext starlarkRepositoryContext =
new StarlarkRepositoryContext(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@
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.cmdline.BazelStarlarkContext;
import com.google.devtools.build.lib.packages.BzlInitThreadContext;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.RuleClass;
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/cmdline/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@ java_library(
srcs = [
"BazelCompileContext.java",
"BazelModuleContext.java",
"BazelStarlarkContext.java",
"Label.java",
"LabelCodec.java",
"LabelConstants.java",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -11,9 +11,11 @@
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// 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;
Expand Down Expand Up @@ -94,19 +96,31 @@ 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<RepositoryMapping> mainRepoMappingSupplier;

/**
* @param phase the phase to which this Starlark thread belongs
*/
public BazelStarlarkContext(Phase phase) {
public BazelStarlarkContext(
Phase phase, @Nullable InterruptibleSupplier<RepositoryMapping> mainRepoMappingSupplier) {
Wyverald marked this conversation as resolved.
Show resolved Hide resolved
this.phase = Preconditions.checkNotNull(phase);
this.mainRepoMappingSupplier = mainRepoMappingSupplier;
}

/** Returns the phase associated with this context. */
public Phase getPhase() {
return phase;
}

/**
* 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();
Expand Down
19 changes: 17 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,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()
Expand Down
Loading
Loading