Skip to content

Commit

Permalink
Allow use_repo_rule repos to load from each other
Browse files Browse the repository at this point in the history 
Generate one synthetic "innate" module extension per repo rule class instead of a single one for all `use_repo_rule` usages in a given module. This avoids load cycles if one repo rule definition loads another one.

Closes #24104.

PiperOrigin-RevId: 691954099
Change-Id: Ia2eae6b6f45b169f364f1e83b76e8d907378329f
  • Loading branch information
@fmeum @copybara-github
fmeum authored and copybara-github committed Oct 31, 2024
1 parent 20f9a5e commit aa7317f
Show file tree
Hide file tree
Showing 8 changed files with 170 additions and 168 deletions.
7 changes: 5 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -181,6 +181,9 @@ private ImmutableBiMap<String, ModuleExtensionId> calculateUniqueNameForUsedExte
private static String makeUniqueNameCandidate(ModuleExtensionId id, int attempt) {
Preconditions.checkArgument(attempt >= 1);
String extensionNameDisambiguator = attempt == 1 ? "" : String.valueOf(attempt);
// An innate extension name is of the form @repo//path/to/defs.bzl%repo_rule_name, which cannot
// be part of a valid repo name.
String extensionName = id.isInnate() ? "_repo_rules" : id.getExtensionName();
return id.getIsolationKey()
.map(
isolationKey ->
Expand All @@ -191,15 +194,15 @@ private static String makeUniqueNameCandidate(ModuleExtensionId id, int attempt)
// case of an exported symbol cannot start with "_".
"%s+_%s%s+%s+%s+%s",
id.getBzlFileLabel().getRepository().getName(),
id.getExtensionName(),
extensionName,
extensionNameDisambiguator,
isolationKey.getModule().name(),
isolationKey.getModule().version(),
isolationKey.getUsageExportedName()))
.orElse(
id.getBzlFileLabel().getRepository().getName()
+ "+"
+ id.getExtensionName()
+ extensionName
+ extensionNameDisambiguator);
}

Expand Down
205 changes: 81 additions & 124 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/InnateRunnableExtension.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -15,10 +15,9 @@

package com.google.devtools.build.lib.bazel.bzlmod;

import static com.google.common.collect.ImmutableList.toImmutableList;
import static com.google.common.collect.ImmutableSet.toImmutableSet;

import com.google.auto.value.AutoBuilder;
import com.google.common.base.Preconditions;
import com.google.common.base.Splitter;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
Expand All @@ -38,11 +37,7 @@
import com.google.devtools.build.lib.skyframe.BzlLoadFailedException;
import com.google.devtools.build.lib.skyframe.BzlLoadFunction;
import com.google.devtools.build.lib.skyframe.BzlLoadValue;
import com.google.devtools.build.lib.util.Fingerprint;
import com.google.devtools.build.skyframe.SkyFunction.Environment;
import com.google.devtools.build.skyframe.SkyframeLookupResult;
import java.util.ArrayList;
import java.util.HashSet;
import java.util.Iterator;
import java.util.Map.Entry;
import java.util.Optional;
Expand All @@ -53,25 +48,34 @@
import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.spelling.SpellChecker;
import net.starlark.java.syntax.Location;

/**
* A fabricated module extension "innate" to each module, used to generate all repos defined using
* {@code use_repo_rule}.
* {@code use_repo_rule} for a single repo rule.
*/
final class InnateRunnableExtension implements RunnableExtension {
private final ModuleKey moduleKey;
private final ImmutableList<InnateExtensionRepo> repos;
private final byte[] transitiveBzlDigest;
private final Label bzlLabel;
private final String ruleName;
private final BzlLoadValue loadedBzl;
// Never empty.
private final ImmutableList<Tag> tags;
private final BlazeDirectories directories;

InnateRunnableExtension(
ModuleKey moduleKey,
ImmutableList<InnateExtensionRepo> repos,
byte[] transitiveBzlDigest,
Label bzlLabel,
String ruleName,
BzlLoadValue loadedBzl,
ImmutableList<Tag> tags,
BlazeDirectories directories) {
this.moduleKey = moduleKey;
this.repos = repos;
this.transitiveBzlDigest = transitiveBzlDigest;
this.bzlLabel = bzlLabel;
this.ruleName = ruleName;
this.loadedBzl = loadedBzl;
Preconditions.checkArgument(!tags.isEmpty());
this.tags = tags;
this.directories = directories;
}

Expand All @@ -93,65 +97,46 @@ static InnateRunnableExtension load(
usagesValue.getExtensionUsages().keySet());
}
ModuleKey moduleKey = Iterables.getOnlyElement(usagesValue.getExtensionUsages().keySet());
// ModuleFileFunction doesn't add usages for use_repo_rule without any instantiations, so we can
// assume that there is at least one tag.
ImmutableList<Tag> tags =
Iterables.getOnlyElement(usagesValue.getExtensionUsages().values()).getTags();
RepositoryMapping repoMapping = usagesValue.getRepoMappings().get(moduleKey);

