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

Add Label.to_display_name() #21120

Closed
wants to merge 5 commits into from
Closed
Changes from 1 commit
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
Next Next commit
Add Label.to_display_form()
fmeum committed Feb 1, 2024
commit 4e071c9cabea2068e6ac0f0c79b9ad5bb5d3fbfb
Original file line number Diff line number Diff line change
@@ -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 =
Original file line number Diff line number Diff line change
@@ -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, possibly without WORKSPACE repos or
* null. This is purely meant to support {@link Label#getDisplayFormForStarlark(StarlarkThread)}.
*/
@Nullable
public abstract RepositoryMapping bestEffortMainRepoMapping();

/** 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 bestEffortMainRepoMapping,
String filename,
ImmutableList<Module> loads,
byte[] bzlTransitiveDigest) {
return new AutoValue_BazelModuleContext(
label, repoMapping, filename, loads, bzlTransitiveDigest);
label, repoMapping, bestEffortMainRepoMapping, filename, loads, bzlTransitiveDigest);
}

public final Label.PackageContext packageContext() {
20 changes: 19 additions & 1 deletion src/main/java/com/google/devtools/build/lib/cmdline/Label.java
Original file line number Diff line number Diff line change
@@ -438,10 +438,28 @@ 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;
}

@StarlarkMethod(
name = "to_display_form",
useStarlarkThread = true,
doc =
"Returns a string representation of this label that is optimized for human readability."
+ " The exact form of the return value is explicitly unspecified and subject to"
+ " change."
+ " The following properties are guaranteed for a <code>Label</code> <code>l</code>:"
+ "<ul>"
+ " <li><code>l.to_display_form()</code> has no repository part if and only if <code>l</code> references the main repository;</li>"
+ " <li><code>Label(l.to_display_form()) == l</code> if the call to <code>Label</code> occurs in the main repository.</li>"
+ "</ul>")
public String getDisplayFormForStarlark(StarlarkThread starlarkThread) throws EvalException {
checkRepoVisibilityForStarlark("to_display_form");
return getDisplayForm(
BazelModuleContext.ofInnermostBzlOrThrow(starlarkThread).bestEffortMainRepoMapping());
}

/**
* Returns a shorthand label string that is suitable for display, i.e. in addition to simplifying
* the repository part, labels of the form {@code [@repo]//foo/bar:bar} are simplified to the
Original file line number Diff line number Diff line change
@@ -24,6 +24,8 @@
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;

/**
@@ -222,7 +224,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;
}

Original file line number Diff line number Diff line change
@@ -247,20 +247,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
Original file line number Diff line number Diff line change
@@ -66,6 +66,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;
@@ -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 =
@@ -840,6 +846,7 @@ private BzlLoadValue computeInternalWithCompiledBzl(
BazelModuleContext.create(
label,
repoMapping,
mainRepoMapping.orElse(null),
prog.getFilename(),
ImmutableList.copyOf(loadMap.values()),
transitiveDigest);
@@ -974,6 +981,34 @@ 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;
// When adding cases for other key types such as WORKSPACE or Bzlmod, make sure to track the
// usages of the repo mapping in persistent caches, such as repository marker files and the
// MODULE.bazel.lock file.
if (key instanceof BzlLoadValue.KeyForBuild) {
repoMappingKey = RepositoryMappingValue.key(RepositoryName.MAIN);
} else if (key instanceof BzlLoadValue.KeyForBuiltins) {
// Using the full main repo mapping here results in a cycle as it depends on WORKSPACE, but
// builtins are injected into WORKSPACE. Fixing this fully would require adding a new key type
// for builtins (transitively) loaded from WORKSPACE.
repoMappingKey = RepositoryMappingValue.KEY_FOR_ROOT_MODULE_WITHOUT_WORKSPACE_REPOS;
} else {
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.
Original file line number Diff line number Diff line change
@@ -78,6 +78,7 @@ private static Object exec(String... lines) throws Exception {
BazelModuleContext.create(
Label.parseCanonicalUnchecked("//test:label"),
RepositoryMapping.ALWAYS_FALLBACK,
/* bestEffortMainRepoMapping= */ null,
"test/label.bzl",
/* loads= */ ImmutableList.of(),
/* bzlTransitiveDigest= */ new byte[0])),
Original file line number Diff line number Diff line change
@@ -2479,4 +2479,37 @@ public void innate_noSuchValue() throws Exception {
"//:repo.bzl does not export a repository_rule called data_repo, yet its use is"
+ " requested at /ws/MODULE.bazel");
}

