Skip to content

Commit

Permalink
Remove RootModuleFileFixupEvent
Browse files Browse the repository at this point in the history
  • Loading branch information
@fmeum
fmeum committed Apr 20, 2024
1 parent 2c9ec39 commit 8adb93b
Show file tree
Hide file tree
Showing 12 changed files with 107 additions and 118 deletions.
11 changes: 7 additions & 4 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 @@ -153,6 +153,7 @@ java_library(
":module_extension_metadata",
":registry",
":repo_rule_creator",
":root_module_file_fixup",
"//src/main/java/com/google/devtools/build/docgen/annot",
"//src/main/java/com/google/devtools/build/lib/analysis:blaze_directories",
"//src/main/java/com/google/devtools/build/lib/cmdline",
Expand Down Expand Up @@ -211,6 +212,7 @@ java_library(
":repo_rule_creator",
":repo_rule_value",
":resolution",
":root_module_file_fixup",
"//src/main/java/com/google/devtools/build/docgen/annot",
"//src/main/java/com/google/devtools/build/lib:runtime",
"//src/main/java/com/google/devtools/build/lib/actions:file_metadata",
Expand Down Expand Up @@ -289,6 +291,7 @@ java_library(
],
deps = [
":resolution",
":root_module_file_fixup",
"//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options",
"//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 @@ -310,6 +313,7 @@ java_library(
":exception",
":resolution",
":resolution_impl",
":root_module_file_fixup",
":tidy",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/rules:repository/repository_function",
Expand Down Expand Up @@ -367,7 +371,7 @@ java_library(
deps = [
":common",
":module_extension",
":module_file_fixup_event",
":root_module_file_fixup",
"//src/main/java/com/google/devtools/build/docgen/annot",
"//src/main/java/com/google/devtools/build/lib/cmdline",
"//src/main/java/com/google/devtools/build/lib/events",
Expand All @@ -382,13 +386,12 @@ java_library(
)

java_library(
name = "module_file_fixup_event",
name = "root_module_file_fixup",
srcs = [
"RootModuleFileFixupEvent.java",
"RootModuleFileFixup.java",
],
deps = [
":module_extension",
"//src/main/java/com/google/devtools/build/lib/events",
"//third_party:guava",
],
)
3 changes: 2 additions & 1 deletion src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -77,7 +77,8 @@ public void afterCommand() throws AbruptExitException {
.collect(
toImmutableMap(
entry -> ((SingleExtensionEvalValue.UnvalidatedKey) entry.getKey()).argument(),
entry -> ((SingleExtensionEvalValue) entry.getValue()).getLockFileInfo()));
entry ->
((SingleExtensionEvalValue) entry.getValue()).getLockFileInfo().get()));

BazelLockFileValue oldLockfile = moduleResolutionEvent.getOnDiskLockfileValue();
BazelLockFileValue newLockfile;
Expand Down
36 changes: 12 additions & 24 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 @@ -21,14 +21,14 @@
import static com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction.MODULE_OVERRIDES;
import static com.google.devtools.build.lib.skyframe.PrecomputedValue.STARLARK_SEMANTICS;

import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.common.collect.ImmutableSet;
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.rules.repository.NeedsSkyframeRestartException;
import com.google.devtools.build.lib.rules.repository.RepositoryFunction;
import com.google.devtools.build.lib.server.FailureDetails;
import com.google.devtools.build.lib.skyframe.RepositoryMappingValue;
import com.google.devtools.build.lib.vfs.RootedPath;
import com.google.devtools.build.skyframe.SkyFunction;
Expand All @@ -40,9 +40,8 @@
import net.starlark.java.eval.EvalException;

/**
* Computes all information required for the {@code bazel mod tidy} command. The evaluation of all
* module extensions used by the root module is triggered to, as a side effect, emit any {@link
* RootModuleFileFixupEvent}s.
* Computes all information required for the {@code bazel mod tidy} command, which in particular
* requires evaluating all module extensions used by the root module.
*/
public class BazelModTidyFunction implements SkyFunction {

Expand Down Expand Up @@ -88,40 +87,29 @@ public SkyValue compute(SkyKey skyKey, Environment env)
.getOrDefault(ModuleKey.ROOT, ImmutableMap.of())
.keySet()
.stream()
.map(SingleExtensionEvalValue::key)
// Use the unvalidated key to avoid errors caused by incorrect imports - we can fix
// them.
.map(SingleExtensionEvalValue::unvalidatedKey)
.collect(toImmutableSet());
SkyframeLookupResult result = env.getValuesAndExceptions(extensionsUsedByRootModule);
if (env.valuesMissing()) {
return null;
}
ImmutableList.Builder<RootModuleFileFixup> fixups = ImmutableList.builder();
for (SkyKey extension : extensionsUsedByRootModule) {
try {
result.getOrThrow(extension, ExternalDepsException.class);
} catch (ExternalDepsException e) {
if (e.getDetailedExitCode().getFailureDetail() == null
|| !e.getDetailedExitCode()
.getFailureDetail()
.getExternalDeps()
.getCode()
.equals(FailureDetails.ExternalDeps.Code.INVALID_EXTENSION_IMPORT)) {
throw new BazelModTidyFunctionException(e, SkyFunctionException.Transience.PERSISTENT);
}
// This is an error bazel mod tidy can fix, so don't fail.
if (result.get(extension) instanceof SingleExtensionEvalValue evalValue) {
evalValue.getFixup().ifPresent(fixups::add);
} else {
return null;
}
}

return BazelModTidyValue.create(
fixups.build(),
buildozer.asPath(),
MODULE_OVERRIDES.get(env),
IGNORE_DEV_DEPS.get(env),
LOCKFILE_MODE.get(env),
STARLARK_SEMANTICS.get(env));
}

static final class BazelModTidyFunctionException extends SkyFunctionException {

BazelModTidyFunctionException(ExternalDepsException cause, Transience transience) {
super(cause, transience);
}
}
}
7 changes: 7 additions & 0 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 @@ -16,13 +16,15 @@
package com.google.devtools.build.lib.bazel.bzlmod;

import com.google.auto.value.AutoValue;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableMap;
import com.google.devtools.build.lib.bazel.repository.RepositoryOptions.LockfileMode;
import com.google.devtools.build.lib.skyframe.SkyFunctions;
import com.google.devtools.build.lib.skyframe.serialization.autocodec.SerializationConstant;
import com.google.devtools.build.lib.vfs.Path;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import java.util.List;
import java.util.Map;
import net.starlark.java.eval.StarlarkSemantics;

Expand All @@ -32,6 +34,9 @@ public abstract class BazelModTidyValue implements SkyValue {

@SerializationConstant public static final SkyKey KEY = () -> SkyFunctions.BAZEL_MOD_TIDY;

/** Buildozer fixups for incorrect use_repo declarations by the root module. */
public abstract ImmutableList<RootModuleFileFixup> fixups();

/** The path of the buildozer binary provided by the "buildozer" module. */
public abstract Path buildozer();

Expand All @@ -51,12 +56,14 @@ public abstract class BazelModTidyValue implements SkyValue {
public abstract StarlarkSemantics starlarkSemantics();

static BazelModTidyValue create(
List<RootModuleFileFixup> fixups,
Path buildozer,
Map<String, ModuleOverride> moduleOverrides,
boolean ignoreDevDeps,
LockfileMode lockfileMode,
StarlarkSemantics starlarkSemantics) {
return new AutoValue_BazelModTidyValue(
ImmutableList.copyOf(fixups),
buildozer,
ImmutableMap.copyOf(moduleOverrides),
ignoreDevDeps,
Expand Down
34 changes: 16 additions & 18 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -19,16 +19,14 @@

import com.google.auto.value.AutoValue;
import com.google.common.base.Preconditions;
import com.google.common.collect.ImmutableList;
import com.google.common.collect.ImmutableSet;
import com.google.common.collect.ImmutableSortedSet;
import com.google.common.collect.Iterables;
import com.google.common.collect.Sets;
import com.google.devtools.build.docgen.annot.DocCategory;
import com.google.devtools.build.lib.cmdline.RepositoryName;
import com.google.devtools.build.lib.events.Event;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import com.google.devtools.build.lib.events.Reportable;
import com.google.devtools.build.lib.events.EventHandler;
import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
import java.util.ArrayList;
import java.util.Collection;
Expand Down Expand Up @@ -173,30 +171,25 @@ static ModuleExtensionMetadata create(
reproducible);
}

public void evaluate(
Collection<ModuleExtensionUsage> usages, Set<String> allRepos, ExtendedEventHandler handler)
public Optional<RootModuleFileFixup> generateFixupMessage(
Collection<ModuleExtensionUsage> usages, Set<String> allRepos, EventHandler eventHandler)
throws EvalException {
generateFixupMessage(usages, allRepos).forEach(reportable -> reportable.reportTo(handler));
}

private ImmutableList<Reportable> generateFixupMessage(
Collection<ModuleExtensionUsage> usages, Set<String> allRepos) throws EvalException {
var rootUsages =
usages.stream()
.filter(usage -> usage.getUsingModule().equals(ModuleKey.ROOT))
.collect(toImmutableList());
if (rootUsages.isEmpty()) {
// The root module doesn't use the current extension. Do not suggest fixes as the user isn't
// expected to modify any other module's MODULE.bazel file.
return ImmutableList.of();
return Optional.empty();
}
// Every module only has at most a single usage of a given extension.
ModuleExtensionUsage rootUsage = Iterables.getOnlyElement(rootUsages);

var rootModuleDirectDevDeps = getRootModuleDirectDevDeps(allRepos);
var rootModuleDirectDeps = getRootModuleDirectDeps(allRepos);
if (rootModuleDirectDevDeps.isEmpty() && rootModuleDirectDeps.isEmpty()) {
return ImmutableList.of();
return Optional.empty();
}
Preconditions.checkState(
rootModuleDirectDevDeps.isPresent() && rootModuleDirectDeps.isPresent());
Expand All @@ -213,14 +206,19 @@ private ImmutableList<Reportable> generateFixupMessage(
}

return generateFixupMessage(
rootUsage, allRepos, rootModuleDirectDeps.get(), rootModuleDirectDevDeps.get());
rootUsage,
allRepos,
rootModuleDirectDeps.get(),
rootModuleDirectDevDeps.get(),
eventHandler);
}

private static ImmutableList<Reportable> generateFixupMessage(
private static Optional<RootModuleFileFixup> generateFixupMessage(
ModuleExtensionUsage rootUsage,
Set<String> allRepos,
Set<String> expectedImports,
Set<String> expectedDevImports) {
Set<String> expectedDevImports,
EventHandler eventHandler) {
var actualDevImports = ImmutableSet.copyOf(rootUsage.getDevImports());
var actualImports =
rootUsage.getImports().values().stream()
Expand All @@ -243,7 +241,7 @@ private static ImmutableList<Reportable> generateFixupMessage(
&& importsToRemove.isEmpty()
&& devImportsToAdd.isEmpty()
&& devImportsToRemove.isEmpty()) {
return ImmutableList.of();
return Optional.empty();
}

var message =
Expand Down Expand Up @@ -343,8 +341,8 @@ private static ImmutableList<Reportable> generateFixupMessage(
.flatMap(Optional::stream)
.collect(toImmutableList());

return ImmutableList.of(
Event.warn(location, message), new RootModuleFileFixupEvent(buildozerCommands, rootUsage));
eventHandler.handle(Event.warn(location, message));
return Optional.of(new RootModuleFileFixup(buildozerCommands, rootUsage));
}

private static Optional<String> makeUseRepoCommand(
Expand Down
14 changes: 4 additions & 10 deletions ...azel/bzlmod/RootModuleFileFixupEvent.java → ...lib/bazel/bzlmod/RootModuleFileFixup.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,19 +16,18 @@
package com.google.devtools.build.lib.bazel.bzlmod;

import com.google.common.collect.ImmutableList;
import com.google.devtools.build.lib.events.ExtendedEventHandler;
import java.util.List;

/**
* Generated when incorrect use_repo calls are detected in the root module file according to {@link
* ModuleExtensionMetadata}, this event contains the buildozer commands required to bring the root
* module file into the expected state.
* ModuleExtensionMetadata} and contains the buildozer commands required to bring the root module
* file into the expected state.
*/
public final class RootModuleFileFixupEvent implements ExtendedEventHandler.Postable {
public final class RootModuleFileFixup {
private final ImmutableList<String> buildozerCommands;
private final ModuleExtensionUsage usage;

public RootModuleFileFixupEvent(List<String> buildozerCommands, ModuleExtensionUsage usage) {
public RootModuleFileFixup(List<String> buildozerCommands, ModuleExtensionUsage usage) {
this.buildozerCommands = ImmutableList.copyOf(buildozerCommands);
this.usage = usage;
}
Expand All @@ -50,9 +49,4 @@ public String getSuccessMessage() {
key.getUsageExportedName(), extensionId))
.orElseGet(() -> String.format("Updated use_repo calls for %s", extensionId));
}

@Override
public boolean storeForReplay() {
return true;
}
}
23 changes: 16 additions & 7 deletions src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalValue.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@
import com.google.devtools.build.skyframe.SkyFunctionName;
import com.google.devtools.build.skyframe.SkyKey;
import com.google.devtools.build.skyframe.SkyValue;
import javax.annotation.Nullable;
import java.util.Optional;

/**
* The result of evaluating a single module extension (see {@link
Expand All @@ -49,26 +49,35 @@ public abstract class SingleExtensionEvalValue implements SkyValue {
public abstract ImmutableBiMap<RepositoryName, String> getCanonicalRepoNameToInternalNames();

/**
* Returns the information stored about the extension in the lockfile. Is null if the lockfile
* Returns the information stored about the extension in the lockfile. Is empty if the lockfile
* mode is not UPDATE.
*/
@Nullable
public abstract LockFileModuleExtension.WithFactors getLockFileInfo();
public abstract Optional<LockFileModuleExtension.WithFactors> getLockFileInfo();

/**
* Returns the buildozer fixup for incorrect use_repo declarations by the root module (if any).
*/
public abstract Optional<RootModuleFileFixup> getFixup();

@AutoCodec.Instantiator
public static SingleExtensionEvalValue create(
ImmutableMap<String, RepoSpec> generatedRepoSpecs,
ImmutableBiMap<RepositoryName, String> canonicalRepoNameToInternalNames,
@Nullable LockFileModuleExtension.WithFactors lockFileInfo) {
Optional<LockFileModuleExtension.WithFactors> lockFileInfo,
Optional<RootModuleFileFixup> fixup) {
return new AutoValue_SingleExtensionEvalValue(
generatedRepoSpecs, canonicalRepoNameToInternalNames, lockFileInfo);
generatedRepoSpecs, canonicalRepoNameToInternalNames, lockFileInfo, fixup);
}

public static Key key(ModuleExtensionId id) {
return Key.create(id);
}

/** For use by {@link SingleExtensionEvalFunction only}. */
/**
* Provides access to {@link SingleExtensionEvalValue} without validating the imports for the
* repositories generated by the extension. This is only meant for special applications such as
* {@code bazel mod tidy}.
*/
static UnvalidatedKey unvalidatedKey(ModuleExtensionId id) {
return UnvalidatedKey.create(id);
}
Expand Down
Loading

0 comments on commit 8adb93b

Please sign in to comment.