From 9c03c2a860f1d7c6e1d0c5d17308f45aa4ec4c5c Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 12 Jul 2023 09:28:22 -0700 Subject: [PATCH] Track dev/non-dev `use_extension` calls By always tracking whether a given extension usage by a module has `use_extension` calls with and/or without `dev_dependency = True` instead of just for isolated extension usages, we obtain the following advantages: * Module extensions can use `module_ctx.is_dev_dependency` to learn whether the root module contains only `use_extension` calls with `dev_dependency = True` for them. This is necessary to decide whether repositories that do not directly correspond to tags (e.g. hub repos) should be marked as dev or non-dev dependencies in `module_ctx.extension_metadata`. * `ModuleExtensionMetadata` consistency checks of the type "no dev/non-dev imports without dev/non-dev usage" are generalized from isolated to all extensions. * Prepares for the removal of `isDevUsage` from `IsolationKey` in a follow-up change which will instead use the exported name of the (only) usage proxy of an isolated usage as the key. Closes #18829. PiperOrigin-RevId: 547517851 Change-Id: I1290e53adf735a16d62e2c103f3776ecbd5b1a18 --- .../bazel/bzlmod/ModuleExtensionContext.java | 26 +++-- .../bazel/bzlmod/ModuleExtensionMetadata.java | 72 +++++-------- .../bazel/bzlmod/ModuleExtensionUsage.java | 16 +++ .../lib/bazel/bzlmod/ModuleFileGlobals.java | 10 ++ .../bzlmod/SingleExtensionEvalFunction.java | 6 +- .../bzlmod/BazelDepGraphFunctionTest.java | 2 + .../bzlmod/ModuleExtensionResolutionTest.java | 102 +++++++++++------- .../bazel/bzlmod/ModuleFileFunctionTest.java | 10 ++ .../bazel/bzlmod/StarlarkBazelModuleTest.java | 4 +- 9 files changed, 154 insertions(+), 94 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java index 369a833c07db66..b7a392673c2db9 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionContext.java @@ -46,6 +46,7 @@ public class ModuleExtensionContext extends StarlarkBaseExternalContext { private final ModuleExtensionId extensionId; private final StarlarkList modules; + private final boolean rootModuleHasNonDevDependency; protected ModuleExtensionContext( Path workingDirectory, @@ -57,7 +58,8 @@ protected ModuleExtensionContext( StarlarkSemantics starlarkSemantics, @Nullable RepositoryRemoteExecutor remoteExecutor, ModuleExtensionId extensionId, - StarlarkList modules) { + StarlarkList modules, + boolean rootModuleHasNonDevDependency) { super( workingDirectory, env, @@ -69,6 +71,7 @@ protected ModuleExtensionContext( remoteExecutor); this.extensionId = extensionId; this.modules = modules; + this.rootModuleHasNonDevDependency = rootModuleHasNonDevDependency; } public Path getWorkingDirectory() { @@ -96,10 +99,11 @@ protected ImmutableMap getRemoteExecProperties() throws EvalExce name = "modules", structField = true, doc = - "A list of all the Bazel modules in the external dependency graph, each of which is a bazel_module object that exposes all the tags it" - + " specified for this module extension. The iteration order of this dictionary is" - + " guaranteed to be the same as breadth-first search starting from the root module.") + "A list of all the Bazel modules in the external dependency graph that use this module " + + "extension, each of which is a " + + "bazel_module object that exposes all the tags it specified for this extension." + + " The iteration order of this dictionary is guaranteed to be the same as" + + " breadth-first search starting from the root module.") public StarlarkList getModules() { return modules; } @@ -130,6 +134,14 @@ public boolean isIsolated() { return extensionId.getIsolationKey().isPresent(); } + @StarlarkMethod( + name = "root_module_has_non_dev_dependency", + doc = "Whether the root module uses this extension as a non-dev dependency.", + structField = true) + public boolean rootModuleHasNonDevDependency() { + return rootModuleHasNonDevDependency; + } + @StarlarkMethod( name = "extension_metadata", doc = @@ -151,7 +163,7 @@ public boolean isIsolated() { + " disjoint.