@Test
public void labelToDisplayForm() throws Exception {
scratch.file(
workspaceRoot.getRelative("MODULE.bazel").getPathString(),
"bazel_dep(name='data_repo', version='1.0')",
"ext = use_extension('//:defs.bzl', 'ext')",
"use_repo(ext, 'foo', 'bar', 'baz')");
scratch.file(
workspaceRoot.getRelative("defs.bzl").getPathString(),
"load('@data_repo//:defs.bzl','data_repo')",
"def _ext_impl(ctx):",
" data_repo(name='foo',data=Label('//:foo').to_display_form())",
" data_repo(name='bar',data=Label('@data_repo//:bar').to_display_form())",
" data_repo(name='baz',data=Label('@@canonical_name//:baz').to_display_form())",
"ext = module_extension(implementation=_ext_impl)");
scratch.file(workspaceRoot.getRelative("BUILD").getPathString());
scratch.file(
workspaceRoot.getRelative("data.bzl").getPathString(),
"load('@foo//:data.bzl', foo_data='data')",
"load('@bar//:data.bzl', bar_data='data')",
"load('@baz//:data.bzl', baz_data='data')",
"data = 'foo:'+foo_data+' bar:'+bar_data+' baz:'+baz_data");

SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl"));
EvaluationResult<BzlLoadValue> result =
evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext);
if (result.hasError()) {
throw result.getError().getException();
}
assertThat(result.get(skyKey).getModule().getGlobal("data"))
.isEqualTo("foo://:foo bar:@@data_repo~1.0//:bar baz:@@canonical_name//:baz");
}
}
Original file line number Diff line number Diff line change
@@ -419,6 +419,12 @@ public void testDisplayForm() throws Exception {
.isEqualTo("@unremapped//:unremapped");
}

@Test
public void testDisplayFormNullMapping() throws Exception {
assertThat(displayFormFor("//foo/bar:bar", null)).isEqualTo("//foo/bar:bar");
assertThat(displayFormFor("@@foo//bar:bar", null)).isEqualTo("@@foo//bar:bar");
}

private static String shorthandDisplayFormFor(
String rawLabel, RepositoryMapping repositoryMapping) throws Exception {
return Label.parseCanonical(rawLabel).getShorthandDisplayForm(repositoryMapping);
Original file line number Diff line number Diff line change
@@ -120,6 +120,7 @@ public void testDisplayFormInMainRepository() throws Exception {
RepositoryMapping.create(
ImmutableMap.of("foo", RepositoryName.create("bar")), RepositoryName.MAIN)))
.isEqualTo("//some/pkg");
assertThat(pkg.getDisplayForm(null)).isEqualTo("//some/pkg");
}

@Test
@@ -139,5 +140,6 @@ public void testDisplayFormInExternalRepository() throws Exception {
ImmutableMap.of("local", RepositoryName.create("other_repo")),
RepositoryName.MAIN)))
.isEqualTo("@@canonical//some/pkg");
assertThat(pkg.getDisplayForm(null)).isEqualTo("@@canonical//some/pkg");
}
}
Original file line number Diff line number Diff line change
@@ -105,4 +105,22 @@ public void testGetDisplayForm() throws Exception {
.getDisplayForm(repositoryMapping))
.isEqualTo("@@[unknown repo 'local' requested from @@owner]");
}

@Test
public void testGetDisplayFormWithNullMapping() throws Exception {
assertThat(RepositoryName.create("").getDisplayForm(null)).isEmpty();
assertThat(RepositoryName.create("canonical").getDisplayForm(null))
.isEqualTo("@@canonical");

assertThat(
RepositoryName.create("")
.toNonVisible(RepositoryName.create("owner"))
.getDisplayForm(null))
.isEqualTo("@@[unknown repo '' requested from @@owner]");
assertThat(
RepositoryName.create("canonical")
.toNonVisible(RepositoryName.create("owner"))
.getDisplayForm(null))
.isEqualTo("@@[unknown repo 'canonical' requested from @@owner]");
}
}
Original file line number Diff line number Diff line change
@@ -5047,6 +5047,7 @@ public void testLabelWithStrictVisibility() throws Exception {
bzlLabel,
RepositoryMapping.create(
ImmutableMap.of("my_module", currentRepo, "dep", otherRepo), currentRepo),
/* bestEffortMainRepoMapping= */ null,
"lib/label.bzl",
/* loads= */ ImmutableList.of(),
/* bzlTransitiveDigest= */ new byte[0]);
Original file line number Diff line number Diff line change
@@ -175,6 +175,7 @@ private Object newModule(ImmutableMap.Builder<String, Object> predeclared) {
return BazelModuleContext.create(
label,
RepositoryMapping.ALWAYS_FALLBACK,
/* bestEffortMainRepoMapping= */ null,
"test/label.bzl",
/* loads= */ ImmutableList.of(),
/* bzlTransitiveDigest= */ new byte[0]);
1,049 changes: 585 additions & 464 deletions src/test/py/bazel/bzlmod/bazel_module_test.py

Large diffs are not rendered by default.