diff --git a/site/en/external/extension.md b/site/en/external/extension.md index 0d973ab830fcac..67ea7fd7d5b54b 100644 --- a/site/en/external/extension.md +++ b/site/en/external/extension.md @@ -176,6 +176,63 @@ several repo visibility rules: the repo visible to the module instead of the extension-generated repo of the same name. +### Overriding and injecting module extension repos + +The root module can use +[`override_repo`](/rules/lib/globals/module#override_repo) and +[`inject_repo`](/rules/lib/globals/module#inject_repo) to override or inject +module extension repos. + +#### Example: Replacing `rules_java`'s `java_tools` with a vendored copy + +```python +# MODULE.bazel +local_repository = use_repo_rule("@bazel_tools//tools/build_defs/repo:local.bzl", "local_repository") +local_repository( + name = "my_java_tools", + path = "vendor/java_tools", +) + +bazel_dep(name = "rules_java", version = "7.11.1") +java_toolchains = use_extension("@rules_java//java:extension.bzl", "toolchains") + +override_repo(java_toolchains, remote_java_tools = "my_java_tools") +``` + +#### Example: Patch a Go dependency to depend on `@zlib` instead of the system zlib + +```python +# MODULE.bazel +bazel_dep(name = "gazelle", version = "0.38.0") +bazel_dep(name = "zlib", version = "1.3.1.bcr.3") + +go_deps = use_extension("@gazelle//:extensions.bzl", "go_deps") +go_deps.from_file(go_mod = "//:go.mod") +go_deps.module_override( + patches = [ + "//patches:my_module_zlib.patch", + ], + path = "example.com/my_module", +) +use_repo(go_deps, ...) + +inject_repo(go_deps, "zlib") +``` + +```diff +# patches/my_module_zlib.patch +--- a/BUILD.bazel ++++ b/BUILD.bazel +@@ -1,6 +1,6 @@ + go_binary( + name = "my_module", + importpath = "example.com/my_module", + srcs = ["my_module.go"], +- copts = ["-lz"], ++ cdeps = ["@zlib"], + ) +``` + ## Best practices This section describes best practices when writing extensions so they are diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java index 7faf5a17e796fb..a04e21e1f6686d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphFunction.java @@ -79,15 +79,24 @@ public SkyValue compute(SkyKey skyKey, Environment env) ImmutableBiMap extensionUniqueNames = calculateUniqueNameForUsedExtensionId(extensionUsagesById, starlarkSemantics); + char repoNameSeparator = + starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES) + ? '+' + : '~'; + return BazelDepGraphValue.create( depGraph, canonicalRepoNameLookup, depGraph.values().stream().map(AbridgedModule::from).collect(toImmutableList()), extensionUsagesById, extensionUniqueNames.inverse(), - starlarkSemantics.getBool(BuildLanguageOptions.INCOMPATIBLE_USE_PLUS_IN_REPO_NAMES) - ? '+' - : '~'); + resolveRepoOverrides( + depGraph, + extensionUsagesById, + extensionUniqueNames.inverse(), + canonicalRepoNameLookup, + repoNameSeparator), + repoNameSeparator); } private static ImmutableTable @@ -218,6 +227,40 @@ private static String makeUniqueNameCandidate( + extensionNameDisambiguator); } + private static ImmutableTable resolveRepoOverrides( + ImmutableMap depGraph, + ImmutableTable extensionUsagesTable, + ImmutableMap extensionUniqueNames, + ImmutableBiMap canonicalRepoNameLookup, + char repoNameSeparator) { + RepositoryMapping rootModuleMappingWithoutOverrides = + BazelDepGraphValue.getRepositoryMapping( + ModuleKey.ROOT, + depGraph, + extensionUsagesTable, + extensionUniqueNames, + canonicalRepoNameLookup, + // ModuleFileFunction ensures that repos that override other repos are not themselves + // overridden, so we can safely pass an empty table here instead of resolving chains + // of overrides. + ImmutableTable.of(), + repoNameSeparator); + ImmutableTable.Builder repoOverridesBuilder = + ImmutableTable.builder(); + for (var extensionId : extensionUsagesTable.rowKeySet()) { + var rootUsage = extensionUsagesTable.row(extensionId).get(ModuleKey.ROOT); + if (rootUsage != null) { + for (var override : rootUsage.getRepoOverrides().entrySet()) { + repoOverridesBuilder.put( + extensionId, + override.getKey(), + rootModuleMappingWithoutOverrides.get(override.getValue().overridingRepoName())); + } + } + } + return repoOverridesBuilder.buildOrThrow(); + } + static class BazelDepGraphFunctionException extends SkyFunctionException { BazelDepGraphFunctionException(ExternalDepsException e, Transience transience) { super(e, transience); diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java index 86217750a73459..1015784e659ed2 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelDepGraphValue.java @@ -43,6 +43,7 @@ public static BazelDepGraphValue create( ImmutableList abridgedModules, ImmutableTable extensionUsagesTable, ImmutableMap extensionUniqueNames, + ImmutableTable repoOverrides, char repoNameSeparator) { return new AutoValue_BazelDepGraphValue( depGraph, @@ -50,6 +51,7 @@ public static BazelDepGraphValue create( abridgedModules, extensionUsagesTable, extensionUniqueNames, + repoOverrides, repoNameSeparator); } @@ -75,6 +77,7 @@ public static BazelDepGraphValue createEmptyDepGraph() { ImmutableList.of(), ImmutableTable.of(), ImmutableMap.of(), + ImmutableTable.of(), '+'); } @@ -107,6 +110,12 @@ public static BazelDepGraphValue createEmptyDepGraph() { */ public abstract ImmutableMap getExtensionUniqueNames(); + /** + * For each module extension, a mapping from the name of the repo exported by the extension to the + * canonical name of the repo that should override it (if any). + */ + public abstract ImmutableTable getRepoOverrides(); + /** The character to use to separate the different segments of a canonical repo name. */ public abstract char getRepoNameSeparator(); @@ -115,22 +124,45 @@ public static BazelDepGraphValue createEmptyDepGraph() { * module deps and module extensions. */ public final RepositoryMapping getFullRepoMapping(ModuleKey key) { + return getRepositoryMapping( + key, + getDepGraph(), + getExtensionUsagesTable(), + getExtensionUniqueNames(), + getCanonicalRepoNameLookup(), + getRepoOverrides(), + getRepoNameSeparator()); + } + + static RepositoryMapping getRepositoryMapping( + ModuleKey key, + ImmutableMap depGraph, + ImmutableTable extensionUsagesTable, + ImmutableMap extensionUniqueNames, + ImmutableBiMap canonicalRepoNameLookup, + ImmutableTable repoOverrides, + char repoNameSeparator) { ImmutableMap.Builder mapping = ImmutableMap.builder(); for (Map.Entry extIdAndUsage : - getExtensionUsagesTable().column(key).entrySet()) { + extensionUsagesTable.column(key).entrySet()) { ModuleExtensionId extensionId = extIdAndUsage.getKey(); ModuleExtensionUsage usage = extIdAndUsage.getValue(); - String repoNamePrefix = getExtensionUniqueNames().get(extensionId) + getRepoNameSeparator(); + String repoNamePrefix = extensionUniqueNames.get(extensionId) + repoNameSeparator; for (ModuleExtensionUsage.Proxy proxy : usage.getProxies()) { for (Map.Entry entry : proxy.getImports().entrySet()) { - String canonicalRepoName = repoNamePrefix + entry.getValue(); - mapping.put(entry.getKey(), RepositoryName.createUnvalidated(canonicalRepoName)); + RepositoryName defaultCanonicalRepoName = + RepositoryName.createUnvalidated(repoNamePrefix + entry.getValue()); + mapping.put( + entry.getKey(), + repoOverrides + .row(extensionId) + .getOrDefault(entry.getValue(), defaultCanonicalRepoName)); } } } - return getDepGraph() + return depGraph .get(key) - .getRepoMappingWithBazelDepsOnly(getCanonicalRepoNameLookup().inverse()) + .getRepoMappingWithBazelDepsOnly(canonicalRepoNameLookup.inverse()) .withAdditionalMappings(mapping.buildOrThrow()); } } 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 2509f78a983748..4768ea4a14685a 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 @@ -66,6 +66,7 @@ record RepoRuleCall( private final String repoPrefix; private final PackageIdentifier basePackageId; private final RepositoryMapping baseRepoMapping; + private final ImmutableMap repoOverrides; private final BlazeDirectories directories; private final ExtendedEventHandler eventHandler; private final Map deferredRepos = new LinkedHashMap<>(); @@ -75,12 +76,14 @@ public ModuleExtensionEvalStarlarkThreadContext( String repoPrefix, PackageIdentifier basePackageId, RepositoryMapping baseRepoMapping, + ImmutableMap repoOverrides, BlazeDirectories directories, ExtendedEventHandler eventHandler) { this.extensionId = extensionId; this.repoPrefix = repoPrefix; this.basePackageId = basePackageId; this.baseRepoMapping = baseRepoMapping; + this.repoOverrides = repoOverrides; this.directories = directories; this.eventHandler = eventHandler; } @@ -127,13 +130,15 @@ public ImmutableMap createRepos(StarlarkSemantics starlarkSema // 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. - var extensionRepos = + ImmutableMap.Builder entries = ImmutableMap.builder(); + entries.putAll(baseRepoMapping.entries()); + entries.putAll( Maps.asMap( deferredRepos.keySet(), - apparentName -> RepositoryName.createUnvalidated(repoPrefix + apparentName)); + apparentName -> RepositoryName.createUnvalidated(repoPrefix + apparentName))); + entries.putAll(repoOverrides); RepositoryMapping fullRepoMapping = - RepositoryMapping.create(extensionRepos, baseRepoMapping.ownerRepo()) - .withAdditionalMappings(baseRepoMapping); + RepositoryMapping.create(entries.buildKeepingLast(), baseRepoMapping.ownerRepo()); // LINT.ThenChange(//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionRepoMappingEntriesFunction.java) ImmutableMap.Builder repoSpecs = ImmutableMap.builder(); 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 9253d38813ea04..8e400bc37093cb 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 @@ -74,6 +74,7 @@ private ModuleExtensionRepoMappingEntriesValue computeRepoMappingEntries( ImmutableMap.Builder entries = ImmutableMap.builder(); entries.putAll(bazelDepGraphValue.getFullRepoMapping(moduleKey).entries()); entries.putAll(extensionEvalValue.getCanonicalRepoNameToInternalNames().inverse()); + entries.putAll(bazelDepGraphValue.getRepoOverrides().row(extensionId)); 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/ModuleExtensionUsage.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionUsage.java index e5dc76f120e9fa..f4fc11683fe970 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 @@ -19,6 +19,7 @@ import com.google.auto.value.AutoValue; import com.google.common.collect.ImmutableBiMap; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.errorprone.annotations.CanIgnoreReturnValue; import com.ryanharter.auto.value.gson.GenerateTypeAdapter; @@ -130,6 +131,24 @@ public final boolean getHasNonDevUseExtension() { return getProxies().stream().anyMatch(p -> !p.isDevDependency()); } + /** + * Represents a repo that overrides another repo within the scope of the extension. + * + * @param overridingRepoName The apparent name of the overriding repo in the root module. + * @param mustExist Whether this override should apply to an existing repo. + * @param location The location of the {@code override_repo} or {@code inject_repo} call. + */ + @GenerateTypeAdapter + public record RepoOverride(String overridingRepoName, boolean mustExist, Location location) {} + + /** + * Contains information about overrides that apply to repos generated by this extension. Keyed by + * the extension-local repo name. + * + *