// Each tag of this usage defines a repo. The name of the tag is of the form
// "<bzl_file_label>%<rule_name>". Collect the .bzl files referenced and load them.
Label.RepoContext repoContext = Label.RepoContext.of(repoMapping.ownerRepo(), repoMapping);
ArrayList<InnateExtensionRepo.Builder> repoBuilders = new ArrayList<>(tags.size());
for (Tag tag : tags) {
Iterator<String> parts = Splitter.on('%').split(tag.getTagName()).iterator();
InnateExtensionRepo.Builder repoBuilder = InnateExtensionRepo.builder().tag(tag);
repoBuilders.add(repoBuilder);
try {
Label label = Label.parseWithRepoContext(parts.next(), repoContext);
BzlLoadFunction.checkValidLoadLabel(label, starlarkSemantics);
repoBuilder.bzlLabel(label).ruleName(parts.next());
} catch (LabelSyntaxException e) {
throw ExternalDepsException.withCauseAndMessage(
Code.BAD_MODULE, e, "bad repo rule .bzl file label at %s", tag.getLocation());
}

// The name of the extension is of the form "<bzl_file_label>%<rule_name>".
Iterator<String> parts = Splitter.on('%').split(extensionId.getExtensionName()).iterator();
Location location = tags.getFirst().getLocation();
Label bzlLabel;
try {
bzlLabel = Label.parseWithRepoContext(parts.next(), repoContext);
BzlLoadFunction.checkValidLoadLabel(bzlLabel, starlarkSemantics);
} catch (LabelSyntaxException e) {
throw ExternalDepsException.withCauseAndMessage(
Code.BAD_MODULE, e, "bad repo rule .bzl file label at %s", location);
}
ImmutableSet<BzlLoadValue.Key> loadKeys =
repoBuilders.stream()
.map(r -> BzlLoadValue.keyForBzlmod(r.bzlLabel()))
.collect(toImmutableSet());
HashSet<Label> digestedLabels = new HashSet<>();
Fingerprint transitiveBzlDigest = new Fingerprint();
SkyframeLookupResult loadResult = env.getValuesAndExceptions(loadKeys);
for (InnateExtensionRepo.Builder repoBuilder : repoBuilders) {
BzlLoadValue loadedBzl;
try {
loadedBzl =
(BzlLoadValue)
loadResult.getOrThrow(
BzlLoadValue.keyForBzlmod(repoBuilder.bzlLabel()),
BzlLoadFailedException.class);
} catch (BzlLoadFailedException e) {
throw ExternalDepsException.withCauseAndMessage(
Code.BAD_MODULE,
e,
"error loading '%s' for repo rules, requested by %s",
repoBuilder.bzlLabel(),
repoBuilder.tag().getLocation());
}
if (loadedBzl == null) {
return null;
}
repoBuilder.loadedBzl(loadedBzl);
if (digestedLabels.add(repoBuilder.bzlLabel())) {
// Only digest this BzlLoadValue if we haven't seen this bzl label before.
transitiveBzlDigest.addBytes(loadedBzl.getTransitiveDigest());
}
String ruleName = parts.next();

// Load the .bzl file.
BzlLoadValue loadedBzl;
try {
loadedBzl =
(BzlLoadValue)
env.getValueOrThrow(
BzlLoadValue.keyForBzlmod(bzlLabel), BzlLoadFailedException.class);
} catch (BzlLoadFailedException e) {
throw ExternalDepsException.withCauseAndMessage(
Code.BAD_MODULE,
e,
"error loading '%s' for repo rules, requested by %s",
bzlLabel,
location);
}
if (loadedBzl == null) {
return null;
}

return new InnateRunnableExtension(
moduleKey,
repoBuilders.stream().map(InnateExtensionRepo.Builder::build).collect(toImmutableList()),
transitiveBzlDigest.digestAndReset(),
directories);
return new InnateRunnableExtension(moduleKey, bzlLabel, ruleName, loadedBzl, tags, directories);
}

@Override
Expand All @@ -161,7 +146,7 @@ public ModuleExtensionEvalFactors getEvalFactors() {

@Override
public byte[] getBzlTransitiveDigest() {
return transitiveBzlDigest;
return loadedBzl.getTransitiveDigest();
}

@Override
Expand All @@ -177,42 +162,42 @@ public RunModuleExtensionResult run(
ModuleExtensionId extensionId,
RepositoryMapping mainRepositoryMapping)
throws InterruptedException, ExternalDepsException {
var generatedRepoSpecs = ImmutableMap.<String, RepoSpec>builderWithExpectedSize(repos.size());
Object exported = loadedBzl.getModule().getGlobal(ruleName);
if (exported == null) {
ImmutableSet<String> exportedRepoRules =
loadedBzl.getModule().getGlobals().entrySet().stream()
.filter(e -> e.getValue() instanceof RepositoryRuleFunction)
.map(Entry::getKey)
.collect(toImmutableSet());
throw ExternalDepsException.withMessage(
Code.BAD_MODULE,
"%s does not export a repository_rule called %s, yet its use is requested at" + " %s%s",
bzlLabel,
ruleName,
tags.getFirst().getLocation(),
SpellChecker.didYouMean(ruleName, exportedRepoRules));
} else if (!(exported instanceof RepositoryRuleFunction)) {
throw ExternalDepsException.withMessage(
Code.BAD_MODULE,
"%s exports a value called %s of type %s, yet a repository_rule is requested" + " at %s",
bzlLabel,
ruleName,
Starlark.type(exported),
tags.getFirst().getLocation());
}
RepositoryRuleFunction repoRule = (RepositoryRuleFunction) exported;

var generatedRepoSpecs = ImmutableMap.<String, RepoSpec>builderWithExpectedSize(tags.size());
// Instantiate the repos one by one.
for (InnateExtensionRepo repo : repos) {
Object exported = repo.loadedBzl().getModule().getGlobal(repo.ruleName());
if (exported == null) {
ImmutableSet<String> exportedRepoRules =
repo.loadedBzl().getModule().getGlobals().entrySet().stream()
.filter(e -> e.getValue() instanceof RepositoryRuleFunction)
.map(Entry::getKey)
.collect(toImmutableSet());
throw ExternalDepsException.withMessage(
Code.BAD_MODULE,
"%s does not export a repository_rule called %s, yet its use is requested at" + " %s%s",
repo.bzlLabel(),
repo.ruleName(),
repo.tag().getLocation(),
SpellChecker.didYouMean(repo.ruleName(), exportedRepoRules));
} else if (!(exported instanceof RepositoryRuleFunction)) {
throw ExternalDepsException.withMessage(
Code.BAD_MODULE,
"%s exports a value called %s of type %s, yet a repository_rule is requested"
+ " at %s",
repo.bzlLabel(),
repo.ruleName(),
Starlark.type(exported),
repo.tag().getLocation());
}
RepositoryRuleFunction repoRule = (RepositoryRuleFunction) exported;
Dict<String, Object> kwargs = repo.tag().getAttributeValues().attributes();
for (Tag tag : tags) {
Dict<String, Object> kwargs = tag.getAttributeValues().attributes();
// This cast should be safe since it should have been verified at tag creation time.
String name = (String) kwargs.get("name");
String prefixedName = usagesValue.getExtensionUniqueName() + "+" + name;
Rule ruleInstance;
AttributeValues attributesValue;
var fakeCallStackEntry =
StarlarkThread.callStackEntry("InnateRunnableExtension.run", repo.tag().getLocation());
StarlarkThread.callStackEntry("InnateRunnableExtension.run", tag.getLocation());
// Rule creation strips the top-most entry from the call stack, so we need to add the fake
// one twice.
ImmutableList<StarlarkThread.CallStackEntry> fakeCallStack =
Expand All @@ -239,11 +224,7 @@ public RunModuleExtensionResult run(
String.format("%s '%s'", ruleInstance.getRuleClass(), name));
} catch (InvalidRuleException | NoSuchPackageException | EvalException e) {
throw ExternalDepsException.withCauseAndMessage(
Code.BAD_MODULE,
e,
"error creating repo %s requested at %s",
name,
repo.tag().getLocation());
Code.BAD_MODULE, e, "error creating repo %s requested at %s", name, tag.getLocation());
}
RepoSpec repoSpec =
RepoSpec.builder()
Expand All @@ -265,28 +246,4 @@ public RunModuleExtensionResult run(
Optional.of(ModuleExtensionMetadata.REPRODUCIBLE),
ImmutableTable.of());
}

/** Information about a single repo to be created by an innate extension. */
record InnateExtensionRepo(Label bzlLabel, String ruleName, Tag tag, BzlLoadValue loadedBzl) {
static Builder builder() {
return new AutoBuilder_InnateRunnableExtension_InnateExtensionRepo_Builder();
}

@AutoBuilder
interface Builder {
Builder bzlLabel(Label value);

Label bzlLabel();

Builder ruleName(String value);

Builder tag(Tag value);

Tag tag();

Builder loadedBzl(BzlLoadValue value);

InnateExtensionRepo build();
}
}
}
8 changes: 1 addition & 7 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionId.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -35,12 +35,6 @@ public abstract class ModuleExtensionId {
ModuleExtensionId::getIsolationKey,
emptiesFirst(IsolationKey.LEXICOGRAPHIC_COMPARATOR));

/**
* The "magical" name of innate extensions, which are fabricated extensions that modules with
* usages of {@code use_repo_rule} have.
*/
public static final String INNATE_EXTENSION_NAME = "_repo_rules";

/** A unique identifier for a single isolated usage of a fixed module extension. */
@AutoValue
abstract static class IsolationKey {
Expand Down Expand Up @@ -82,7 +76,7 @@ public static ModuleExtensionId create(
}

public final boolean isInnate() {
return getExtensionName().equals(INNATE_EXTENSION_NAME);
return getExtensionName().contains("%");
}

public String asTargetString() {
Expand Down
Loading

0 comments on commit aa7317f

Please sign in to comment.