diff --git a/site/en/external/extension.md b/site/en/external/extension.md index e0374f8b4bd175..d7a930eca7bf3f 100644 --- a/site/en/external/extension.md +++ b/site/en/external/extension.md @@ -165,6 +165,14 @@ several repo visibility rules: the apparent name `foo`, and the extension generates a repo with the specified name `foo`, then for all repos generated by that extension `foo` refers to the former. +* Similarly, in a module extension's implementation function, repos created + by the extension can refer to each other by their apparent names in + attributes, regardless of the order in which they are created. + * In case of a conflict with a repository visible to the module, labels + passed to repository rule attributes can be wrapped in a call to + [`Label`](/rules/lib/toplevel/attr#label) to ensure that they refer to + the repo visible to the module instead of the repo generated by the + extension of the same name. ## Best practices diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java index a7de7b0b2042d7..92eeda01a4a090 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java @@ -67,7 +67,7 @@ record RepoRuleCall( private final String repoPrefix; private final PackageIdentifier basePackageId; - private final RepositoryMapping repoMapping; + private final RepositoryMapping baseRepoMapping; private final BlazeDirectories directories; private final ExtendedEventHandler eventHandler; private final Map deferredRepos = new LinkedHashMap<>(); @@ -75,19 +75,23 @@ record RepoRuleCall( public ModuleExtensionEvalStarlarkThreadContext( String repoPrefix, PackageIdentifier basePackageId, - RepositoryMapping repoMapping, + RepositoryMapping baseRepoMapping, RepositoryMapping mainRepoMapping, BlazeDirectories directories, ExtendedEventHandler eventHandler) { super(() -> mainRepoMapping); this.repoPrefix = repoPrefix; this.basePackageId = basePackageId; - this.repoMapping = repoMapping; + this.baseRepoMapping = baseRepoMapping; this.directories = directories; this.eventHandler = eventHandler; } - public void lazilyCreateRepos( + /** + * Records a call to a repo rule that should be created at the end of the module extension + * evaluation. + */ + public void lazilyCreateRepo( StarlarkThread thread, Dict kwargs, RuleClass ruleClass) throws EvalException { Object nameValue = kwargs.getOrDefault("name", Starlark.NONE); @@ -106,18 +110,34 @@ public void lazilyCreateRepos( name, new RepoRuleCall( ruleClass, + // The extension may mutate the values of the kwargs after this function returns. (Dict) deepCloneStarlarkObject(kwargs), thread.getCallerLocation(), thread.getCallStack())); } /** - * Returns the repos generated by the extension so far. The key is the "internal name" (as - * specified by the extension) of the repo, and the value is the package containing (only) the - * repo rule. + * Evaluates the repo rule calls recorded by {@link #lazilyCreateRepo} and returns all repos + * generated by the extension. The key is the "internal name" (as specified by the extension) of + * the repo, and the value is the package containing (only) the repo rule. */ public ImmutableMap createAndGetRepos(StarlarkSemantics starlarkSemantics) throws EvalException, InterruptedException { + // LINT.IfChange + // Make it possible to refer to extension repos in the label attributes of another extension + // repo. Wrapping a label in Label(...) ensures that it is evaluated with respect to the + // containing module's repo mapping instead. + ImmutableMap.Builder fullRepoMappingEntries = ImmutableMap.builder(); + fullRepoMappingEntries.putAll(baseRepoMapping.entries()); + fullRepoMappingEntries.putAll( + Maps.asMap( + deferredRepos.keySet(), + apparentName -> RepositoryName.createUnvalidated(repoPrefix + apparentName))); + RepositoryMapping fullRepoMapping = + RepositoryMapping.create( + fullRepoMappingEntries.buildKeepingLast(), baseRepoMapping.ownerRepo()); + // LINT.ThenChange(//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java) + ImmutableMap.Builder repoSpecs = ImmutableMap.builder(); for (var entry : deferredRepos.entrySet()) { String name = entry.getKey(); @@ -127,7 +147,7 @@ public ImmutableMap createAndGetRepos(StarlarkSemantics starla Rule rule = BzlmodRepoRuleCreator.createRule( basePackageId, - repoMapping, + fullRepoMapping, directories, starlarkSemantics, eventHandler, @@ -162,6 +182,7 @@ public ImmutableMap createAndGetRepos(StarlarkSemantics starla return repoSpecs.buildOrThrow(); } + /** Deep-clones a potentially mutable Starlark object. Immutable (sub-)objects are not cloned. */ private static Object deepCloneStarlarkObject(Object x) { if (x instanceof String || x instanceof Boolean) { return x; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java index f128ae882df9e2..9253d38813ea04 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java @@ -70,9 +70,11 @@ private ModuleExtensionRepoMappingEntriesValue computeRepoMappingEntries( // extension generates an internal repo name "bar", then within a repo generated by the // extension, "bar" will refer to the latter. We should explore a way to differentiate between // the two to avoid any surprises. + // LINT.IfChange ImmutableMap.Builder entries = ImmutableMap.builder(); entries.putAll(bazelDepGraphValue.getFullRepoMapping(moduleKey).entries()); entries.putAll(extensionEvalValue.getCanonicalRepoNameToInternalNames().inverse()); return ModuleExtensionRepoMappingEntriesValue.create(entries.buildKeepingLast(), moduleKey); + // LINT.ThenChange(//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionEvalStarlarkThreadContext.java) } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java index e162e41fbf867b..27194373cb26c3 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java @@ -569,7 +569,7 @@ RunModuleExtensionResult run( SingleExtensionUsagesValue usagesValue, StarlarkSemantics starlarkSemantics, ModuleExtensionId extensionId, - RepositoryMapping repositoryMapping) + RepositoryMapping mainRepositoryMapping) throws InterruptedException, SingleExtensionEvalFunctionException; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java index a8dde169645031..a8e3571eebce5c 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/repository/starlark/StarlarkRepositoryModule.java @@ -222,7 +222,7 @@ public Object call(StarlarkThread thread, Tuple args, Dict kwarg if (!isExported()) { throw new EvalException("attempting to instantiate a non-exported repository rule"); } - extensionEvalContext.lazilyCreateRepos(thread, kwargs, getRuleClass()); + extensionEvalContext.lazilyCreateRepo(thread, kwargs, getRuleClass()); return Starlark.NONE; } diff --git a/src/main/java/net/starlark/java/eval/EvalException.java b/src/main/java/net/starlark/java/eval/EvalException.java index b8d00ddbdcac57..41abf725f7f71b 100644 --- a/src/main/java/net/starlark/java/eval/EvalException.java +++ b/src/main/java/net/starlark/java/eval/EvalException.java @@ -57,6 +57,10 @@ public EvalException(Throwable cause) { super(getCauseMessage(cause), cause); } + /** + * Constructs an EvalException using the same message as the cause exception and with the given + * callstack. + */ public EvalException(Throwable cause, List callstack) { this(cause); this.callstack = ImmutableList.copyOf(callstack); diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index c04f2799313530..cae52eee6e4f15 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -3066,8 +3066,8 @@ def _other_ext_impl(ctx): } assertThat((List) result.get(skyKey).getModule().getGlobal("foo_list")) .containsExactly( - "@@+other_ext+other_foo//:target1", - "@@+other_ext+other_bar//:target2", + "@@+ext+foo//:target1", + "@@+ext+bar//:target2", "@@+other_ext+other_foo//:target3", "@@+other_ext+other_bar//:target4", "@@+other_ext+other_foo//:foo", @@ -3075,8 +3075,8 @@ def _other_ext_impl(ctx): .inOrder(); assertThat((List) result.get(skyKey).getModule().getGlobal("bar_list")) .containsExactly( - "@@+other_ext+other_foo//:target5", - "@@+other_ext+other_bar//:target6", + "@@+ext+foo//:target5", + "@@+ext+bar//:target6", "@@+other_ext+other_foo//:target7", "@@+other_ext+other_bar//:target8", "@@+other_ext+other_foo//:foo",