Skip to content

Commit

Permalink
bzlmod: Store a Package instead of a Rule in BzlmodRepoRuleValue
Browse files Browse the repository at this point in the history 
    (bazelbuild/bazel#13316)

    This is to work around a restriction in Google. The external packages we create in BzlmodRepoRuleFunction host just one rule each; so instead of storing a rule, we can just store the package and get the rule from there.

    This does require adding the rule to the package, which we weren't doing before. This involved a bit of a detour to clean up the RuleFactory#createAndAddRule method, which for some reason has had a much more sensible overload that's package private since time immemorial. This CL simply removes the overload that just gets two fields out of PackageContext.

    PiperOrigin-RevId: 387543141
  • Loading branch information
Luca Di Grazia committed Sep 4, 2022
1 parent a99831c commit 4baebd5
Show file tree
Hide file tree
Showing 8 changed files with 1,600 additions and 1,553 deletions.
566 changes: 331 additions & 235 deletions ...main/java/com/google/devtools/build/lib/analysis/starlark/StarlarkRuleClassFunctions.java
View file

Large diffs are not rendered by default.

5 changes: 3 additions & 2 deletions ...bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleCreator.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Package.NameConflictException;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
import java.util.Map;
Expand All @@ -26,10 +27,10 @@
* parameters.
*/
public interface BzlmodRepoRuleCreator {
Rule createRule(
Rule createAndAddRule(
Package.Builder packageBuilder,
StarlarkSemantics semantics,
Map<String, Object> kwargs,
EventHandler handler)
throws InterruptedException, InvalidRuleException;
throws InterruptedException, InvalidRuleException, NameConflictException;
}
14 changes: 9 additions & 5 deletions ...d.bazel/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BzlmodRepoRuleValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,25 +16,29 @@

import com.google.common.collect.Interner;
import com.google.devtools.build.lib.concurrent.BlazeInterners;
import com.google.devtools.build.lib.packages.Package;
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec;
import com.google.devtools.build.skyframe.AbstractSkyKey;
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyValue;

/** The result of {@link BzlmodRepoRuleFunction}, holding a repository rule instance. */
@AutoCodec(explicitlyAllowClass = {Package.class})
public class BzlmodRepoRuleValue implements SkyValue {
public static final SkyFunctionName BZLMOD_REPO_RULE =
SkyFunctionName.createHermetic("BZLMOD_REPO_RULE");

private final Rule rule;
private final Package pkg;
private final String ruleName;

public BzlmodRepoRuleValue(Rule rule) {
this.rule = rule;
public BzlmodRepoRuleValue(Package pkg, String ruleName) {
this.pkg = pkg;
this.ruleName = ruleName;
}

public Rule getRule() {
return rule;
return pkg.getRule(ruleName);
}

public static Key key(String repositoryName) {
Expand All @@ -44,7 +48,7 @@ public static Key key(String repositoryName) {
/** Represents an unsuccessful repository lookup. */
public static final class RepoRuleNotFoundValue extends BzlmodRepoRuleValue {
private RepoRuleNotFoundValue() {
super(/* rule= */ null);
super(/*pkg=*/ null, /*ruleName=*/ null);
}

@Override
Expand Down
73 changes: 54 additions & 19 deletions ...ava/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,10 +20,13 @@
import static com.google.devtools.build.lib.packages.Type.STRING_LIST;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.analysis.BaseRuleClasses;
import com.google.devtools.build.lib.analysis.starlark.StarlarkAttrModule.Descriptor;
import com.google.devtools.build.lib.bazel.bzlmod.BzlmodRepoRuleCreator;
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.events.EventHandler;
import com.google.devtools.build.lib.packages.AttributeValueSource;
import com.google.devtools.build.lib.packages.BazelModuleContext;
import com.google.devtools.build.lib.packages.BazelStarlarkContext;
Expand All @@ -34,20 +37,27 @@
import com.google.devtools.build.lib.packages.Rule;
import com.google.devtools.build.lib.packages.RuleClass;
import com.google.devtools.build.lib.packages.RuleClass.Builder.RuleClassType;
import com.google.devtools.build.lib.packages.RuleFactory;
import com.google.devtools.build.lib.packages.RuleFactory.BuildLangTypedAttributeValuesMap;
import com.google.devtools.build.lib.packages.RuleFactory.InvalidRuleException;
import com.google.devtools.build.lib.packages.StarlarkExportable;
import com.google.devtools.build.lib.packages.WorkspaceFactoryHelper;
import com.google.devtools.build.lib.skylarkbuildapi.repository.RepositoryModuleApi;
import com.google.devtools.build.lib.syntax.Dict;
import com.google.devtools.build.lib.syntax.EvalException;
import com.google.devtools.build.lib.syntax.Module;
import com.google.devtools.build.lib.syntax.Printer;
import com.google.devtools.build.lib.syntax.Sequence;
import com.google.devtools.build.lib.syntax.Starlark;
import com.google.devtools.build.lib.syntax.StarlarkCallable;
import com.google.devtools.build.lib.syntax.StarlarkThread;
import com.google.devtools.build.lib.syntax.Tuple;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryModuleApi;
import java.util.Map;
import net.starlark.java.annot.StarlarkBuiltin;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Module;
import net.starlark.java.eval.Printer;
import net.starlark.java.eval.Sequence;
import net.starlark.java.eval.Starlark;
import net.starlark.java.eval.StarlarkCallable;
import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.StarlarkThread.CallStackEntry;
import net.starlark.java.eval.Tuple;
import net.starlark.java.syntax.Location;

/**
* The Starlark module containing the definition of {@code repository_rule} function to define a
Expand Down Expand Up @@ -77,12 +87,11 @@ public StarlarkCallable repositoryRule(

builder.addAttribute(attr("$local", BOOLEAN).defaultValue(local).build());
builder.addAttribute(attr("$configure", BOOLEAN).defaultValue(configure).build());
if (thread.getSemantics().experimentalRepoRemoteExec()) {
if (thread.getSemantics().getBool(BuildLanguageOptions.EXPERIMENTAL_REPO_REMOTE_EXEC)) {
builder.addAttribute(attr("$remotable", BOOLEAN).defaultValue(remotable).build());
BaseRuleClasses.execPropertiesAttribute(builder);
}
builder.addAttribute(attr("$environ", STRING_LIST).defaultValue(environ).build());
BaseRuleClasses.nameAttribute(builder);
BaseRuleClasses.commonCoreAndStarlarkAttributes(builder);
builder.add(attr("expect_failure", STRING));
if (attrs != Starlark.NONE) {
Expand All @@ -100,7 +109,7 @@ public StarlarkCallable repositoryRule(
}
builder.setConfiguredTargetFunction(implementation);
BazelModuleContext bzlModule =
(BazelModuleContext) Module.ofInnermostEnclosingStarlarkFunction(thread).getClientData();
BazelModuleContext.of(Module.ofInnermostEnclosingStarlarkFunction(thread));
builder.setRuleDefinitionEnvironmentLabelAndDigest(
bzlModule.label(), bzlModule.bzlTransitiveDigest());
builder.setWorkspaceOnly();
Expand All @@ -109,8 +118,14 @@ public StarlarkCallable repositoryRule(

// RepositoryRuleFunction is the result of repository_rule(...).
// It is a callable value; calling it yields a Rule instance.
@StarlarkBuiltin(
name = "repository_rule",
category = DocCategory.BUILTIN,
doc =
"A callable value that may be invoked during evaluation of the WORKSPACE file to"
+ " instantiate and return a repository rule.")
private static final class RepositoryRuleFunction
implements StarlarkCallable, StarlarkExportable {
implements StarlarkCallable, StarlarkExportable, BzlmodRepoRuleCreator {
private final RuleClass.Builder builder;
private final StarlarkCallable implementation;
private Label extensionLabel;
Expand All @@ -132,7 +147,7 @@ public boolean isImmutable() {
}

@Override
public void export(Label extensionLabel, String exportedName) {
public void export(EventHandler handler, Label extensionLabel, String exportedName) {
this.extensionLabel = extensionLabel;
this.exportedName = exportedName;
}
Expand All @@ -152,10 +167,11 @@ public void repr(Printer printer) {
}

@Override
public Object call(StarlarkThread thread, Tuple<Object> args, Dict<String, Object> kwargs)
public Object call(StarlarkThread thread, Tuple args, Dict<String, Object> kwargs)
throws EvalException, InterruptedException {
BazelStarlarkContext.from(thread).checkWorkspacePhase("repository rule " + exportedName);
if (!args.isEmpty()) {
throw new EvalException(null, "unexpected positional arguments");
throw new EvalException("unexpected positional arguments");
}
String ruleClassName;
// If the function ever got exported (the common case), we take the name
Expand All @@ -172,7 +188,7 @@ public Object call(StarlarkThread thread, Tuple<Object> args, Dict<String, Objec
// now many projects create and instantiate repository_rules without an
// intervening export; see b/111199163. An incompatible flag is required.
if (false) {
throw new EvalException(null, "attempt to instantiate a non-exported repository rule");
throw new EvalException("attempt to instantiate a non-exported repository rule");
}

// The historical workaround was a fragile hack to introspect on the call
Expand Down Expand Up @@ -203,12 +219,31 @@ public Object call(StarlarkThread thread, Tuple<Object> args, Dict<String, Objec
throw Starlark.errorf("%s", e.getMessage());
}
}

@Override
public Rule createAndAddRule(
Package.Builder packageBuilder,
StarlarkSemantics semantics,
Map<String, Object> kwargs,
EventHandler handler)
throws InterruptedException, InvalidRuleException, NameConflictException {
RuleClass ruleClass = builder.build(exportedName, exportedName);
BuildLangTypedAttributeValuesMap attributeValues =
new BuildLangTypedAttributeValuesMap(kwargs);
ImmutableList.Builder<CallStackEntry> callStack = ImmutableList.builder();
// TODO(pcloudy): Optimize the callstack
callStack.add(new CallStackEntry("RepositoryRuleFunction.createRule", Location.BUILTIN));
return RuleFactory.createAndAddRule(
packageBuilder, ruleClass, attributeValues, handler, semantics, callStack.build());
}
}

@Override
public void failWithIncompatibleUseCcConfigureFromRulesCc(StarlarkThread thread)
throws EvalException {
if (thread.getSemantics().incompatibleUseCcConfigureFromRulesCc()) {
if (thread
.getSemantics()
.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_CC_CONFIGURE_FROM_RULES_CC)) {
throw Starlark.errorf(
"Incompatible flag "
+ "--incompatible_use_cc_configure_from_rules_cc has been flipped. Please use "
Expand Down
Loading

0 comments on commit 4baebd5

Please sign in to comment.