Skip to content

Commit

Permalink
Record repo mapping entries for labels in module extension tags
Browse files Browse the repository at this point in the history
The mapping of an apparent name in a module extension tag's `attr.label`
attribute to the corresponding canonical name has to be recorded in the
lockfile so that instantiated repo rules don't reference the stale
repos.

Fixes bazelbuild#25063
  • Loading branch information
fmeum committed Jan 25, 2025
1 parent e66d61d commit 4b8ba71
Show file tree
Hide file tree
Showing 7 changed files with 60 additions and 11 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ public abstract class BazelLockFileValue implements SkyValue {
// https://cs.opensource.google/bazel/bazel/+/release-7.3.0:src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java;l=120-127;drc=5f5355b75c7c93fba1e15f6658f308953f4baf51
// While this hack exists on 7.x, lockfile version increments should be done 2 at a time (i.e.
// keep this number even).
public static final int LOCK_FILE_VERSION = 16;
public static final int LOCK_FILE_VERSION = 18;

@SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_LOCK_FILE;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -267,7 +267,7 @@ private RunModuleExtensionResult runInternal(
try (Mutability mu =
Mutability.create("module extension", usagesValue.getExtensionUniqueName());
ModuleExtensionContext moduleContext =
createContext(env, usagesValue, starlarkSemantics, extensionId)) {
createContext(env, usagesValue, starlarkSemantics, extensionId, repoMappingRecorder)) {
StarlarkThread thread =
StarlarkThread.create(
mu,
Expand Down Expand Up @@ -325,7 +325,8 @@ private ModuleExtensionContext createContext(
Environment env,
SingleExtensionUsagesValue usagesValue,
StarlarkSemantics starlarkSemantics,
ModuleExtensionId extensionId)
ModuleExtensionId extensionId,
Label.RepoMappingRecorder repoMappingRecorder)
throws ExternalDepsException {
Path workingDirectory =
directories
Expand All @@ -340,7 +341,8 @@ private ModuleExtensionContext createContext(
abridgedModule,
extension,
usagesValue.getRepoMappings().get(moduleKey),
usagesValue.getExtensionUsages().get(moduleKey)));
usagesValue.getExtensionUsages().get(moduleKey),
repoMappingRecorder));
}
ModuleExtensionUsage rootUsage = usagesValue.getExtensionUsages().get(ModuleKey.ROOT);
boolean rootModuleHasNonDevDependency =
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.Maps;
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.PackageIdentifier;
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import com.google.devtools.build.lib.packages.LabelConverter;
Expand Down Expand Up @@ -107,12 +108,14 @@ public static StarlarkBazelModule create(
AbridgedModule module,
ModuleExtension extension,
RepositoryMapping repoMapping,
@Nullable ModuleExtensionUsage usage)
@Nullable ModuleExtensionUsage usage,
Label.RepoMappingRecorder repoMappingRecorder)
throws ExternalDepsException {
LabelConverter labelConverter =
new LabelConverter(
PackageIdentifier.create(repoMapping.ownerRepo(), PathFragment.EMPTY_FRAGMENT),
repoMapping);
repoMapping,
repoMappingRecorder);
ImmutableList<Tag> tags = usage == null ? ImmutableList.of() : usage.getTags();
HashMap<String, ArrayList<TypeCheckedTag>> typeCheckedTags = new HashMap<>();
for (String tagClassName : extension.tagClasses().keySet()) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import com.google.devtools.build.lib.cmdline.RepositoryMapping;
import java.util.HashMap;
import java.util.Map;
import javax.annotation.Nullable;
import net.starlark.java.eval.StarlarkThread;

/**
Expand All @@ -44,16 +45,35 @@ public static LabelConverter forBzlEvaluatingThread(StarlarkThread thread) {

private final Label.PackageContext packageContext;
private final Map<String, Label> labelCache = new HashMap<>();
@Nullable private final Label.RepoMappingRecorder repoMappingRecorder;

public LabelConverter(Label.PackageContext packageContext) {
private LabelConverter(
Label.PackageContext packageContext,
@Nullable Label.RepoMappingRecorder repoMappingRecorder) {
this.packageContext = packageContext;
this.repoMappingRecorder = repoMappingRecorder;
}

public LabelConverter(Label.PackageContext packageContext) {
this(packageContext, null);
}

/** Creates a label converter using the given base package and repo mapping. */
public LabelConverter(PackageIdentifier base, RepositoryMapping repositoryMapping) {
this(Label.PackageContext.of(base, repositoryMapping));
}

