Skip to content

Commit 764614e

Browse files
WyveraldWyverald
authored
Bzlmod: Allow multiple use_extensions on the same extension (bazelbuild#14945)
Fixes bazelbuild#14635. PiperOrigin-RevId: 432155351
1 parent c45838b commit 764614e

3 files changed

Lines changed: 87 additions & 18 deletions

File tree

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -31,7 +31,11 @@ public abstract class ModuleExtensionUsage {
3131
/** The name of the extension. */
3232
public abstract String getExtensionName();
3333

34-
/** The location where this proxy object was created (by the {@code use_extension} call). */
34+
/**
35+
* The location where this proxy object was created (by the {@code use_extension} call). Note that
36+
* if there were multiple {@code use_extension} calls on same extension, then this only stores the
37+
* location of the first one.
38+
*/
3539
public abstract Location getLocation();
3640

3741
/**

‎src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java‎

Lines changed: 14 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -273,20 +273,25 @@ public void bazelDep(
273273
public ModuleExtensionProxy useExtension(
274274
String extensionBzlFile, String extensionName, boolean devDependency, StarlarkThread thread)
275275
throws EvalException {
276+
ModuleExtensionProxy newProxy =
277+
new ModuleExtensionProxy(extensionBzlFile, extensionName, thread.getCallerLocation());
278+
279+
if (ignoreDevDeps && devDependency) {
280+
// This is a no-op proxy.
281+
return newProxy;
282+
}
283+
284+
// Find an existing proxy object corresponding to this extension.
276285
for (ModuleExtensionProxy proxy : extensionProxies) {
277286
if (proxy.extensionBzlFile.equals(extensionBzlFile)
278287
&& proxy.extensionName.equals(extensionName)) {
279-
throw Starlark.errorf("this extension is already being used at %s", proxy.location);
288+
return proxy;
280289
}
281290
}
282-
ModuleExtensionProxy proxy =
283-
new ModuleExtensionProxy(extensionBzlFile, extensionName, thread.getCallerLocation());
284291

285-
if (!(ignoreDevDeps && devDependency)) {
286-
extensionProxies.add(proxy);
287-
}
288-
289-
return proxy;
292+
// If no such proxy exists, we can just use a new one.
293+
extensionProxies.add(newProxy);
294+
return newProxy;
290295
}
291296

292297
@StarlarkBuiltin(name = "module_extension_proxy", documented = false)
@@ -368,9 +373,7 @@ public String getErrorMessageForUnknownField(String field) {
368373
parameters = {
369374
@Param(
370375
name = "extension_proxy",
371-
doc =
372-
"A module extension proxy object returned by a <code>get_module_extension</code>"
373-
+ " call."),
376+
doc = "A module extension proxy object returned by a <code>use_extension</code> call."),
374377
},
375378
extraPositionals = @Param(name = "args", doc = "The names of the repos to import."),
376379
extraKeywords =

‎src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java‎

Lines changed: 68 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -499,21 +499,83 @@ public void testModuleExtensions_good() throws Exception {
499499
}
500500

501501
@Test
502-
public void testModuleExtensions_duplicateProxy() throws Exception {
502+
public void testModuleExtensions_duplicateProxy_asRoot() throws Exception {
503+
scratch.file(
504+
rootDirectory.getRelative("MODULE.bazel").getPathString(),
505+
"myext1 = use_extension('//:defs.bzl','myext',dev_dependency=True)",
506+
"use_repo(myext1, 'alpha')",
507+
"myext2 = use_extension('//:defs.bzl','myext')",
508+
"use_repo(myext2, 'beta')",
509+
"myext3 = use_extension('//:defs.bzl','myext',dev_dependency=True)",
510+
"use_repo(myext3, 'gamma')",
511+
"myext4 = use_extension('//:defs.bzl','myext')",
512+
"use_repo(myext4, 'delta')");
513+
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of());
514+
515+
SkyKey skyKey = ModuleFileValue.KEY_FOR_ROOT_MODULE;
516+
EvaluationResult<ModuleFileValue> result =
517+
driver.evaluate(ImmutableList.of(skyKey), evaluationContext);
518+
if (result.hasError()) {
519+
throw result.getError().getException();
520+
}
521+
ModuleFileValue moduleFileValue = result.get(skyKey);
522+
assertThat(moduleFileValue.getModule())
523+
.isEqualTo(
524+
Module.builder()
525+
.setKey(ModuleKey.ROOT)
526+
.addExtensionUsage(
527+
ModuleExtensionUsage.builder()
528+
.setExtensionBzlFile("//:defs.bzl")
529+
.setExtensionName("myext")
530+
.setLocation(Location.fromFileLineColumn("<root>/MODULE.bazel", 1, 23))
531+
.setImports(
532+
ImmutableBiMap.of(
533+
"alpha", "alpha", "beta", "beta", "gamma", "gamma", "delta",
534+
"delta"))
535+
.build())
536+
.build());
537+
}
538+
539+
@Test
540+
public void testModuleExtensions_duplicateProxy_asDep() throws Exception {
503541
FakeRegistry registry =
504542
registryFactory
505543
.newFakeRegistry("/foo")
506544
.addModule(
507545
createModuleKey("mymod", "1.0"),
508546
"module(name='mymod',version='1.0')",
509-
"myext1 = use_extension('//:defs.bzl','myext')",
510-
"myext2 = use_extension('//:defs.bzl','myext')");
547+
"myext1 = use_extension('//:defs.bzl','myext',dev_dependency=True)",
548+
"use_repo(myext1, 'alpha')",
549+
"myext2 = use_extension('//:defs.bzl','myext')",
550+
"use_repo(myext2, 'beta')",
551+
"myext3 = use_extension('//:defs.bzl','myext',dev_dependency=True)",
552+
"use_repo(myext3, 'gamma')",
553+
"myext4 = use_extension('//:defs.bzl','myext')",
554+
"use_repo(myext4, 'delta')");
511555
ModuleFileFunction.REGISTRIES.set(differencer, ImmutableList.of(registry.getUrl()));
512556

513557
SkyKey skyKey = ModuleFileValue.key(createModuleKey("mymod", "1.0"), null);
514-
reporter.removeHandler(failFastHandler); // expect failures
515-
driver.evaluate(ImmutableList.of(skyKey), evaluationContext);
516-
assertContainsEvent("this extension is already being used at");
558+
EvaluationResult<ModuleFileValue> result =
559+
driver.evaluate(ImmutableList.of(skyKey), evaluationContext);
560+
if (result.hasError()) {
561+
throw result.getError().getException();
562+
}
563+
ModuleFileValue moduleFileValue = result.get(skyKey);
564+
assertThat(moduleFileValue.getModule())
565+
.isEqualTo(
566+
Module.builder()
567+
.setName("mymod")
568+
.setVersion(Version.parse("1.0"))
569+
.setKey(createModuleKey("mymod", "1.0"))
570+
.setRegistry(registry)
571+
.addExtensionUsage(
572+
ModuleExtensionUsage.builder()
573+
.setExtensionBzlFile("//:defs.bzl")
574+
.setExtensionName("myext")
575+
.setLocation(Location.fromFileLineColumn("[email protected]/MODULE.bazel", 4, 23))
576+
.setImports(ImmutableBiMap.of("beta", "beta", "delta", "delta"))
577+
.build())
578+
.build());
517579
}
518580

519581
@Test

0 commit comments

Comments
 (0)