Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Add --inject_repository and fix crash on overridden non-existent repo #23791

Closed
wants to merge 6 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
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 @@ -469,6 +471,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 @@ -603,7 +623,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 @@ -259,6 +259,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/vfs",
"//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/lib/vfs/inmemoryfs",
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,11 +76,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 @@ -95,6 +97,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 @@ -194,6 +198,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 @@ -287,6 +292,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 @@ -420,6 +426,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 @@ -432,6 +439,7 @@ public static RootModuleFileValue evaluateRootModuleFile(
ModuleKey.ROOT,
ignoreDevDeps,
builtinModules,
injectedRepositories,
/* printIsNoop= */ false,
starlarkSemantics,
eventHandler,
Expand Down Expand Up @@ -495,6 +503,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 @@ -524,6 +533,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 @@ -532,6 +543,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)
Wyverald marked this conversation as resolved.
Show resolved Hide resolved
.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 @@ -72,6 +72,7 @@ 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",
"//src/main/java/net/starlark/java/eval",
"//third_party:auto_value",
"//third_party:guava",
],
Expand Down
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) {}
fmeum marked this conversation as resolved.
Show resolved Hide resolved

/** 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 @@ -413,6 +413,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
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@

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

import static com.google.devtools.build.lib.skyframe.RepositoryMappingFunction.REPOSITORY_OVERRIDES;
import static java.nio.charset.StandardCharsets.UTF_8;

import com.google.common.annotations.VisibleForTesting;
Expand Down Expand Up @@ -71,9 +72,6 @@
* this function.
*/
public final class RepositoryDelegatorFunction implements SkyFunction {
public static final Precomputed<Map<RepositoryName, PathFragment>> REPOSITORY_OVERRIDES =
new Precomputed<>("repository_overrides");

public static final String FORCE_FETCH_DISABLED = "";

public static final Precomputed<String> FORCE_FETCH =
Expand Down
1 change: 1 addition & 0 deletions src/main/java/com/google/devtools/build/lib/skyframe/BUILD
Original file line number Diff line number Diff line change
Expand Up @@ -2396,6 +2396,7 @@ java_library(
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//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/vfs:pathfragment",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//src/main/java/net/starlark/java/eval",
"//third_party:guava",
Expand Down
Loading
Loading