/**
* Creates a label converter using the given base package and repo mapping, recording all repo
* mapping lookups in the given recorder.
*/
public LabelConverter(
PackageIdentifier base,
RepositoryMapping repositoryMapping,
Label.RepoMappingRecorder repoMappingRecorder) {
this(Label.PackageContext.of(base, repositoryMapping), repoMappingRecorder);
}

/** Returns the base package identifier that relative labels will be resolved against. */
PackageIdentifier getBasePackage() {
return packageContext.packageIdentifier();
Expand All @@ -67,7 +87,7 @@ public Label convert(String input) throws LabelSyntaxException {
// label-strings across all their attribute values.
Label converted = labelCache.get(input);
if (converted == null) {
converted = Label.parseWithPackageContext(input, packageContext);
converted = Label.parseWithPackageContext(input, packageContext, repoMappingRecorder);
labelCache.put(input, converted);
}
return converted;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -873,6 +873,24 @@ public void labels_passedOnToRepoRule() throws Exception {
}
assertThat(result.get(skyKey).getModule().getGlobal("data"))
.isEqualTo("get up at 6am. go to bed at 11pm.");

SkyKey extensionSkyKey =
SingleExtensionValue.key(
ModuleExtensionId.create(
Label.parseCanonicalUnchecked("@@ext+//:defs.bzl"), "ext", Optional.empty()));
EvaluationResult<SingleExtensionValue> extensionResult =
evaluator.evaluate(ImmutableList.of(extensionSkyKey), evaluationContext);
if (extensionResult.hasError()) {
throw extensionResult.getError().getException();
}
assertThat(
extensionResult
.get(extensionSkyKey)
.lockFileInfo()
.get()
.moduleExtension()
.getRecordedRepoMappingEntries())
.containsCell(RepositoryName.create("foo+"), "bar", RepositoryName.create("bar+"));
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,7 @@ public void basic() throws Exception {
Module module = buildModule("foo", "1.0").setKey(fooKey).addDep("bar", barKey).build();
AbridgedModule abridgedModule = AbridgedModule.from(module);

Label.RepoMappingRecorder repoMappingRecorder = new Label.RepoMappingRecorder();
StarlarkBazelModule moduleProxy =
StarlarkBazelModule.create(
abridgedModule,
Expand All @@ -98,7 +99,8 @@ public void basic() throws Exception {
ImmutableMap.of(
fooKey, fooKey.getCanonicalRepoNameWithoutVersion(),
barKey, barKey.getCanonicalRepoNameWithoutVersion())),
usage);
usage,
repoMappingRecorder);

assertThat(moduleProxy.getName()).isEqualTo("foo");
assertThat(moduleProxy.getVersion()).isEqualTo("1.0");
Expand All @@ -125,6 +127,9 @@ public void basic() throws Exception {
StarlarkList.immutableOf(
Label.parseCanonical("@@foo+//:pom.xml"),
Label.parseCanonical("@@bar+//:pom.xml")));

assertThat(repoMappingRecorder.recordedEntries())
.containsCell(RepositoryName.create("foo+"), "bar", RepositoryName.create("bar+"));
}

@Test
Expand All @@ -145,7 +150,8 @@ public void unknownTagClass() throws Exception {
extension,
module.getRepoMappingWithBazelDepsOnly(
ImmutableMap.of(fooKey, fooKey.getCanonicalRepoNameWithoutVersion())),
usage));
usage,
new Label.RepoMappingRecorder()));
assertThat(e).hasMessageThat().contains("does not have a tag class named blep");
}
}
2 changes: 1 addition & 1 deletion src/test/tools/bzlmod/MODULE.bazel.lock

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 4b8ba71

Please sign in to comment.