Skip to content

Commit

Permalink
Add support for --keep_going to bazel mod
Browse files Browse the repository at this point in the history 
RELNOTES: `bazel mod` now tries to evaluate all module extensions, even when some have failed to evaluate.

Closes #23828.

PiperOrigin-RevId: 694448503
Change-Id: I1e4f7a36384c7dfdc16d13cdfcd0b4b3aeb41834
  • Loading branch information
@fmeum @copybara-github
fmeum authored and copybara-github committed Nov 8, 2024
1 parent e9a8e7b commit a0471c7
Show file tree
Hide file tree
Showing 8 changed files with 292 additions and 27 deletions.
7 changes: 7 additions & 0 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD
View file
Original file line number Diff line number Diff line change
Expand Up @@ -359,6 +359,7 @@ java_library(
"BazelModTidyValue.java",
],
deps = [
":exception",
":root_module_file_fixup",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
"//src/main/java/com/google/devtools/build/lib/skyframe/serialization/autocodec:serialization-constant",
Expand All @@ -377,10 +378,12 @@ java_library(
],
deps = [
":common",
":exception",
":resolution",
":root_module_file_fixup",
":tidy",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
"//src/main/java/com/google/devtools/build/lib/skyframe:repository_mapping_value",
"//src/main/java/com/google/devtools/build/lib/vfs",
Expand All @@ -398,6 +401,7 @@ java_library(
],
deps = [
":common",
":exception",
":module_extension",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/skyframe:sky_functions",
Expand All @@ -415,9 +419,12 @@ java_library(
],
deps = [
":common",
":exception",
":inspection",
":module_extension",
":resolution",
"//src/main/java/com/google/devtools/build/lib/events",
"//src/main/java/com/google/devtools/build/lib/skyframe:precomputed_value",
"//src/main/java/com/google/devtools/build/skyframe:skyframe-objects",
"//third_party:guava",
"//third_party:jsr305",
Expand Down
17 changes: 15 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
import com.google.devtools.build.lib.cmdline.Label;
import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.rules.repository.NeedsSkyframeRestartException;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.skyframe.RepositoryMappingValue;
Expand Down Expand Up @@ -91,8 +92,17 @@ public SkyValue compute(SkyKey skyKey, Environment env)
return null;
}
ImmutableList.Builder<RootModuleFileFixup> fixups = ImmutableList.builder();
ImmutableList.Builder<ExternalDepsException> errors = ImmutableList.builder();
for (SkyKey extension : extensionsUsedByRootModule) {
SkyValue value = result.get(extension);
SkyValue value;
try {
value = result.getOrThrow(extension, ExternalDepsException.class);
} catch (ExternalDepsException e) {
// This extension failed, but we can still tidy up other extensions in keep going mode.
errors.add(e);
env.getListener().handle(Event.error(e.getMessage()));
continue;
}
if (value == null) {
return null;
}
Expand All @@ -102,6 +112,9 @@ public SkyValue compute(SkyKey skyKey, Environment env)
}

return BazelModTidyValue.create(
fixups.build(), buildozer.asPath(), rootModuleFileValue.getModuleFilePaths());
fixups.build(),
buildozer.asPath(),
rootModuleFileValue.getModuleFilePaths(),
errors.build());
}
}
8 changes: 6 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -41,11 +41,15 @@ public abstract class BazelModTidyValue implements SkyValue {
/** The set of paths to the root MODULE.bazel file and all its includes. */
public abstract ImmutableSet<PathFragment> moduleFilePaths();

/** Errors encountered while evaluating prerequisites for {@code bazel mod tidy}. */
public abstract ImmutableList<ExternalDepsException> errors();

static BazelModTidyValue create(
List<RootModuleFileFixup> fixups,
Path buildozer,
ImmutableSet<PathFragment> moduleFilePaths) {
ImmutableSet<PathFragment> moduleFilePaths,
ImmutableList<ExternalDepsException> errors) {
return new AutoValue_BazelModTidyValue(
ImmutableList.copyOf(fixups), buildozer, moduleFilePaths);
ImmutableList.copyOf(fixups), buildozer, moduleFilePaths, errors);
}
}
36 changes: 26 additions & 10 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleInspectorFunction.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule;
import com.google.devtools.build.lib.bazel.bzlmod.BazelModuleInspectorValue.AugmentedModule.ResolutionReason;
import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.skyframe.SkyFunction;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
Expand Down Expand Up @@ -67,9 +68,8 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
ImmutableMap<ModuleKey, AugmentedModule> depGraph =
computeAugmentedGraph(unprunedDepGraph, resolvedDepGraph.keySet(), overrides);

