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

[8.0.0] Add support for --keep_going to bazel mod #24259

Merged
merged 1 commit into from
Nov 11, 2024
Merged
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
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
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());
}
}
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);
}
}
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());
}
}
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
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
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
Loading