This is only non-empty for root module usages. + */ + public abstract ImmutableMap getRepoOverrides(); + public abstract Builder toBuilder(); public static Builder builder() { @@ -152,6 +171,11 @@ ModuleExtensionUsage trimForEvaluation() { // Extension implementation functions do not see the imports, they are only validated // against the set of generated repos in a validation step that comes afterward. .setProxies(ImmutableList.of()) + // Tracked in SingleExtensionUsagesValue instead, using canonical instead of apparent names. + // Whether this override must apply to an existing repo as well as its source location also + // don't influence the evaluation of the extension as they are checked in + // SingleExtensionFunction. + .setRepoOverrides(ImmutableMap.of()) .build(); } @@ -185,6 +209,9 @@ public Builder addTag(Tag value) { return this; } + @CanIgnoreReturnValue + public abstract Builder setRepoOverrides(ImmutableMap repoOverrides); + public abstract ModuleExtensionUsage build(); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java index fa8f303938ada1..7d1c977074a486 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleFileFunction.java @@ -435,7 +435,7 @@ public static RootModuleFileValue evaluateRootModuleFile( try { module = moduleThreadContext.buildModule(/* registry= */ null); } catch (EvalException e) { - eventHandler.handle(Event.error(e.getMessageWithStack())); + eventHandler.handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack())); throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for the root module"); } for (ModuleExtensionUsage usage : module.getExtensionUsages()) { @@ -521,7 +521,7 @@ private static ModuleThreadContext execModuleFile( }); compiledRootModuleFile.runOnThread(thread); } catch (EvalException e) { - eventHandler.handle(Event.error(e.getMessageWithStack())); + eventHandler.handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack())); throw errorf(Code.BAD_MODULE, "error executing MODULE.bazel file for %s", moduleKey); } return context; 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 b3371ae77ca6e3..028e4cbdb69f7f 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 @@ -15,7 +15,6 @@ package com.google.devtools.build.lib.bazel.bzlmod; - import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableCollection; import com.google.common.collect.ImmutableList; @@ -49,7 +48,6 @@ import net.starlark.java.eval.StarlarkValue; import net.starlark.java.eval.Structure; import net.starlark.java.eval.Tuple; -import net.starlark.java.syntax.Location; /** A collection of global Starlark build API functions that apply to MODULE.bazel files. */ @GlobalMethods(environment = Environment.MODULE) @@ -171,11 +169,10 @@ public void module( } if (repoName.isEmpty()) { repoName = name; - context.addRepoNameUsage(name, "as the current module name", thread.getCallerLocation()); + context.addRepoNameUsage(name, "as the current module name", thread.getCallStack()); } else { RepositoryName.validateUserProvidedRepoName(repoName); - context.addRepoNameUsage( - repoName, "as the module's own repo name", thread.getCallerLocation()); + context.addRepoNameUsage(repoName, "as the module's own repo name", thread.getCallStack()); } Version parsedVersion; try { @@ -293,7 +290,7 @@ public void bazelDep( name, parsedVersion, maxCompatibilityLevel.toInt("max_compatibility_level"))); } - context.addRepoNameUsage(repoName, "by a bazel_dep", thread.getCallerLocation()); + context.addRepoNameUsage(repoName, "by a bazel_dep", thread.getCallStack()); } @StarlarkMethod( @@ -525,12 +522,25 @@ static class ModuleExtensionProxy implements Structure, StarlarkExportable { usageBuilder.addProxyBuilder(proxyBuilder); } - void addImport(String localRepoName, String exportedName, String byWhat, Location location) + void addImport( + String localRepoName, + String exportedName, + String byWhat, + ImmutableList stack) throws EvalException { - usageBuilder.addImport(localRepoName, exportedName, byWhat, location); + usageBuilder.addImport(localRepoName, exportedName, byWhat, stack); proxyBuilder.addImport(localRepoName, exportedName); } + void addOverride( + String overriddenRepoName, + String overridingRepoName, + boolean mustExist, + ImmutableList stack) + throws EvalException { + usageBuilder.addRepoOverride(overriddenRepoName, overridingRepoName, mustExist, stack); + } + class TagCallable implements StarlarkValue { final String tagName; @@ -610,13 +620,120 @@ public void useRepo( throws EvalException { ModuleThreadContext context = ModuleThreadContext.fromOrFail(thread, "use_repo()"); context.setNonModuleCalled(); - Location location = thread.getCallerLocation(); + ImmutableList stack = thread.getCallStack(); + for (String arg : Sequence.cast(args, String.class, "args")) { + extensionProxy.addImport(arg, arg, "by a use_repo() call", stack); + } + for (Map.Entry entry : + Dict.cast(kwargs, String.class, String.class, "kwargs").entrySet()) { + extensionProxy.addImport(entry.getKey(), entry.getValue(), "by a use_repo() call", stack); + } + } + + @StarlarkMethod( + name = "override_repo", + doc = + """ + Overrides one or more repos defined by the given module extension with the given repos + visible to the current module. This is ignored if the current module is not the root + module or `--ignore_dev_dependency` is enabled. + +

Use inject_repo instead to add a new repo. + """, + parameters = { + @Param( + name = "extension_proxy", + doc = "A module extension proxy object returned by a use_extension call."), + }, + extraPositionals = + @Param( + name = "args", + doc = + """ + The repos in the extension that should be overridden with the repos of the same + name in the current module."""), + extraKeywords = + @Param( + name = "kwargs", + doc = + """ + The overrides to apply to the repos generated by the extension, where the values + are the names of repos in the scope of the current module and the keys are the + names of the repos they will override in the extension."""), + useStarlarkThread = true) + public void overrideRepo( + ModuleExtensionProxy extensionProxy, + Tuple args, + Dict kwargs, + StarlarkThread thread) + throws EvalException { + ModuleThreadContext context = ModuleThreadContext.fromOrFail(thread, "override_repo()"); + context.setNonModuleCalled(); + if (context.shouldIgnoreDevDeps()) { + // Ignore calls early as they may refer to repos that are dev dependencies (or this is not the + // root module). + return; + } + ImmutableList stack = thread.getCallStack(); + for (String arg : Sequence.cast(args, String.class, "args")) { + extensionProxy.addOverride(arg, arg, /* mustExist= */ true, stack); + } + for (Map.Entry entry : + Dict.cast(kwargs, String.class, String.class, "kwargs").entrySet()) { + extensionProxy.addOverride(entry.getKey(), entry.getValue(), /* mustExist= */ true, stack); + } + } + + @StarlarkMethod( + name = "inject_repo", + doc = + """ + Injects one or more new repos into the given module extension. + This is ignored if the current module is not the root module or `--ignore_dev_dependency` + is enabled. +

Use override_repo instead to override an + existing repo.""", + parameters = { + @Param( + name = "extension_proxy", + doc = "A module extension proxy object returned by a use_extension call."), + }, + extraPositionals = + @Param( + name = "args", + doc = + """ + The repos visible to the current module that should be injected into the + extension under the same name."""), + extraKeywords = + @Param( + name = "kwargs", + doc = + """ + The new repos to inject into the extension, where the values are the names of + repos in the scope of the current module and the keys are the name they will be + visible under in the extension."""), + useStarlarkThread = true) + public void injectRepo( + ModuleExtensionProxy extensionProxy, + Tuple args, + Dict kwargs, + StarlarkThread thread) + throws EvalException { + ModuleThreadContext context = ModuleThreadContext.fromOrFail(thread, "inject_repo()"); + context.setNonModuleCalled(); + if (context.shouldIgnoreDevDeps()) { + // Ignore calls early as they may refer to repos that are dev dependencies (or this is not the + // root module). + return; + } + ImmutableList stack = thread.getCallStack(); for (String arg : Sequence.cast(args, String.class, "args")) { - extensionProxy.addImport(arg, arg, "by a use_repo() call", location); + extensionProxy.addOverride(arg, arg, /* mustExist= */ false, stack); } for (Map.Entry entry : Dict.cast(kwargs, String.class, String.class, "kwargs").entrySet()) { - extensionProxy.addImport(entry.getKey(), entry.getValue(), "by a use_repo() call", location); + extensionProxy.addOverride(entry.getKey(), entry.getValue(), /* mustExist= */ false, stack); } } @@ -697,7 +814,7 @@ public void call( .setContainingModuleFilePath( usageBuilder.getContext().getCurrentModuleFilePath())); extensionProxy.getValue(tagName).call(kwargs, thread); - extensionProxy.addImport(name, name, "by a repo rule", thread.getCallerLocation()); + extensionProxy.addImport(name, name, "by a repo rule", thread.getCallStack()); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java index cdfeb6622bc7ee..06231cc7c502db 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleThreadContext.java @@ -21,6 +21,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.common.collect.Iterables; +import com.google.common.collect.Maps; import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelConstants; @@ -53,6 +54,9 @@ public class ModuleThreadContext { private final Map overrides = new LinkedHashMap<>(); private final Map repoNameUsages = new HashMap<>(); + private final Map overriddenRepos = new HashMap<>(); + private final Map overridingRepos = new HashMap<>(); + public static ModuleThreadContext fromOrFail(StarlarkThread thread, String what) throws EvalException { ModuleThreadContext context = thread.getThreadLocal(ModuleThreadContext.class); @@ -77,14 +81,33 @@ public ModuleThreadContext( this.includeLabelToCompiledModuleFile = includeLabelToCompiledModuleFile; } - record RepoNameUsage(String how, Location where) {} + record RepoOverride( + String overriddenRepoName, + String overridingRepoName, + boolean mustExist, + String extensionName, + ImmutableList stack) { + Location location() { + // Skip over the override_repo builtin frame. + return stack.reverse().get(1).location; + } + } + + record RepoNameUsage(String how, ImmutableList stack) { + Location location() { + // Skip over the override_repo builtin frame. + return stack.reverse().get(1).location; + } + } - public void addRepoNameUsage(String repoName, String how, Location where) throws EvalException { - RepoNameUsage collision = repoNameUsages.put(repoName, new RepoNameUsage(how, where)); + public void addRepoNameUsage( + String repoName, String how, ImmutableList stack) + throws EvalException { + RepoNameUsage collision = repoNameUsages.put(repoName, new RepoNameUsage(how, stack)); if (collision != null) { throw Starlark.errorf( "The repo name '%s' is already being used %s at %s", - repoName, collision.how(), collision.where()); + repoName, collision.how(), collision.location()); } } @@ -123,12 +146,14 @@ List getExtensionUsageBuilders() { } static class ModuleExtensionUsageBuilder { + private final ModuleThreadContext context; private final String extensionBzlFile; private final String extensionName; private final boolean isolate; private final ArrayList proxyBuilders; private final HashBiMap imports; + private final Map repoOverrides; private final ImmutableList.Builder tags; ModuleExtensionUsageBuilder( @@ -142,6 +167,7 @@ static class ModuleExtensionUsageBuilder { this.isolate = isolate; this.proxyBuilders = new ArrayList<>(); this.imports = HashBiMap.create(); + this.repoOverrides = new HashMap<>(); this.tags = ImmutableList.builder(); } @@ -163,20 +189,41 @@ void addImport( String localRepoName, String exportedName, String byWhat, - Location location) + ImmutableList stack) throws EvalException { RepositoryName.validateUserProvidedRepoName(localRepoName); RepositoryName.validateUserProvidedRepoName(exportedName); - context.addRepoNameUsage(localRepoName, byWhat, location); + context.addRepoNameUsage(localRepoName, byWhat, stack); if (imports.containsValue(exportedName)) { String collisionRepoName = imports.inverse().get(exportedName); throw Starlark.errorf( "The repo exported as '%s' by module extension '%s' is already imported at %s", - exportedName, extensionName, context.repoNameUsages.get(collisionRepoName).where()); + exportedName, extensionName, context.repoNameUsages.get(collisionRepoName).location()); } imports.put(localRepoName, exportedName); } + public void addRepoOverride( + String overriddenRepoName, + String overridingRepoName, + boolean mustExist, + ImmutableList stack) + throws EvalException { + RepositoryName.validateUserProvidedRepoName(overriddenRepoName); + RepositoryName.validateUserProvidedRepoName(overridingRepoName); + RepoOverride collision = + repoOverrides.put( + overriddenRepoName, + new RepoOverride( + overriddenRepoName, overridingRepoName, mustExist, extensionName, stack)); + if (collision != null) { + throw Starlark.errorf( + "The repo exported as '%s' by module extension '%s' is already overridden with '%s' at" + + " %s", + overriddenRepoName, extensionName, collision.overridingRepoName, collision.location()); + } + } + void addTag(Tag tag) { tags.add(tag); } @@ -203,6 +250,41 @@ ModuleExtensionUsage buildUsage() throws EvalException { } else { builder.setIsolationKey(Optional.empty()); } + + for (var override : repoOverrides.entrySet()) { + String overriddenRepoName = override.getKey(); + String overridingRepoName = override.getValue().overridingRepoName; + if (!context.repoNameUsages.containsKey(overridingRepoName)) { + throw Starlark.errorf( + "The repo exported as '%s' by module extension '%s' is overridden with '%s', but" + + " no repo is visible under this name", + overriddenRepoName, extensionName, overridingRepoName) + .withCallStack(override.getValue().stack); + } + String importedAs = imports.inverse().get(overriddenRepoName); + if (importedAs != null) { + if (!override.getValue().mustExist) { + throw Starlark.errorf( + "Cannot import repo '%s' that has been injected into module extension '%s' at" + + " %s. Please refer to @%s directly.", + overriddenRepoName, + extensionName, + override.getValue().location(), + overridingRepoName) + .withCallStack(context.repoNameUsages.get(importedAs).stack); + } + context.overriddenRepos.put(importedAs, override.getValue()); + } + context.overridingRepos.put(overridingRepoName, override.getValue()); + } + builder.setRepoOverrides( + ImmutableMap.copyOf( + Maps.transformValues( + repoOverrides, + v -> + new ModuleExtensionUsage.RepoOverride( + v.overridingRepoName, v.mustExist, v.location())))); + return builder.build(); } } @@ -249,7 +331,7 @@ public InterimModule buildModule(@Nullable Registry registry) throws EvalExcepti } deps.put(builtinModule, DepSpec.create(builtinModule, Version.EMPTY, -1)); try { - addRepoNameUsage(builtinModule, "as a built-in dependency", Location.BUILTIN); + addRepoNameUsage(builtinModule, "as a built-in dependency", ImmutableList.of()); } catch (EvalException e) { throw new EvalException( e.getMessage() @@ -269,6 +351,24 @@ public InterimModule buildModule(@Nullable Registry registry) throws EvalExcepti } extensionUsages.add(extensionUsageBuilder.buildUsage()); } + // A repo cannot be both overriding and overridden. This ensures that repo overrides can be + // applied to repo mappings in a single step (and also prevents cycles). + Optional overridingAndOverridden = + overridingRepos.keySet().stream().filter(overriddenRepos::containsKey).findFirst(); + if (overridingAndOverridden.isPresent()) { + var override = overridingRepos.get(overridingAndOverridden.get()); + var overrideOnOverride = overriddenRepos.get(overridingAndOverridden.get()); + throw Starlark.errorf( + "The repo '%s' used as an override for '%s' in module extension '%s' is itself" + + " overridden with '%s' at %s, which is not supported.", + override.overridingRepoName, + override.overriddenRepoName, + override.extensionName, + overrideOnOverride.overridingRepoName, + overrideOnOverride.location()) + .withCallStack(override.stack); + } + return module .setRegistry(registry) .setDeps(ImmutableMap.copyOf(deps)) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java index 71a3a4490edcea..be4672af5d7cfe 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RegularRunnableExtension.java @@ -263,6 +263,7 @@ private RunModuleExtensionResult runInternal( usagesValue.getExtensionUniqueName() + separator, extensionId.getBzlFileLabel().getPackageIdentifier(), BazelModuleContext.of(bzlLoadValue.getModule()).repoMapping(), + usagesValue.getRepoOverrides(), directories, env.getListener()); Optional moduleExtensionMetadata; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java index d0a8e98c8dc51d..fbce3dfd7dac3b 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java @@ -52,7 +52,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) for (ModuleExtensionUsage usage : usagesValue.getExtensionUsages().values()) { for (ModuleExtensionUsage.Proxy proxy : usage.getProxies()) { for (Entry repoImport : proxy.getImports().entrySet()) { - if (!evalOnlyValue.getGeneratedRepoSpecs().containsKey(repoImport.getValue())) { + if (!evalOnlyValue.getGeneratedRepoSpecs().containsKey(repoImport.getValue()) + && !usagesValue.getRepoOverrides().containsKey(repoImport.getValue())) { throw new SingleExtensionFunctionException( ExternalDepsException.withMessage( Code.INVALID_EXTENSION_IMPORT, @@ -71,6 +72,38 @@ public SkyValue compute(SkyKey skyKey, Environment env) } } + // Check that repo overrides apply as declared. + for (ModuleExtensionUsage usage : usagesValue.getExtensionUsages().values()) { + for (var override : usage.getRepoOverrides().entrySet()) { + boolean repoExists = evalOnlyValue.getGeneratedRepoSpecs().containsKey(override.getKey()); + if (repoExists && !override.getValue().mustExist()) { + throw new SingleExtensionFunctionException( + ExternalDepsException.withMessage( + Code.INVALID_EXTENSION_IMPORT, + "module extension \"%s\" from \"%s\" generates repository \"%s\", yet" + + " it is injected via inject_repo() at %s. Use override_repo() instead to" + + " override an existing repository.", + extensionId.getExtensionName(), + extensionId.getBzlFileLabel(), + override.getKey(), + override.getValue().location()), + Transience.PERSISTENT); + } else if (!repoExists && override.getValue().mustExist()) { + throw new SingleExtensionFunctionException( + ExternalDepsException.withMessage( + Code.INVALID_EXTENSION_IMPORT, + "module extension \"%s\" from \"%s\" does not generate repository \"%s\", yet" + + " it is overridden via override_repo() at %s. Use inject_repo() instead to" + + " inject a new repository.", + extensionId.getExtensionName(), + extensionId.getBzlFileLabel(), + override.getKey(), + override.getValue().location()), + Transience.PERSISTENT); + } + } + } + return evalOnlyValue; } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesFunction.java index 870eb6c276b39b..4a3ca17e13f372 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesFunction.java @@ -61,6 +61,7 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept .collect(toImmutableList()), // TODO(wyv): Maybe cache these mappings? usagesTable.row(id).keySet().stream() - .collect(toImmutableMap(key -> key, bazelDepGraphValue::getFullRepoMapping))); + .collect(toImmutableMap(key -> key, bazelDepGraphValue::getFullRepoMapping)), + bazelDepGraphValue.getRepoOverrides().row(id)); } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesValue.java index fef7930072a014..69a5c1375cf865 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUsagesValue.java @@ -21,6 +21,7 @@ import com.google.common.collect.Maps; import com.google.common.hash.Hashing; import com.google.devtools.build.lib.cmdline.RepositoryMapping; +import com.google.devtools.build.lib.cmdline.RepositoryName; import com.google.devtools.build.lib.skyframe.SkyFunctions; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; import com.google.devtools.build.skyframe.AbstractSkyKey; @@ -55,13 +56,17 @@ public abstract class SingleExtensionUsagesValue implements SkyValue { /** The repo mappings to use for each module that used this extension. */ public abstract ImmutableMap getRepoMappings(); + /** Maps an extension-local repo name to the canonical name of the repo it is overridden with. */ + public abstract ImmutableMap getRepoOverrides(); + public static SingleExtensionUsagesValue create( ImmutableMap extensionUsages, String extensionUniqueName, ImmutableList abridgedModules, - ImmutableMap repoMappings) { + ImmutableMap repoMappings, + ImmutableMap repoOverrides) { return new AutoValue_SingleExtensionUsagesValue( - extensionUsages, extensionUniqueName, abridgedModules, repoMappings); + extensionUsages, extensionUniqueName, abridgedModules, repoMappings, repoOverrides); } /** @@ -88,7 +93,8 @@ SingleExtensionUsagesValue trimForEvaluation() { // repoMappings: The usage of repo mappings by the extension's implementation function is // tracked on the level of individual entries and all label attributes are provided as // `Label`, which exclusively reference canonical repository names. - ImmutableMap.of()); + ImmutableMap.of(), + getRepoOverrides()); } public static Key key(ModuleExtensionId id) { 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 19eae46c5ac7bc..46b48454f05014 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 @@ -220,6 +220,7 @@ private static ModuleExtensionUsage createModuleExtensionUsage( .setExtensionBzlFile(bzlFile) .setExtensionName(name) .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setDevDependency(false) 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 3c36c8e1e4ba51..45d76832687d8c 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 @@ -1451,6 +1451,7 @@ public void nonVisibleLabelInLabelAttr() throws Exception { """ ERROR /ws/defs.bzl:9:12: Traceback (most recent call last): \tFile "/ws/defs.bzl", line 9, column 12, in _ext_impl + \t\tdata_repo(name='ext',data='@not_other_repo//:foo') Error in repository_rule: no repository visible as '@not_other_repo' in \ the extension '@@//:defs.bzl%ext', but referenced by label \ '@not_other_repo//:foo' in attribute 'data' of data_repo 'ext'."""); @@ -1495,6 +1496,7 @@ public void nonVisibleLabelInLabelAttrNonRootModule() throws Exception { Traceback (most recent call last): \tFile "/usr/local/google/_blaze_jrluser/FAKEMD5/external/ext_module~/defs.bzl", \ line 9, column 12, in _ext_impl + \t\tdata_repo(name='ext',data='@not_other_repo//:foo') Error in repository_rule: no repository visible as '@not_other_repo' in the extension \ '@@ext_module~//:defs.bzl%ext', but referenced by label '@not_other_repo//:foo' in \ attribute 'data' of data_repo 'ext'."""); @@ -1573,6 +1575,7 @@ public void nonVisibleLabelInLabelListAttr() throws Exception { """ ERROR /ws/defs.bzl:9:12: Traceback (most recent call last): \tFile "/ws/defs.bzl", line 9, column 12, in _ext_impl + \t\tdata_repo(name='ext',data=['@not_other_repo//:foo']) Error in repository_rule: no repository visible as '@not_other_repo' \ in the extension '@@//:defs.bzl%ext', but referenced by label \ '@not_other_repo//:foo' in attribute 'data' of data_repo 'ext'."""); @@ -1609,6 +1612,7 @@ public void nonVisibleLabelInLabelKeyedStringDictAttr() throws Exception { """ ERROR /ws/defs.bzl:9:12: Traceback (most recent call last): \tFile "/ws/defs.bzl", line 9, column 12, in _ext_impl + \t\tdata_repo(name='ext',data={'@not_other_repo//:foo':'bar'}) Error in repository_rule: no repository visible as '@not_other_repo' \ in the extension '@@//:defs.bzl%ext', but referenced by label \ '@not_other_repo//:foo' in attribute 'data' of data_repo 'ext'."""); @@ -2065,6 +2069,7 @@ public void extensionMetadata() throws Exception { " 'dev_as_non_dev_dep',", " my_direct_dep = 'direct_dep',", ")", + "inject_repo(ext, my_data_repo = 'data_repo')", "ext_dev = use_extension('@ext//:defs.bzl', 'ext', dev_dependency = True)", "use_repo(", " ext_dev,", @@ -3120,4 +3125,211 @@ def _other_ext_impl(ctx): "@@_main~other_ext~other_bar//:bar") .inOrder(); } + + @Test + public void overrideRepo_override() throws Exception { + scratch.file( + workspaceRoot.getRelative("MODULE.bazel").getPathString(), + """ + bazel_dep(name = "data_repo", version = "1.0") + ext = use_extension("//:defs.bzl","ext") + use_repo(ext, "bar", module_foo = "foo") + data_repo = use_repo_rule("@data_repo//:defs.bzl", "data_repo") + data_repo(name = "override", data = "overridden_data") + override_repo(ext, foo = "override") + """); + scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + scratch.file( + workspaceRoot.getRelative("data.bzl").getPathString(), + """ + load("@bar//:list.bzl", _bar_list = "list") + load("@override//:data.bzl", _override_data = "data") + load("@module_foo//:data.bzl", _foo_data = "data") + bar_list = _bar_list + foo_data = _foo_data + override_data = _override_data + """); + scratch.file( + workspaceRoot.getRelative("defs.bzl").getPathString(), + """ + load("@data_repo//:defs.bzl", "data_repo") + def _list_repo_impl(ctx): + ctx.file("WORKSPACE") + ctx.file("BUILD") + labels = [str(Label(l)) for l in ctx.attr.labels] + labels += [str(Label("@module_foo//:target3"))] + ctx.file("list.bzl", "list = " + repr(labels) + " + [str(Label('@foo//:target4'))]") + list_repo = repository_rule( + implementation = _list_repo_impl, + attrs = { + "labels": attr.label_list(), + }, + ) + def _fail_repo_impl(ctx): + fail("This rule should not be evaluated") + fail_repo = repository_rule(implementation = _fail_repo_impl) + def _ext_impl(ctx): + fail_repo(name = "foo") + list_repo( + name = "bar", + labels = [ + # lazy extension implementation function repository mapping + "@foo//:target1", + # module repo repository mapping + "@module_foo//:target2", + ], + ) + ext = module_extension(implementation = _ext_impl) + """); + + SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl")); + EvaluationResult result = + evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); + if (result.hasError()) { + throw result.getError().getException(); + } + assertThat((List) result.get(skyKey).getModule().getGlobal("bar_list")) + .containsExactly( + "@@_main~_repo_rules~override//:target1", + "@@_main~_repo_rules~override//:target2", + "@@_main~_repo_rules~override//:target3", + "@@_main~_repo_rules~override//:target4") + .inOrder(); + Object overrideData = result.get(skyKey).getModule().getGlobal("override_data"); + assertThat(overrideData).isInstanceOf(String.class); + assertThat(overrideData).isEqualTo("overridden_data"); + Object fooData = result.get(skyKey).getModule().getGlobal("foo_data"); + assertThat(fooData).isSameInstanceAs(overrideData); + } + + @Test + public void overrideRepo_override_onNonExistentRepoFails() throws Exception { + scratch.file( + workspaceRoot.getRelative("MODULE.bazel").getPathString(), + """ + bazel_dep(name = "data_repo", version = "1.0") + ext = use_extension("//:defs.bzl","ext") + use_repo(ext, "bar", module_foo = "foo") + data_repo = use_repo_rule("@data_repo//:defs.bzl", "data_repo") + data_repo(name = "foo", data = "overridden_data") + override_repo(ext, "foo") + """); + scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + scratch.file( + workspaceRoot.getRelative("data.bzl").getPathString(), + """ + load("@bar//:list.bzl", _bar_list = "list") + load("@foo//:data.bzl", _foo_data = "data") + bar_list = _bar_list + foo_data = _foo_data + """); + scratch.file( + workspaceRoot.getRelative("defs.bzl").getPathString(), + """ + load("@data_repo//:defs.bzl", "data_repo") + def _list_repo_impl(ctx): + ctx.file("WORKSPACE") + ctx.file("BUILD") + labels = [str(Label(l)) for l in ctx.attr.labels] + labels += [str(Label("@foo//:target3"))] + ctx.file("list.bzl", "list = " + repr(labels) + " + [str(Label('@foo//:target4'))]") + list_repo = repository_rule( + implementation = _list_repo_impl, + attrs = { + "labels": attr.label_list(), + }, + ) + def _ext_impl(ctx): + list_repo( + name = "bar", + labels = [ + # lazy extension implementation function repository mapping + "@foo//:target1", + # module repo repository mapping + Label("@foo//:target2"), + ], + ) + ext = module_extension(implementation = _ext_impl) + """); + + SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl")); + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); + assertThat(result.hasError()).isTrue(); + assertThat(result.getError().getException()) + .hasMessageThat() + .isEqualTo( + "module extension \"ext\" from \"//:defs.bzl\" does not generate repository \"foo\"," + + " yet it is overridden via override_repo() at /ws/MODULE.bazel:6:14. Use" + + " inject_repo() instead to inject a new repository."); + } + + @Test + public void overrideRepo_inject() throws Exception { + scratch.file( + workspaceRoot.getRelative("MODULE.bazel").getPathString(), + """ + bazel_dep(name = "data_repo", version = "1.0") + ext = use_extension("//:defs.bzl","ext") + use_repo(ext, "bar") + data_repo = use_repo_rule("@data_repo//:defs.bzl", "data_repo") + data_repo(name = "foo", data = "overridden_data") + inject_repo(ext, "foo") + """); + scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); + scratch.file( + workspaceRoot.getRelative("data.bzl").getPathString(), + """ + load("@bar//:list.bzl", _bar_list = "list") + load("@foo//:data.bzl", _foo_data = "data") + bar_list = _bar_list + foo_data = _foo_data + """); + scratch.file( + workspaceRoot.getRelative("defs.bzl").getPathString(), + """ + load("@data_repo//:defs.bzl", "data_repo") + def _list_repo_impl(ctx): + ctx.file("WORKSPACE") + ctx.file("BUILD") + labels = [str(Label(l)) for l in ctx.attr.labels] + labels += [str(Label("@foo//:target3"))] + ctx.file("list.bzl", "list = " + repr(labels) + " + [str(Label('@foo//:target4'))]") + list_repo = repository_rule( + implementation = _list_repo_impl, + attrs = { + "labels": attr.label_list(), + }, + ) + def _ext_impl(ctx): + list_repo( + name = "bar", + labels = [ + # lazy extension implementation function repository mapping + "@foo//:target1", + # module repo repository mapping + Label("@foo//:target2"), + ], + ) + ext = module_extension(implementation = _ext_impl) + """); + + SkyKey skyKey = BzlLoadValue.keyForBuild(Label.parseCanonical("//:data.bzl")); + EvaluationResult result = + evaluator.evaluate(ImmutableList.of(skyKey), evaluationContext); + if (result.hasError()) { + throw result.getError().getException(); + } + assertThat((List) result.get(skyKey).getModule().getGlobal("bar_list")) + .containsExactly( + "@@_main~_repo_rules~foo//:target1", + "@@_main~_repo_rules~foo//:target2", + "@@_main~_repo_rules~foo//:target3", + "@@_main~_repo_rules~foo//:target4") + .inOrder(); + Object fooData = result.get(skyKey).getModule().getGlobal("foo_data"); + assertThat(fooData).isInstanceOf(String.class); + assertThat(fooData).isEqualTo("overridden_data"); + } } 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 60bbeedc804e3f..22bd752fba39a2 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 @@ -781,6 +781,7 @@ public void testModuleExtensions_good() throws Exception { .setExtensionBzlFile("@mymod//:defs.bzl") .setExtensionName("myext1") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -811,6 +812,7 @@ public void testModuleExtensions_good() throws Exception { .setExtensionBzlFile("@mymod//:defs.bzl") .setExtensionName("myext2") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -855,6 +857,7 @@ public void testModuleExtensions_good() throws Exception { .setExtensionBzlFile("@rules_jvm_external//:defs.bzl") .setExtensionName("maven") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -932,6 +935,7 @@ public void testModuleExtensions_duplicateProxy_asRoot() throws Exception { .setExtensionBzlFile("@//:defs.bzl") .setExtensionName("myext") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -1063,6 +1067,7 @@ public void testModuleExtensions_duplicateProxy_asDep() throws Exception { .setExtensionBzlFile("@mymod//:defs.bzl") .setExtensionName("myext") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -1182,6 +1187,7 @@ public void testModuleExtensions_innate() throws Exception { .setExtensionBzlFile("//:MODULE.bazel") .setExtensionName("_repo_rules") .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation( @@ -1698,7 +1704,8 @@ public void testSingleVersionOverridePatches() throws Exception { .isEqualTo(ImmutableList.of(">=7.0.0")); assertThat(result.get(moduleFileKey).getModule().getDeps()) .containsExactly( - "ccc", InterimModule.DepSpec.fromModuleKey(ModuleKey.create("ccc", Version.parse("3.0")))); + "ccc", + InterimModule.DepSpec.fromModuleKey(ModuleKey.create("ccc", Version.parse("3.0")))); FileSystemUtils.writeContentAsLatin1( otherPatch, @@ -1721,7 +1728,8 @@ public void testSingleVersionOverridePatches() throws Exception { .isEqualTo(ImmutableList.of(">=7.0.0")); assertThat(result.get(moduleFileKey).getModule().getDeps()) .containsExactly( - "ccc", InterimModule.DepSpec.fromModuleKey(ModuleKey.create("ccc", Version.parse("2.0")))); + "ccc", + InterimModule.DepSpec.fromModuleKey(ModuleKey.create("ccc", Version.parse("2.0")))); } @Test @@ -1761,4 +1769,203 @@ public void testSingleVersionOverridePatches_failsOnRename() throws Exception { + " Renaming /module/MODULE.bazel while applying patches to it as a single file is" + " not supported."); } + + @Test + public void testOverrideRepo_overridingRepoDoesntExist() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + ext = use_extension('//:defs.bzl', 'ext') + override_repo(ext, 'foo') + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:3:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 3, column 14, in + \t\toverride_repo(ext, 'foo') + Error in override_repo: The repo exported as 'foo' by module extension 'ext' is \ + overridden with 'foo', but no repo is visible under this name"""); + } + + @Test + public void testOverrideRepo_duplicateOverride() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = "override1", version = "1.0") + bazel_dep(name = "override2", version = "1.0") + ext = use_extension('//:defs.bzl', 'ext') + override_repo(ext, foo = "override1") + override_repo(ext, foo = "override2") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:6:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 6, column 14, in + \t\toverride_repo(ext, foo = "override2") + Error in override_repo: The repo exported as 'foo' by module extension 'ext' is already \ + overridden with 'override1' at /workspace/MODULE.bazel:5:14"""); + } + + @Test + public void testOverrideRepo_chain_singleExtension() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = "override", version = "1.0") + ext = use_extension('//:defs.bzl', 'ext') + use_repo(ext, bar = "foo") + override_repo(ext, baz = "bar") + override_repo(ext, foo = "override") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 14, in + \t\toverride_repo(ext, baz = "bar") + Error in override_repo: The repo 'bar' used as an override for 'baz' in module extension \ + 'ext' is itself overridden with 'override' at /workspace/MODULE.bazel:6:14, which is not \ + supported."""); + } + + @Test + public void testOverrideRepo_chain_multipleExtensions() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = "override", version = "1.0") + ext1 = use_extension('//:defs.bzl', 'ext1') + ext2 = use_extension('//:defs.bzl', 'ext2') + override_repo(ext1, baz = "bar") + override_repo(ext2, foo = "override") + use_repo(ext2, bar = "foo") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 14, in + \t\toverride_repo(ext1, baz = "bar") + Error in override_repo: The repo 'bar' used as an override for 'baz' in module extension \ + 'ext1' is itself overridden with 'override' at /workspace/MODULE.bazel:6:14, which is not \ + supported."""); + } + + @Test + public void testOverrideRepo_cycle_singleExtension() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = "override", version = "1.0") + ext = use_extension('//:defs.bzl', 'ext') + use_repo(ext, my_foo = "foo", my_bar = "bar") + override_repo(ext, foo = "my_bar", bar = "my_foo") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 14, in + \t\toverride_repo(ext, foo = "my_bar", bar = "my_foo") + Error in override_repo: The repo 'my_foo' used as an override for 'bar' in module \ + extension 'ext' is itself overridden with 'my_bar' at /workspace/MODULE.bazel:5:14, which \ + is not supported."""); + } + + @Test + public void testOverrideRepo_cycle_multipleExtensions() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + ext1 = use_extension('//:defs.bzl', 'ext1') + ext2 = use_extension('//:defs.bzl', 'ext2') + override_repo(ext1, foo = "my_bar") + override_repo(ext2, bar = "my_foo") + use_repo(ext1, my_foo = "foo") + use_repo(ext2, my_bar = "bar") + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:14: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 14, in + \t\toverride_repo(ext2, bar = "my_foo") + Error in override_repo: The repo 'my_foo' used as an override for 'bar' in module \ + extension 'ext2' is itself overridden with 'my_bar' at /workspace/MODULE.bazel:4:14, \ + which is not supported."""); + } + + @Test + public void testInjectRepo_imported() throws Exception { + scratch.overwriteFile( + rootDirectory.getRelative("MODULE.bazel").getPathString(), + """ + module(name='aaa') + bazel_dep(name = 'my_repo', version = "1.0") + ext = use_extension('//:defs.bzl', 'ext') + inject_repo(ext, foo = 'my_repo') + use_repo(ext, bar = 'foo') + """); + + reporter.removeHandler(failFastHandler); + EvaluationResult result = + evaluator.evaluate( + ImmutableList.of(ModuleFileValue.KEY_FOR_ROOT_MODULE), evaluationContext); + assertThat(result.hasError()).isTrue(); + + assertContainsEvent( + """ + ERROR /workspace/MODULE.bazel:5:9: Traceback (most recent call last): + \tFile "/workspace/MODULE.bazel", line 5, column 9, in + \t\tuse_repo(ext, bar = 'foo') + Error in use_repo: Cannot import repo 'foo' that has been injected into \ + module extension 'ext' at /workspace/MODULE.bazel:4:12. Please refer \ + to @my_repo directly.\ + """); + } } 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 121bab0d494c15..b300c58e81172b 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 @@ -45,7 +45,8 @@ private static ModuleExtensionUsage.Builder getBaseUsageBuilder() { return ModuleExtensionUsage.builder() .setExtensionBzlFile("//:rje.bzl") .setExtensionName("maven") - .setIsolationKey(Optional.empty()); + .setIsolationKey(Optional.empty()) + .setRepoOverrides(ImmutableMap.of()); } /** A builder for ModuleExtension that sets all the mandatory but irrelevant fields. */ diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutorTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutorTest.java index 6ac2a9d7714c4b..9deb60bf20849e 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutorTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand/ModExecutorTest.java @@ -607,6 +607,7 @@ public void testExtensionsInfoTextAndGraph() throws Exception { ModuleExtensionUsage.builder() .setExtensionBzlFile("//extensions:extensions.bzl") .setExtensionName("maven") + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation(Location.fromFileLineColumn("C@1.0/MODULE.bazel", 2, 23)) @@ -621,6 +622,7 @@ public void testExtensionsInfoTextAndGraph() throws Exception { ModuleExtensionUsage.builder() .setExtensionBzlFile("//extensions:extensions.bzl") .setExtensionName("maven") + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation(Location.fromFileLineColumn("D@1.0/MODULE.bazel", 1, 10)) @@ -635,6 +637,7 @@ public void testExtensionsInfoTextAndGraph() throws Exception { ModuleExtensionUsage.builder() .setExtensionBzlFile("//extensions:extensions.bzl") .setExtensionName("gradle") + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation(Location.fromFileLineColumn("Y@2.0/MODULE.bazel", 2, 13)) @@ -649,6 +652,7 @@ public void testExtensionsInfoTextAndGraph() throws Exception { ModuleExtensionUsage.builder() .setExtensionBzlFile("//extensions:extensions.bzl") .setExtensionName("maven") + .setRepoOverrides(ImmutableMap.of()) .addProxy( ModuleExtensionUsage.Proxy.builder() .setLocation(Location.fromFileLineColumn("Y@2.0/MODULE.bazel", 13, 10)) diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java index 0a8790d978b6fb..6282c71e4446b7 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkDefinedAspectsTest.java @@ -796,6 +796,7 @@ public void aspectFailingExecution() throws Exception { + "//test:aspect.bzl%MyAspect aspect on java_library rule //test:xxx: \n" + "Traceback (most recent call last):\n" + "\tFile \"/workspace/test/aspect.bzl\", line 2, column 13, in _impl\n" + + "\t\treturn 1 // 0\n" + "Error: integer division by zero"); } diff --git a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java index 3c008929ea2210..f667de302c8735 100644 --- a/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java +++ b/src/test/java/com/google/devtools/build/lib/starlark/StarlarkIntegrationTest.java @@ -444,17 +444,17 @@ protected void runStackTraceTest(String expr, String errorMessage) throws Except "Traceback (most recent call last):", "\tFile \"/workspace/test/starlark/extension.bzl\", line 6, column 6, in" + " custom_rule_impl", - // "\t\tfoo()", + "\t\tfoo()", "\tFile \"/workspace/test/starlark/extension.bzl\", line 9, column 6, in foo", - // "\t\tbar(2, 4)", + "\t\tbar(2, 4)", "\tFile \"/workspace/test/starlark/extension.bzl\", line 11, column 8, in bar", - // "\t\tfirst(x, y, z)", + "\t\tfirst(x, y, z)", "\tFile \"/workspace/test/starlark/functions.bzl\", line 2, column 9, in first", - // "\t\tsecond(a, b)", + "\t\tsecond(a, b)", "\tFile \"/workspace/test/starlark/functions.bzl\", line 5, column 8, in second", - // "\t\tthird(\"legal\")", + "\t\tthird('legal')", "\tFile \"/workspace/test/starlark/functions.bzl\", line 7, column 12, in third", - // ... + "\t\t" + expr.stripLeading(), errorMessage); scratch.file( "test/starlark/extension.bzl", @@ -466,9 +466,9 @@ protected void runStackTraceTest(String expr, String errorMessage) throws Except " foo()", " return [MyInfo(provider_key = ftb)]", "def foo():", - " bar(2,4)", + " bar(2, 4)", "def bar(x,y,z=1):", - " first(x,y, z)", + " first(x, y, z)", "custom_rule = rule(implementation = custom_rule_impl,", " attrs = {'attr1': attr.label_list(mandatory=True, allow_files=True)})"); scratch.file( diff --git a/src/test/java/com/google/devtools/build/lib/testutil/BUILD b/src/test/java/com/google/devtools/build/lib/testutil/BUILD index f5d1477482c697..54c734fe7250f7 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/BUILD +++ b/src/test/java/com/google/devtools/build/lib/testutil/BUILD @@ -73,6 +73,7 @@ java_library( "//src/main/java/com/google/devtools/build/lib/vfs", "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment", "//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs", + "//src/main/java/net/starlark/java/eval", "//src/main/java/net/starlark/java/syntax", "//src/main/protobuf:failure_details_java_proto", "//third_party:error_prone_annotations", diff --git a/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java b/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java index 3c70cb77967f3f..aa845ffb132ce5 100644 --- a/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/testutil/FoundationTestCase.java @@ -13,8 +13,11 @@ // limitations under the License. package com.google.devtools.build.lib.testutil; +import static java.nio.charset.StandardCharsets.UTF_8; import static org.junit.Assert.fail; +import com.google.common.base.Splitter; +import com.google.common.collect.Iterables; import com.google.common.eventbus.EventBus; import com.google.devtools.build.lib.clock.BlazeClock; import com.google.devtools.build.lib.events.Event; @@ -24,11 +27,13 @@ import com.google.devtools.build.lib.events.Reporter; import com.google.devtools.build.lib.vfs.DigestHashFunction; import com.google.devtools.build.lib.vfs.FileSystem; +import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.Path; import com.google.devtools.build.lib.vfs.Root; import com.google.devtools.build.lib.vfs.inmemoryfs.InMemoryFileSystem; import java.util.Set; import java.util.regex.Pattern; +import net.starlark.java.eval.EvalException; import org.junit.After; import org.junit.Before; @@ -78,6 +83,19 @@ public final void initializeFileSystemAndDirectories() throws Exception { scratch.file(rootDirectory.getRelative("WORKSPACE").getPathString()); scratch.file(rootDirectory.getRelative("MODULE.bazel").getPathString()); root = Root.fromPath(rootDirectory); + + // Let the Starlark interpreter know how to read source files. + EvalException.setSourceReaderSupplier( + () -> + loc -> { + try { + String content = FileSystemUtils.readContent(fileSystem.getPath(loc.file()), UTF_8); + return Iterables.get(Splitter.on("\n").split(content), loc.line() - 1, null); + } catch (Exception ignored) { + // ignore any exceptions reading the file -- this is just for extra info + return null; + } + }); } @Before