ImmutableSetMultimap<ModuleExtensionId, String> extensionToRepoInternalNames =
computeExtensionToRepoInternalNames(depGraphValue, env);
if (extensionToRepoInternalNames == null) {
ExtensionRepos extensionRepos = computExtensionRepos(depGraphValue, env);
if (extensionRepos == null) {
return null;
}

Expand All @@ -85,8 +85,9 @@ public SkyValue compute(SkyKey skyKey, Environment env) throws InterruptedExcept
return BazelModuleInspectorValue.create(
depGraph,
modulesIndex,
extensionToRepoInternalNames,
depGraphValue.getCanonicalRepoNameLookup().inverse());
extensionRepos.extensionToRepoInternalNames(),
depGraphValue.getCanonicalRepoNameLookup().inverse(),
extensionRepos.errors());
}

public static ImmutableMap<ModuleKey, AugmentedModule> computeAugmentedGraph(
Expand Down Expand Up @@ -180,9 +181,13 @@ public static ImmutableMap<ModuleKey, AugmentedModule> computeAugmentedGraph(
.collect(toImmutableMap(Entry::getKey, e -> e.getValue().build()));
}

private record ExtensionRepos(
ImmutableSetMultimap<ModuleExtensionId, String> extensionToRepoInternalNames,
ImmutableList<ExternalDepsException> errors) {}

@Nullable
private ImmutableSetMultimap<ModuleExtensionId, String> computeExtensionToRepoInternalNames(
BazelDepGraphValue depGraphValue, Environment env) throws InterruptedException {
private ExtensionRepos computExtensionRepos(BazelDepGraphValue depGraphValue, Environment env)
throws InterruptedException {
ImmutableSet<ModuleExtensionId> extensionEvalKeys =
depGraphValue.getExtensionUsagesTable().rowKeySet();
ImmutableList<SingleExtensionValue.Key> singleExtensionKeys =
Expand All @@ -191,15 +196,26 @@ private ImmutableSetMultimap<ModuleExtensionId, String> computeExtensionToRepoIn

ImmutableSetMultimap.Builder<ModuleExtensionId, String> extensionToRepoInternalNames =
ImmutableSetMultimap.builder();
ImmutableList.Builder<ExternalDepsException> errors = ImmutableList.builder();
for (SingleExtensionValue.Key singleExtensionKey : singleExtensionKeys) {
SingleExtensionValue singleExtensionValue =
(SingleExtensionValue) singleExtensionValues.get(singleExtensionKey);
SingleExtensionValue singleExtensionValue;
try {
singleExtensionValue =
(SingleExtensionValue)
singleExtensionValues.getOrThrow(singleExtensionKey, ExternalDepsException.class);
} catch (ExternalDepsException e) {
// The extension failed, so we can't report its generated repos. We can still report the
// imported repos in keep going mode, so don't fail and just skip this extension.
errors.add(e);
env.getListener().handle(Event.error(e.getMessage()));
continue;
}
if (singleExtensionValue == null) {
return null;
}
extensionToRepoInternalNames.putAll(
singleExtensionKey.argument(), singleExtensionValue.getGeneratedRepoSpecs().keySet());
}
return extensionToRepoInternalNames.build();
return new ExtensionRepos(extensionToRepoInternalNames.build(), errors.build());
}
}
9 changes: 7 additions & 2 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleInspectorValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,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.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSetMultimap;
Expand Down Expand Up @@ -44,9 +45,10 @@ public static BazelModuleInspectorValue create(
ImmutableMap<ModuleKey, AugmentedModule> depGraph,
ImmutableMap<String, ImmutableSet<ModuleKey>> modulesIndex,
ImmutableSetMultimap<ModuleExtensionId, String> extensionToRepoInternalNames,
ImmutableMap<ModuleKey, RepositoryName> moduleKeyToCanonicalNames) {
ImmutableMap<ModuleKey, RepositoryName> moduleKeyToCanonicalNames,
ImmutableList<ExternalDepsException> errors) {
return new AutoValue_BazelModuleInspectorValue(
depGraph, modulesIndex, extensionToRepoInternalNames, moduleKeyToCanonicalNames);
depGraph, modulesIndex, extensionToRepoInternalNames, moduleKeyToCanonicalNames, errors);
}

/**
Expand Down Expand Up @@ -74,6 +76,9 @@ public static BazelModuleInspectorValue create(
/** A mapping from a module key to the canonical repository name of the module repository. */
public abstract ImmutableMap<ModuleKey, RepositoryName> getModuleKeyToCanonicalNames();

/** A list of exceptions that occurred during module graph inspection. */
public abstract ImmutableList<ExternalDepsException> getErrors();

