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.

Sign up for GitHub

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

[7.1.0] Cover missing cases during module extension label normalization #20630

Merged
merged 3 commits into from
Jan 9, 2024
Merged

[7.1.0] Cover missing cases during module extension label normalization #20630

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
3 commits
Select commit Hold shift + click to select a range
2153048
Cover missing cases during module extension label normalization
fmeum Dec 20, 2023
a6a3674
Merge branch 'release-7.1.0' into cp20482-7.1.0
iancha1992 Dec 21, 2023
1ce0867
Merge branch 'release-7.1.0' into cp20482-7.1.0
Wyverald Jan 4, 2024
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
36 changes: 27 additions & 9 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -29,9 +29,13 @@
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileGlobals.ModuleExtensionUsageBuilder.ModuleExtensionProxy;
import com.google.devtools.build.lib.bazel.bzlmod.Version.ParseException;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.StarlarkExportable;
import com.google.devtools.build.lib.vfs.PathFragment;
import java.util.ArrayList;
import java.util.HashMap;
import java.util.LinkedHashMap;
Expand Down Expand Up @@ -480,17 +484,31 @@ public ModuleExtensionProxy useExtension(
}

private String normalizeLabelString(String rawExtensionBzlFile) {
// Normalize the label by adding the current module's repo_name if the label doesn't specify a
// repository name. This is necessary as ModuleExtensionUsages are grouped by the string value
// of this label, but later mapped to their Label representation. If multiple strings map to the
// same Label, this would result in a crash.
// Normalize the label by parsing and stringifying it with a repo mapping that preserves the
// apparent repository name, except that a reference to the main repository via the empty
// repo name is translated to using the module repo name. This is necessary as
// ModuleExtensionUsages are grouped by the string value of this label, but later mapped to
// their Label representation. If multiple strings map to the same Label, this would result in a
// crash.
// ownName can't change anymore as calling module() after this results in an error.
String ownName = module.getRepoName().orElse(module.getName());
if (module.getKey().equals(ModuleKey.ROOT) && rawExtensionBzlFile.startsWith("@//")) {
return "@" + ownName + rawExtensionBzlFile.substring(1);
} else if (rawExtensionBzlFile.startsWith("//")) {
return "@" + ownName + rawExtensionBzlFile;
} else {
RepositoryName ownRepoName = RepositoryName.createUnvalidated(ownName);
try {
ImmutableMap<String, RepositoryName> repoMapping = ImmutableMap.of();
if (module.getKey().equals(ModuleKey.ROOT)) {
repoMapping = ImmutableMap.of("", ownRepoName);
}
Label label =
Label.parseWithPackageContext(
rawExtensionBzlFile,
Label.PackageContext.of(
PackageIdentifier.create(ownRepoName, PathFragment.EMPTY_FRAGMENT),
RepositoryMapping.createAllowingFallback(repoMapping)));
// Skip over the leading "@" of the unambiguous form.
return label.getUnambiguousCanonicalForm().substring(1);
} catch (LabelSyntaxException ignored) {
// Preserve backwards compatibility by not failing eagerly, rather keep the invalid label and
// let the extension fail when evaluated.
return rawExtensionBzlFile;
}
}
Expand Down
11 changes: 8 additions & 3 deletions src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -363,7 +363,10 @@ public void simpleExtension_nonCanonicalLabel() throws Exception {
"use_repo(ext2, 'bar')",
"ext3 = use_extension('@//:defs.bzl', 'ext')",
"ext3.tag(name='quz', data='qu')",
"use_repo(ext3, 'quz')");
"use_repo(ext3, 'quz')",
"ext4 = use_extension('defs.bzl', 'ext')",
"ext4.tag(name='qor', data='qo')",
"use_repo(ext4, 'qor')");
scratch.file(
workspaceRoot.getRelative("defs.bzl").getPathString(),
"load('@data_repo//:defs.bzl','data_repo')",
Expand All @@ -379,15 +382,17 @@ public void simpleExtension_nonCanonicalLabel() throws Exception {
"load('@foo//:data.bzl', foo_data='data')",
"load('@bar//:data.bzl', bar_data='data')",
"load('@quz//:data.bzl', quz_data='data')",
"data = 'foo:'+foo_data+' bar:'+bar_data+' quz:'+quz_data");
"load('@qor//:data.bzl', qor_data='data')",
"data = 'foo:'+foo_data+' bar:'+bar_data+' quz:'+quz_data+' qor:'+qor_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:fu bar:ba quz:qu");
assertThat(result.get(skyKey).getModule().getGlobal("data"))
.isEqualTo("foo:fu bar:ba quz:qu qor:qo");
}

@Test
Expand Down
Loading