Exactly one of root_module_direct_deps and" + " root_module_direct_dev_deps can be set to the special value" + " \"all\", which is treated as if a list with the names of" - + " allrepositories generated by the extension was specified as the value.", + + " all repositories generated by the extension was specified as the value.", positional = false, named = true, defaultValue = "None", @@ -177,7 +189,7 @@ public boolean isIsolated() { + " disjoint.

Exactly one of root_module_direct_deps and" + " root_module_direct_dev_deps can be set to the special value" + " \"all\", which is treated as if a list with the names of" - + " allrepositories generated by the extension was specified as the value.", + + " all repositories generated by the extension was specified as the value.", positional = false, named = true, defaultValue = "None", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java index 5a38c3445f5fbd..fe2d469d71fa15 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java @@ -20,6 +20,7 @@ import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; import com.google.common.collect.ImmutableSortedSet; +import com.google.common.collect.Iterables; import com.google.common.collect.Sets; import com.google.devtools.build.docgen.annot.DocCategory; import com.google.devtools.build.lib.cmdline.RepositoryName; @@ -27,7 +28,6 @@ import com.google.devtools.build.lib.events.EventHandler; import java.util.Collection; import java.util.LinkedHashSet; -import java.util.List; import java.util.Optional; import java.util.Set; import java.util.stream.Collectors; @@ -82,23 +82,11 @@ static ModuleExtensionMetadata create( // root_module_direct_dev_deps = [], but not root_module_direct_dev_deps = ["some_repo"]. if (rootModuleDirectDepsUnchecked.equals("all") && rootModuleDirectDevDepsUnchecked.equals(StarlarkList.immutableOf())) { - if (extensionId.getIsolationKey().isPresent() - && extensionId.getIsolationKey().get().isDevUsage()) { - throw Starlark.errorf( - "root_module_direct_deps must be empty for an isolated extension usage with " - + "dev_dependency = True"); - } return new ModuleExtensionMetadata(null, null, UseAllRepos.REGULAR); } if (rootModuleDirectDevDepsUnchecked.equals("all") && rootModuleDirectDepsUnchecked.equals(StarlarkList.immutableOf())) { - if (extensionId.getIsolationKey().isPresent() - && !extensionId.getIsolationKey().get().isDevUsage()) { - throw Starlark.errorf( - "root_module_direct_dev_deps must be empty for an isolated extension usage with " - + "dev_dependency = False"); - } return new ModuleExtensionMetadata(null, null, UseAllRepos.DEV); } @@ -128,20 +116,6 @@ static ModuleExtensionMetadata create( Sequence.cast( rootModuleDirectDevDepsUnchecked, String.class, "root_module_direct_dev_deps"); - if (extensionId.getIsolationKey().isPresent()) { - ModuleExtensionId.IsolationKey isolationKey = extensionId.getIsolationKey().get(); - if (isolationKey.isDevUsage() && !rootModuleDirectDeps.isEmpty()) { - throw Starlark.errorf( - "root_module_direct_deps must be empty for an isolated extension usage with " - + "dev_dependency = True"); - } - if (!isolationKey.isDevUsage() && !rootModuleDirectDevDeps.isEmpty()) { - throw Starlark.errorf( - "root_module_direct_dev_deps must be empty for an isolated extension usage with " - + "dev_dependency = False"); - } - } - Set explicitRootModuleDirectDeps = new LinkedHashSet<>(); for (String dep : rootModuleDirectDeps) { try { @@ -192,40 +166,46 @@ Optional generateFixupMessage( // expected to modify any other module's MODULE.bazel file. return Optional.empty(); } + // Every module only has at most a single usage of a given extension. + ModuleExtensionUsage rootUsage = Iterables.getOnlyElement(rootUsages); var rootModuleDirectDevDeps = getRootModuleDirectDevDeps(allRepos); var rootModuleDirectDeps = getRootModuleDirectDeps(allRepos); if (rootModuleDirectDevDeps.isEmpty() && rootModuleDirectDeps.isEmpty()) { return Optional.empty(); } - Preconditions.checkState( rootModuleDirectDevDeps.isPresent() && rootModuleDirectDeps.isPresent()); + + if (!rootUsage.getHasNonDevUseExtension() && !rootModuleDirectDeps.get().isEmpty()) { + throw Starlark.errorf( + "root_module_direct_deps must be empty if the root module contains no " + + "usages with dev_dependency = False"); + } + if (!rootUsage.getHasDevUseExtension() && !rootModuleDirectDevDeps.get().isEmpty()) { + throw Starlark.errorf( + "root_module_direct_dev_deps must be empty if the root module contains no " + + "usages with dev_dependency = True"); + } + return generateFixupMessage( - rootUsages, allRepos, rootModuleDirectDeps.get(), rootModuleDirectDevDeps.get()); + rootUsage, allRepos, rootModuleDirectDeps.get(), rootModuleDirectDevDeps.get()); } private static Optional generateFixupMessage( - List rootUsages, + ModuleExtensionUsage rootUsage, Set allRepos, Set expectedImports, Set expectedDevImports) { - var actualDevImports = - rootUsages.stream() - .flatMap(usage -> usage.getDevImports().stream()) - .collect(toImmutableSet()); + var actualDevImports = ImmutableSet.copyOf(rootUsage.getDevImports()); var actualImports = - rootUsages.stream() - .flatMap(usage -> usage.getImports().values().stream()) + rootUsage.getImports().values().stream() .filter(repo -> !actualDevImports.contains(repo)) .collect(toImmutableSet()); - // All label strings that map to the same Label are equivalent for buildozer as it implements - // the same normalization of label strings with no or empty repo name. - ModuleExtensionUsage firstUsage = rootUsages.get(0); - String extensionBzlFile = firstUsage.getExtensionBzlFile(); - String extensionName = firstUsage.getExtensionName(); - Location location = firstUsage.getLocation(); + String extensionBzlFile = rootUsage.getExtensionBzlFile(); + String extensionName = rootUsage.getExtensionName(); + Location location = rootUsage.getLocation(); var importsToAdd = ImmutableSortedSet.copyOf(Sets.difference(expectedImports, actualImports)); var importsToRemove = @@ -290,28 +270,28 @@ private static Optional generateFixupMessage( importsToAdd, extensionBzlFile, extensionName, - firstUsage.getIsolationKey()), + rootUsage.getIsolationKey()), makeUseRepoCommand( "use_repo_remove", false, importsToRemove, extensionBzlFile, extensionName, - firstUsage.getIsolationKey()), + rootUsage.getIsolationKey()), makeUseRepoCommand( "use_repo_add", true, devImportsToAdd, extensionBzlFile, extensionName, - firstUsage.getIsolationKey()), + rootUsage.getIsolationKey()), makeUseRepoCommand( "use_repo_remove", true, devImportsToRemove, extensionBzlFile, extensionName, - firstUsage.getIsolationKey())) + rootUsage.getIsolationKey())) .flatMap(Optional::stream); return Optional.of( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java index a73ee46cd47d7f..c84f87d62529f2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java @@ -68,6 +68,18 @@ public abstract class ModuleExtensionUsage { /** All the tags specified by this module for this extension. */ public abstract ImmutableList getTags(); + /** + * Whether any use_extension calls for this usage had dev_dependency = True + * set.* + */ + public abstract boolean getHasDevUseExtension(); + + /** + * Whether any use_extension calls for this usage had dev_dependency = False + * set.* + */ + public abstract boolean getHasNonDevUseExtension(); + public static Builder builder() { return new AutoValue_ModuleExtensionUsage.Builder(); } @@ -100,6 +112,10 @@ public ModuleExtensionUsage.Builder addTag(Tag value) { return this; } + public abstract Builder setHasDevUseExtension(boolean hasDevUseExtension); + + public abstract Builder setHasNonDevUseExtension(boolean hasNonDevUseExtension); + public abstract ModuleExtensionUsage build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java index f21fbda0ef044e..d2333beec073ec 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileGlobals.java @@ -515,6 +515,9 @@ class ModuleExtensionUsageBuilder { private final ImmutableSet.Builder devImports; private final ImmutableList.Builder tags; + private boolean hasNonDevUseExtension; + private boolean hasDevUseExtension; + ModuleExtensionUsageBuilder( String extensionBzlFile, String extensionName, @@ -539,6 +542,8 @@ ModuleExtensionUsage buildUsage() { .setLocation(location) .setImports(ImmutableBiMap.copyOf(imports)) .setDevImports(devImports.build()) + .setHasDevUseExtension(hasDevUseExtension) + .setHasNonDevUseExtension(hasNonDevUseExtension) .setTags(tags.build()); return builder.build(); } @@ -548,6 +553,11 @@ ModuleExtensionUsage buildUsage() { * tags with all other such proxies, thus preserving their order across dev/non-dev deps. */ ModuleExtensionProxy getProxy(boolean devDependency) { + if (devDependency) { + hasDevUseExtension = true; + } else { + hasNonDevUseExtension = true; + } return new ModuleExtensionProxy(devDependency); } 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 a3ee9af7e88468..8bb26e5a45f26a 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 @@ -400,6 +400,9 @@ private ModuleExtensionContext createContext( throw new SingleExtensionEvalFunctionException(e, Transience.PERSISTENT); } } + ModuleExtensionUsage rootUsage = usagesValue.getExtensionUsages().get(ModuleKey.ROOT); + boolean rootModuleHasNonDevDependency = + rootUsage != null && rootUsage.getHasNonDevUseExtension(); return new ModuleExtensionContext( workingDirectory, env, @@ -410,7 +413,8 @@ private ModuleExtensionContext createContext( starlarkSemantics, repositoryRemoteExecutor, extensionId, - StarlarkList.immutableCopyOf(modules)); + StarlarkList.immutableCopyOf(modules), + rootModuleHasNonDevDependency); } static final class SingleExtensionEvalFunctionException extends SkyFunctionException { diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java index d4c4f32b46225f..b8602dba90d16e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunctionTest.java @@ -219,6 +219,8 @@ private static ModuleExtensionUsage createModuleExtensionUsage( .setDevImports(ImmutableSet.of()) .setUsingModule(ModuleKey.ROOT) .setLocation(Location.BUILTIN) + .setHasDevUseExtension(false) + .setHasNonDevUseExtension(true) .build(); } 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 10f86b92dbbc12..6012c2392428f0 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 @@ -1691,9 +1691,9 @@ public void extensionMetadata_duplicateRepoAcrossTypes() throws Exception { } @Test - public void extensionMetadata_isolate_devUsageWithAllDirectNonDevDeps() throws Exception { + public void extensionMetadata_devUsageWithAllDirectNonDevDeps() throws Exception { var result = - evaluateIsolatedModuleExtension( + evaluateSimpleModuleExtension( "return" + " ctx.extension_metadata(root_module_direct_deps=\"all\"," + "root_module_direct_dev_deps=[])", @@ -1701,14 +1701,14 @@ public void extensionMetadata_isolate_devUsageWithAllDirectNonDevDeps() throws E assertThat(result.hasError()).isTrue(); assertContainsEvent( - "root_module_direct_deps must be empty for an isolated extension usage with dev_dependency" - + " = True"); + "root_module_direct_deps must be empty if the root module contains no usages with " + + "dev_dependency = False"); } @Test - public void extensionMetadata_isolate_nonDevUsageWithAllDirectDevDeps() throws Exception { + public void extensionMetadata_nonDevUsageWithAllDirectDevDeps() throws Exception { var result = - evaluateIsolatedModuleExtension( + evaluateSimpleModuleExtension( "return" + " ctx.extension_metadata(root_module_direct_deps=[]," + "root_module_direct_dev_deps=\"all\")", @@ -1716,14 +1716,14 @@ public void extensionMetadata_isolate_nonDevUsageWithAllDirectDevDeps() throws E assertThat(result.hasError()).isTrue(); assertContainsEvent( - "root_module_direct_dev_deps must be empty for an isolated extension usage with " - + "dev_dependency = False"); + "root_module_direct_dev_deps must be empty if the root module contains no usages with " + + "dev_dependency = True"); } @Test - public void extensionMetadata_isolate_devUsageWithDirectNonDevDeps() throws Exception { + public void extensionMetadata_devUsageWithDirectNonDevDeps() throws Exception { var result = - evaluateIsolatedModuleExtension( + evaluateSimpleModuleExtension( "return" + " ctx.extension_metadata(root_module_direct_deps=['dep1']," + "root_module_direct_dev_deps=['dep2'])", @@ -1731,14 +1731,14 @@ public void extensionMetadata_isolate_devUsageWithDirectNonDevDeps() throws Exce assertThat(result.hasError()).isTrue(); assertContainsEvent( - "root_module_direct_deps must be empty for an isolated extension usage with dev_dependency" - + " = True"); + "root_module_direct_deps must be empty if the root module contains no usages with " + + "dev_dependency = False"); } @Test - public void extensionMetadata_isolate_nonDevUsageWithDirectDevDeps() throws Exception { + public void extensionMetadata_nonDevUsageWithDirectDevDeps() throws Exception { var result = - evaluateIsolatedModuleExtension( + evaluateSimpleModuleExtension( "return" + " ctx.extension_metadata(root_module_direct_deps=['dep1']," + "root_module_direct_dev_deps=['dep2'])", @@ -1746,8 +1746,8 @@ public void extensionMetadata_isolate_nonDevUsageWithDirectDevDeps() throws Exce assertThat(result.hasError()).isTrue(); assertContainsEvent( - "root_module_direct_dev_deps must be empty for an isolated extension usage with " - + "dev_dependency = False"); + "root_module_direct_dev_deps must be empty if the root module contains no usages with " + + "dev_dependency = True"); } @Test @@ -2214,51 +2214,75 @@ public void extensionMetadata_isolatedDev() throws Exception { ImmutableSet.of(EventKind.WARNING)); } - private EvaluationResult evaluateIsolatedModuleExtension( + private EvaluationResult evaluateSimpleModuleExtension( + String returnStatement) throws Exception { + return evaluateSimpleModuleExtension(returnStatement, /* devDependency= */ false); + } + + private EvaluationResult evaluateSimpleModuleExtension( String returnStatement, boolean devDependency) throws Exception { String devDependencyStr = devDependency ? "True" : "False"; - String isolateStr = "True"; scratch.file( workspaceRoot.getRelative("MODULE.bazel").getPathString(), String.format( - "ext = use_extension('//:defs.bzl', 'ext', dev_dependency = %s, isolate = %s)", - devDependencyStr, isolateStr), - // Isolated module extensions without repo imports result in an error in - // ModuleFileFunction. - "use_repo(ext, 'some_repo')"); + "ext = use_extension('//:defs.bzl', 'ext', dev_dependency = %s)", devDependencyStr)); scratch.file( workspaceRoot.getRelative("defs.bzl").getPathString(), + "repo = repository_rule(lambda ctx: True)", "def _ext_impl(ctx):", + " repo(name = 'dep1')", + " repo(name = 'dep2')", " " + returnStatement, "ext = module_extension(implementation=_ext_impl)"); scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); ModuleExtensionId extensionId = - ModuleExtensionId.create( - Label.parseCanonical("//:defs.bzl"), - "ext", - Optional.of(ModuleExtensionId.IsolationKey.create(ModuleKey.ROOT, devDependency, 0))); + ModuleExtensionId.create(Label.parseCanonical("//:defs.bzl"), "ext", Optional.empty()); reporter.removeHandler(failFastHandler); return evaluator.evaluate( ImmutableList.of(SingleExtensionEvalValue.key(extensionId)), evaluationContext); } - private EvaluationResult evaluateSimpleModuleExtension( - String returnStatement) throws Exception { + @Test + public void isDevDependency_usages() throws Exception { scratch.file( workspaceRoot.getRelative("MODULE.bazel").getPathString(), - "ext = use_extension('//:defs.bzl', 'ext')"); + "module(name='root',version='1.0')", + "bazel_dep(name='data_repo',version='1.0')", + "ext1 = use_extension('//:defs.bzl','ext1')", + "use_repo(ext1,ext1_repo='ext_repo')", + "ext2 = use_extension('//:defs.bzl','ext2',dev_dependency=True)", + "use_repo(ext2,ext2_repo='ext_repo')", + "ext3a = use_extension('//:defs.bzl','ext3')", + "use_repo(ext3a,ext3_repo='ext_repo')", + "ext3b = use_extension('//:defs.bzl','ext3',dev_dependency=True)"); + scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + scratch.file( + workspaceRoot.getRelative("data.bzl").getPathString(), + "load('@ext1_repo//:data.bzl', _ext1_data='data')", + "load('@ext2_repo//:data.bzl', _ext2_data='data')", + "load('@ext3_repo//:data.bzl', _ext3_data='data')", + "ext1_data=_ext1_data", + "ext2_data=_ext2_data", + "ext3_data=_ext3_data"); scratch.file( workspaceRoot.getRelative("defs.bzl").getPathString(), - "def _ext_impl(ctx):", - " " + returnStatement, - "ext = module_extension(implementation=_ext_impl)"); - scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + "load('@data_repo//:defs.bzl','data_repo')", + "def _ext_impl(id,ctx):", + " data_str = id + ': ' + str(ctx.root_module_has_non_dev_dependency)", + " data_repo(name='ext_repo',data=data_str)", + "ext1=module_extension(implementation=lambda ctx: _ext_impl('ext1', ctx))", + "ext2=module_extension(implementation=lambda ctx: _ext_impl('ext2', ctx))", + "ext3=module_extension(implementation=lambda ctx: _ext_impl('ext3', ctx))"); - ModuleExtensionId extensionId = - ModuleExtensionId.create(Label.parseCanonical("//:defs.bzl"), "ext", Optional.empty()); - reporter.removeHandler(failFastHandler); - return evaluator.evaluate( - ImmutableList.of(SingleExtensionEvalValue.key(extensionId)), evaluationContext); + SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl")); + EvaluationResult result = + evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); + if (result.hasError()) { + throw result.getError().getException(); + } + assertThat(result.get(skyKey).getModule().getGlobal("ext1_data")).isEqualTo("ext1: True"); + assertThat(result.get(skyKey).getModule().getGlobal("ext2_data")).isEqualTo("ext2: False"); + assertThat(result.get(skyKey).getModule().getGlobal("ext3_data")).isEqualTo("ext3: True"); } } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java index 52dddf54eea8dc..f265942feb76b4 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunctionTest.java @@ -487,6 +487,8 @@ public void testModuleExtensions_good() throws Exception { "fake:0/modules/mymod/1.0/MODULE.bazel", 2, 23)) .setImports(ImmutableBiMap.of("repo1", "repo1")) .setDevImports(ImmutableSet.of()) + .setHasDevUseExtension(false) + .setHasNonDevUseExtension(true) .addTag( Tag.builder() .setTagName("tag") @@ -512,6 +514,8 @@ public void testModuleExtensions_good() throws Exception { "fake:0/modules/mymod/1.0/MODULE.bazel", 5, 23)) .setImports(ImmutableBiMap.of("other_repo1", "repo1", "repo2", "repo2")) .setDevImports(ImmutableSet.of()) + .setHasDevUseExtension(false) + .setHasNonDevUseExtension(true) .addTag( Tag.builder() .setTagName("tag1") @@ -551,6 +555,8 @@ public void testModuleExtensions_good() throws Exception { .setImports( ImmutableBiMap.of("mvn", "maven", "junit", "junit", "guava", "guava")) .setDevImports(ImmutableSet.of()) + .setHasDevUseExtension(false) + .setHasNonDevUseExtension(true) .addTag( Tag.builder() .setTagName("dep") @@ -622,6 +628,8 @@ public void testModuleExtensions_duplicateProxy_asRoot() throws Exception { "alpha", "alpha", "beta", "beta", "gamma", "gamma", "delta", "delta")) .setDevImports(ImmutableSet.of("alpha", "gamma")) + .setHasDevUseExtension(true) + .setHasNonDevUseExtension(true) .addTag( Tag.builder() .setTagName("tag") @@ -719,6 +727,8 @@ public void testModuleExtensions_duplicateProxy_asDep() throws Exception { "fake:0/modules/mymod/1.0/MODULE.bazel", 5, 23)) .setImports(ImmutableBiMap.of("beta", "beta", "delta", "delta")) .setDevImports(ImmutableSet.of()) + .setHasDevUseExtension(false) + .setHasNonDevUseExtension(true) .addTag( Tag.builder() .setTagName("tag") diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java index 5782eaa2a96461..f20e7c092d8c62 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/StarlarkBazelModuleTest.java @@ -50,7 +50,9 @@ private static ModuleExtensionUsage.Builder getBaseUsageBuilder() { .setUsingModule(ModuleKey.ROOT) .setLocation(Location.BUILTIN) .setImports(ImmutableBiMap.of()) - .setDevImports(ImmutableSet.of()); + .setDevImports(ImmutableSet.of()) + .setHasDevUseExtension(false) + .setHasNonDevUseExtension(true); } /** A builder for ModuleExtension that sets all the mandatory but irrelevant fields. */