Skip to content

Commit

Permalink
--keep_going
Browse files Browse the repository at this point in the history
  • Loading branch information
fmeum committed Nov 6, 2024
1 parent 2fd6115 commit 4cbbc54
Show file tree
Hide file tree
Showing 8 changed files with 275 additions and 23 deletions.
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,6 +378,7 @@ java_library(
],
deps = [
":common",
":exception",
":resolution",
":root_module_file_fixup",
":tidy",
Expand All @@ -398,6 +400,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 +418,11 @@ java_library(
],
deps = [
":common",
":exception",
":inspection",
":module_extension",
":resolution",
"//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 @@ -91,8 +91,16 @@ 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);
continue;
}
if (value == null) {
return null;
}
Expand All @@ -102,6 +110,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 @@ -67,9 +67,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 +84,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 +180,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 +195,25 @@ 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);
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 @@ -56,7 +56,6 @@
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.packages.semantics.BuildLanguageOptions;
import com.google.devtools.build.lib.pkgcache.PackageOptions;
import com.google.devtools.build.lib.runtime.BlazeCommand;
import com.google.devtools.build.lib.runtime.BlazeCommandResult;
import com.google.devtools.build.lib.runtime.Command;
Expand Down Expand Up @@ -100,10 +99,8 @@
@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
},
Expand Down Expand Up @@ -194,6 +191,7 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe
EvaluationContext.newBuilder()
.setParallelism(threadsOption.threads)
.setEventHandler(env.getReporter())
.setKeepGoing(options.getOptions(KeepGoingOption.class).keepGoing)
.build();

try {
Expand All @@ -208,11 +206,11 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe
keys.add(BazelDepGraphValue.KEY, BazelModuleInspectorValue.KEY);
}
EvaluationResult<SkyValue> evaluationResult =
skyframeExecutor.prepareAndGet(keys.build(), evaluationContext);
skyframeExecutor.getEvaluator().evaluate(keys.build(), evaluationContext);

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 +539,14 @@ 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,
"Not all extensions have been processed due to errors",
Code.ERROR_DURING_GRAPH_INSPECTION);
}
}

private BlazeCommandResult runTidy(CommandEnvironment env, BazelModTidyValue modTidyValue) {
Expand Down Expand Up @@ -587,7 +592,14 @@ private BlazeCommandResult runTidy(CommandEnvironment env, BazelModTidyValue mod
env.getReporter().handle(Event.info(fixupEvent.getSuccessMessage()));
}

return BlazeCommandResult.success();
if (modTidyValue.errors().isEmpty()) {
return BlazeCommandResult.success();
} else {
return reportAndCreateFailureResult(
env,
"Not all extensions have been processed due to errors",
Code.ERROR_DURING_GRAPH_INSPECTION);
}
}

/** Collects a list of {@link ModuleArg} into a set of {@link ModuleKey}s. */
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

0 comments on commit 4cbbc54

Please sign in to comment.