Skip to content

Commit

Permalink
[8.0.0] Add --inject_repository and fix crash on overridden non-exi…
Browse files Browse the repository at this point in the history
…stent repo (#24301)

Suggest the new flag instead of crashing when `--override_repository` is
applied to a non-existent repo with `--noenable_workspace`.

Fixes #22691

RELNOTES: The new `--inject_repository` flag can be used to add new
repositories via the CLI with `--enable_bzlmod`. Such repositories
behave as if they were declared by `local_repository` via
`use_repo_rule` in the root module.

Closes #23791.

PiperOrigin-RevId: 695848897
Change-Id: I92ed25261c92d07f289815fcf6a65485ff43f373

Co-authored-by: Fabian Meumertzheim <[email protected]>
  • Loading branch information
Wyverald and fmeum authored Nov 12, 2024
1 parent 0477f38 commit 4d6fd93
Show file tree
Hide file tree
Showing 39 changed files with 385 additions and 58 deletions.
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -52,6 +52,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:mutable_supplier",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_cluster",
"//src/main/java/com/google/devtools/build/lib/skyframe:skyframe_executor_repository_helpers_holder",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -104,6 +104,7 @@
import com.google.devtools.build.lib.skyframe.MutableSupplier;
import com.google.devtools.build.lib.skyframe.PrecomputedValue;
import com.google.devtools.build.lib.skyframe.PrecomputedValue.Injected;
import com.google.devtools.build.lib.skyframe.RepositoryMappingFunction;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.SkyframeExecutorRepositoryHelpersHolder;
import com.google.devtools.build.lib.starlarkbuildapi.repository.RepositoryBootstrap;
Expand Down Expand Up @@ -146,6 +147,7 @@ public class BazelRepositoryModule extends BlazeModule {
private final MutableSupplier<Map<String, String>> clientEnvironmentSupplier =
new MutableSupplier<>();
private ImmutableMap<RepositoryName, PathFragment> overrides = ImmutableMap.of();
private ImmutableMap<String, PathFragment> injections = ImmutableMap.of();
private ImmutableMap<String, ModuleOverride> moduleOverrides = ImmutableMap.of();
private Optional<RootedPath> resolvedFileReplacingWorkspace = Optional.empty();
private FileSystem filesystem;
Expand Down Expand Up @@ -477,6 +479,24 @@ public void beforeCommand(CommandEnvironment env) throws AbruptExitException {
overrides = ImmutableMap.of();
}

if (repoOptions.repositoryInjections != null) {
Map<String, PathFragment> injectionMap = new LinkedHashMap<>();
for (RepositoryOptions.RepositoryInjection injection : repoOptions.repositoryInjections) {
if (injection.path().isEmpty()) {
injectionMap.remove(injection.apparentName());
continue;
}
String repoPath = getAbsolutePath(injection.path(), env);
injectionMap.put(injection.apparentName(), PathFragment.create(repoPath));
}
ImmutableMap<String, PathFragment> newInjections = ImmutableMap.copyOf(injectionMap);
if (!Maps.difference(injections, newInjections).areEqual()) {
injections = newInjections;
}
} else {
injections = ImmutableMap.of();
}

if (repoOptions.moduleOverrides != null) {
Map<String, ModuleOverride> moduleOverrideMap = new LinkedHashMap<>();
for (RepositoryOptions.ModuleOverride override : repoOptions.moduleOverrides) {
Expand Down Expand Up @@ -611,7 +631,8 @@ public ImmutableList<Injected> getPrecomputedValues() {
lastRegistryInvalidation = now;
}
return ImmutableList.of(
PrecomputedValue.injected(RepositoryDelegatorFunction.REPOSITORY_OVERRIDES, overrides),
PrecomputedValue.injected(RepositoryMappingFunction.REPOSITORY_OVERRIDES, overrides),
PrecomputedValue.injected(ModuleFileFunction.INJECTED_REPOSITORIES, injections),
PrecomputedValue.injected(ModuleFileFunction.MODULE_OVERRIDES, moduleOverrides),
PrecomputedValue.injected(
RepositoryDelegatorFunction.RESOLVED_FILE_INSTEAD_OF_WORKSPACE,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,11 +77,13 @@
import java.util.Optional;
import java.util.stream.Stream;
import javax.annotation.Nullable;
import net.starlark.java.eval.Dict;
import net.starlark.java.eval.EvalException;
import net.starlark.java.eval.Mutability;
import net.starlark.java.eval.StarlarkSemantics;
import net.starlark.java.eval.StarlarkThread;
import net.starlark.java.eval.SymbolGenerator;
import net.starlark.java.syntax.Location;

/**
* Takes a {@link ModuleKey} and its override (if any), retrieves the module file from a registry or
Expand All @@ -96,6 +98,8 @@ public class ModuleFileFunction implements SkyFunction {

public static final Precomputed<Map<String, ModuleOverride>> MODULE_OVERRIDES =
new Precomputed<>("module_overrides");
public static final Precomputed<ImmutableMap<String, PathFragment>> INJECTED_REPOSITORIES =
new Precomputed<>("repository_injections");

private final BazelStarlarkEnvironment starlarkEnv;
private final Path workspaceRoot;
Expand Down Expand Up @@ -196,6 +200,7 @@ public SkyValue compute(SkyKey skyKey, Environment env)
// Dev dependencies should always be ignored if the current module isn't the root module
/* ignoreDevDeps= */ true,
builtinModules,
/* injectedRepositories= */ ImmutableMap.of(),
// Disable printing for modules from registries. We don't want them to be able to spam
// the console during resolution, but module files potentially edited by the user as
// part of a non-registry override should permit printing to aid debugging.
Expand Down Expand Up @@ -293,6 +298,7 @@ private SkyValue computeForRootModule(
ImmutableMap.copyOf(state.includeLabelToCompiledModuleFile),
builtinModules,
MODULE_OVERRIDES.get(env),
INJECTED_REPOSITORIES.get(env),
IGNORE_DEV_DEPS.get(env),
starlarkSemantics,
env.getListener(),
Expand Down Expand Up @@ -426,6 +432,7 @@ public static RootModuleFileValue evaluateRootModuleFile(
ImmutableMap<String, CompiledModuleFile> includeLabelToCompiledModuleFile,
ImmutableMap<String, NonRegistryOverride> builtinModules,
Map<String, ModuleOverride> commandOverrides,
Map<String, PathFragment> injectedRepositories,
boolean ignoreDevDeps,
StarlarkSemantics starlarkSemantics,
ExtendedEventHandler eventHandler,
Expand All @@ -438,6 +445,7 @@ public static RootModuleFileValue evaluateRootModuleFile(
ModuleKey.ROOT,
ignoreDevDeps,
builtinModules,
injectedRepositories,
/* printIsNoop= */ false,
starlarkSemantics,
eventHandler,
Expand Down Expand Up @@ -501,6 +509,7 @@ private static ModuleThreadContext execModuleFile(
ModuleKey moduleKey,
boolean ignoreDevDeps,
ImmutableMap<String, NonRegistryOverride> builtinModules,
Map<String, PathFragment> injectedRepositories,
boolean printIsNoop,
StarlarkSemantics starlarkSemantics,
ExtendedEventHandler eventHandler,
Expand Down Expand Up @@ -530,6 +539,8 @@ private static ModuleThreadContext execModuleFile(
}
}
});

injectRepos(injectedRepositories, context, thread);
compiledRootModuleFile.runOnThread(thread);
} catch (EvalException e) {
eventHandler.handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack()));
Expand All @@ -538,6 +549,48 @@ private static ModuleThreadContext execModuleFile(
return context;
}

// Adds a local_repository for each repository injected via --injected_repositories.
private static void injectRepos(
Map<String, PathFragment> injectedRepositories,
ModuleThreadContext context,
StarlarkThread thread)
throws EvalException {
if (injectedRepositories.isEmpty()) {
return;
}
// Use the innate extension backing use_repo_rule.
ModuleThreadContext.ModuleExtensionUsageBuilder usageBuilder =
new ModuleThreadContext.ModuleExtensionUsageBuilder(
context,
"//:MODULE.bazel",
"@bazel_tools//tools/build_defs/repo:local.bzl%local_repository",
/* isolate= */ false);
ModuleFileGlobals.ModuleExtensionProxy extensionProxy =
new ModuleFileGlobals.ModuleExtensionProxy(
usageBuilder,
ModuleExtensionUsage.Proxy.builder()
.setDevDependency(true)
.setLocation(Location.BUILTIN)
.setContainingModuleFilePath(context.getCurrentModuleFilePath()));
for (var injectedRepository : injectedRepositories.entrySet()) {
extensionProxy
.getValue("repo")
.call(
Dict.copyOf(
thread.mutability(),
ImmutableMap.of(
"name", injectedRepository.getKey(),
"path", injectedRepository.getValue().getPathString())),
thread);
extensionProxy.addImport(
injectedRepository.getKey(),
injectedRepository.getKey(),
"by --inject_repository",
thread.getCallStack());
}
context.getExtensionUsageBuilders().add(usageBuilder);
}

/**
* Result of a {@link #getModuleFile} call.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -780,7 +780,8 @@ public RepoRuleProxy useRepoRule(String bzlFile, String ruleName, StarlarkThread
context.setNonModuleCalled();
// Not a valid Starlark identifier so that it can't collide with a real extension.
String extensionName = bzlFile + '%' + ruleName;
// Find or create the builder for the singular "innate" extension of this module.
// Find or create the builder for the singular "innate" extension of this repo rule for this
// module.
for (ModuleExtensionUsageBuilder usageBuilder : context.getExtensionUsageBuilders()) {
if (usageBuilder.isForExtension("//:MODULE.bazel", extensionName)) {
return new RepoRuleProxy(usageBuilder);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -87,13 +87,19 @@ record RepoOverride(
String extensionName,
ImmutableList<StarlarkThread.CallStackEntry> stack) {
Location location() {
if (stack.size() < 2) {
return Location.BUILTIN;
}
// Skip over the override_repo builtin frame.
return stack.reverse().get(1).location;
}
}

record RepoNameUsage(String how, ImmutableList<StarlarkThread.CallStackEntry> stack) {
Location location() {
if (stack.size() < 2) {
return Location.BUILTIN;
}
// Skip over the override_repo builtin frame.
return stack.reverse().get(1).location;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/packages",
"//src/main/java/com/google/devtools/build/lib/packages/semantics",
"//src/main/java/com/google/devtools/build/lib/repository:repository_events",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repo_recorded_input",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_directory_value",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/rules:repository/resolved_file_value",
Expand All @@ -48,7 +47,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/util",
"//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/skyframe",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/com/google/devtools/common/options",
"//src/main/java/net/starlark/java/eval",
Expand All @@ -72,7 +70,6 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/util",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/common/options",
"//third_party:auto_value",
"//third_party:guava",
"//src/main/java/net/starlark/java/eval",
],
)
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,6 @@

package com.google.devtools.build.lib.bazel.repository;

import com.google.auto.value.AutoValue;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.util.OptionsUtils;
Expand All @@ -28,6 +27,7 @@
import com.google.devtools.common.options.OptionsBase;
import com.google.devtools.common.options.OptionsParsingException;
import java.util.List;
import net.starlark.java.eval.EvalException;

/** Command-line options for repositories. */
public class RepositoryOptions extends OptionsBase {
Expand Down Expand Up @@ -131,8 +131,6 @@ public class RepositoryOptions extends OptionsBase {
+ "to download them.")
public List<PathFragment> experimentalDistdir;



@Option(
name = "override_repository",
defaultValue = "null",
Expand All @@ -149,6 +147,24 @@ public class RepositoryOptions extends OptionsBase {
+ " previous overrides.")
public List<RepositoryOverride> repositoryOverrides;

@Option(
name = "inject_repository",
defaultValue = "null",
allowMultiple = true,
converter = RepositoryInjectionConverter.class,
documentationCategory = OptionDocumentationCategory.UNCATEGORIZED,
effectTags = {OptionEffectTag.UNKNOWN},
help =
"Adds a new repository with a local path in the form of <repository name>=<path>. This"
+ " only takes effect with --enable_bzlmod and is equivalent to adding a"
+ " corresponding `local_repository` to the root module's MODULE.bazel file via"
+ " `use_repo_rule`. If the given path is an absolute path, it will be used as it is."
+ " If the given path is a relative path, it is relative to the current working"
+ " directory. If the given path starts with '%workspace%', it is relative to the"
+ " workspace root, which is the output of `bazel info workspace`. If the given path"
+ " is empty, then remove any previous injections.")
public List<RepositoryInjection> repositoryInjections;

@Option(
name = "override_module",
defaultValue = "null",
Expand Down Expand Up @@ -362,7 +378,7 @@ public RepositoryOverride convert(String input) throws OptionsParsingException {
OptionsUtils.PathFragmentConverter pathConverter = new OptionsUtils.PathFragmentConverter();
String pathString = pathConverter.convert(pieces[1]).getPathString();
try {
return RepositoryOverride.create(RepositoryName.create(pieces[0]), pathString);
return new RepositoryOverride(RepositoryName.create(pieces[0]), pathString);
} catch (LabelSyntaxException e) {
throw new OptionsParsingException("Invalid repository name given to override", input, e);
}
Expand All @@ -374,6 +390,35 @@ public String getTypeDescription() {
}
}

/**
* Converts from an equals-separated pair of strings into RepositoryName->PathFragment mapping.
*/
public static class RepositoryInjectionConverter
extends Converter.Contextless<RepositoryInjection> {

@Override
public RepositoryInjection convert(String input) throws OptionsParsingException {
String[] pieces = input.split("=", 2);
if (pieces.length != 2) {
throw new OptionsParsingException(
"Repository injections must be of the form 'repository-name=path'", input);
}
OptionsUtils.PathFragmentConverter pathConverter = new OptionsUtils.PathFragmentConverter();
String pathString = pathConverter.convert(pieces[1]).getPathString();
try {
RepositoryName.validateUserProvidedRepoName(pieces[0]);
return new RepositoryInjection(pieces[0], pathString);
} catch (EvalException e) {
throw new OptionsParsingException("Invalid repository name given to inject", input, e);
}
}

@Override
public String getTypeDescription() {
return "an equals-separated mapping of repository name to path";
}
}

/** Converts from an equals-separated pair of strings into ModuleName->PathFragment mapping. */
public static class ModuleOverrideConverter extends Converter.Contextless<ModuleOverride> {

Expand All @@ -396,7 +441,7 @@ public ModuleOverride convert(String input) throws OptionsParsingException {

OptionsUtils.PathFragmentConverter pathConverter = new OptionsUtils.PathFragmentConverter();
String pathString = pathConverter.convert(pieces[1]).getPathString();
return ModuleOverride.create(pieces[0], pathString);
return new ModuleOverride(pieces[0], pathString);
}

@Override
Expand All @@ -406,28 +451,14 @@ public String getTypeDescription() {
}

/** A repository override, represented by a name and an absolute path to a repository. */
@AutoValue
public abstract static class RepositoryOverride {

private static RepositoryOverride create(RepositoryName repositoryName, String path) {
return new AutoValue_RepositoryOptions_RepositoryOverride(repositoryName, path);
}

public abstract RepositoryName repositoryName();
public record RepositoryOverride(RepositoryName repositoryName, String path) {}

public abstract String path();
}
/**
* A repository injected into the scope of the root module, represented by a name and an absolute
* path to a repository.
*/
public record RepositoryInjection(String apparentName, String path) {}

/** A module override, represented by a name and an absolute path to a module. */
@AutoValue
public abstract static class ModuleOverride {

private static ModuleOverride create(String moduleName, String path) {
return new AutoValue_RepositoryOptions_ModuleOverride(moduleName, path);
}

public abstract String moduleName();

public abstract String path();
}
public record ModuleOverride(String moduleName, String path) {}
}
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/rules/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -412,6 +412,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:package_lookup_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_value_dirtiness_checker",
"//src/main/java/com/google/devtools/build/lib/util",
Expand Down
Loading

0 comments on commit 4d6fd93

Please sign in to comment.