/**
* A wrapper for {@link Module}, augmented with references to dependants (and also those who are
* not used in the final dep graph).
Expand Down
45 changes: 35 additions & 10 deletions src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -61,7 +61,6 @@
import com.google.devtools.build.lib.runtime.BlazeCommandResult;
import com.google.devtools.build.lib.runtime.Command;
import com.google.devtools.build.lib.runtime.CommandEnvironment;
import com.google.devtools.build.lib.runtime.KeepGoingOption;
import com.google.devtools.build.lib.runtime.LoadingPhaseThreadsOption;
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.server.FailureDetails.FailureDetail;
Expand Down Expand Up @@ -100,11 +99,9 @@
@Command(
name = ModCommand.NAME,
buildPhase = LOADS,
// TODO(andreisolo): figure out which extra options are really needed
options = {
ModOptions.class,
PackageOptions.class,
KeepGoingOption.class,
LoadingPhaseThreadsOption.class
},
help = "resource:mod.txt",
Expand Down Expand Up @@ -212,7 +209,7 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe

if (evaluationResult.hasError()) {
Exception e = evaluationResult.getError().getException();
String message = "Unexpected error during repository rule evaluation.";
String message = "Unexpected error during module graph evaluation.";
if (e != null) {
message = e.getMessage();
}
Expand Down Expand Up @@ -541,7 +538,17 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe
return reportAndCreateFailureResult(env, e.getMessage(), Code.INVALID_ARGUMENTS);
}

return BlazeCommandResult.success();
if (moduleInspector.getErrors().isEmpty()) {
return BlazeCommandResult.success();
} else {
return reportAndCreateFailureResult(
env,
String.format(
"Results may be incomplete as %d extension%s failed.",
moduleInspector.getErrors().size(),
moduleInspector.getErrors().size() == 1 ? "" : "s"),
Code.ERROR_DURING_GRAPH_INSPECTION);
}
}

private BlazeCommandResult runTidy(CommandEnvironment env, BazelModTidyValue modTidyValue) {
Expand Down Expand Up @@ -572,7 +579,7 @@ private BlazeCommandResult runTidy(CommandEnvironment env, BazelModTidyValue mod
if (e instanceof AbnormalTerminationException abnormalTerminationException) {
if (abnormalTerminationException.getResult().getTerminationStatus().getRawExitCode() == 3) {
// Buildozer exits with exit code 3 if it didn't make any changes.
return BlazeCommandResult.success();
return reportAndCreateTidyResult(env, modTidyValue);
}
suffix =
":\n" + new String(((AbnormalTerminationException) e).getResult().getStderr(), UTF_8);
Expand All @@ -587,7 +594,21 @@ private BlazeCommandResult runTidy(CommandEnvironment env, BazelModTidyValue mod
env.getReporter().handle(Event.info(fixupEvent.getSuccessMessage()));
}

return BlazeCommandResult.success();
return reportAndCreateTidyResult(env, modTidyValue);
}

private static BlazeCommandResult reportAndCreateTidyResult(
CommandEnvironment env, BazelModTidyValue modTidyValue) {
if (modTidyValue.errors().isEmpty()) {
return BlazeCommandResult.success();
} else {
return reportAndCreateFailureResult(
env,
String.format(
"Failed to process %d extension%s due to errors.",
modTidyValue.errors().size(), modTidyValue.errors().size() == 1 ? "" : "s"),
Code.ERROR_DURING_GRAPH_INSPECTION);
}
}

/** Collects a list of {@link ModuleArg} into a set of {@link ModuleKey}s. */
Expand Down Expand Up @@ -640,9 +661,13 @@ private static ImmutableSortedSet<ModuleExtensionId> extensionArgListToIds(
private static BlazeCommandResult reportAndCreateFailureResult(
CommandEnvironment env, String message, Code detailedCode) {
String fullMessage =
String.format(
"%s%s Type '%s help mod' for syntax and help.",
message, message.endsWith(".") ? "" : ".", env.getRuntime().getProductName());
switch (detailedCode) {
case MISSING_ARGUMENTS, TOO_MANY_ARGUMENTS, INVALID_ARGUMENTS ->
String.format(
"%s%s Type '%s help mod' for syntax and help.",
message, message.endsWith(".") ? "" : ".", env.getRuntime().getProductName());
default -> message;
};
env.getReporter().handle(Event.error(fullMessage));
return createFailureResult(fullMessage, detailedCode);
}
Expand Down
1 change: 1 addition & 0 deletions src/main/protobuf/failure_details.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1372,6 +1372,7 @@ message ModCommand {
TOO_MANY_ARGUMENTS = 2 [(metadata) = { exit_code: 2 }];
INVALID_ARGUMENTS = 3 [(metadata) = { exit_code: 2 }];
BUILDOZER_FAILED = 4 [(metadata) = { exit_code: 2 }];
ERROR_DURING_GRAPH_INSPECTION = 5 [(metadata) = { exit_code: 2 }];
}

Code code = 1;
Expand Down
Loading

0 comments on commit a0471c7

Please sign in to comment.