diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index a63ed044491f2b..ee1bceeebe09fb 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", @@ -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", ], ) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java index 50c74aee8dc6c9..ecab25ebe61bfa 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelLockFileModule.java @@ -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; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java index 6cdf50a1374476..46431ea12a5e62 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyFunction.java @@ -21,6 +21,7 @@ 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; @@ -28,7 +29,6 @@ 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; @@ -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 { @@ -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 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); - } - } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java index 281d45cc547236..f6dd4ab08a7af4 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModTidyValue.java @@ -16,6 +16,7 @@ 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; @@ -23,6 +24,7 @@ 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; @@ -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 fixups(); + /** The path of the buildozer binary provided by the "buildozer" module. */ public abstract Path buildozer(); @@ -51,12 +56,14 @@ public abstract class BazelModTidyValue implements SkyValue { public abstract StarlarkSemantics starlarkSemantics(); static BazelModTidyValue create( + List fixups, Path buildozer, Map moduleOverrides, boolean ignoreDevDeps, LockfileMode lockfileMode, StarlarkSemantics starlarkSemantics) { return new AutoValue_BazelModTidyValue( + ImmutableList.copyOf(fixups), buildozer, ImmutableMap.copyOf(moduleOverrides), ignoreDevDeps, diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java index 2f18222a89686a..96e8983c19b543 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionMetadata.java @@ -19,7 +19,6 @@ 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; @@ -27,8 +26,7 @@ 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; @@ -173,14 +171,9 @@ static ModuleExtensionMetadata create( reproducible); } - public void evaluate( - Collection usages, Set allRepos, ExtendedEventHandler handler) + public Optional generateFixupMessage( + Collection usages, Set allRepos, EventHandler eventHandler) throws EvalException { - generateFixupMessage(usages, allRepos).forEach(reportable -> reportable.reportTo(handler)); - } - - private ImmutableList generateFixupMessage( - Collection usages, Set allRepos) throws EvalException { var rootUsages = usages.stream() .filter(usage -> usage.getUsingModule().equals(ModuleKey.ROOT)) @@ -188,7 +181,7 @@ private ImmutableList generateFixupMessage( 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); @@ -196,7 +189,7 @@ private ImmutableList generateFixupMessage( 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()); @@ -213,14 +206,19 @@ private ImmutableList generateFixupMessage( } return generateFixupMessage( - rootUsage, allRepos, rootModuleDirectDeps.get(), rootModuleDirectDevDeps.get()); + rootUsage, + allRepos, + rootModuleDirectDeps.get(), + rootModuleDirectDevDeps.get(), + eventHandler); } - private static ImmutableList generateFixupMessage( + private static Optional generateFixupMessage( ModuleExtensionUsage rootUsage, Set allRepos, Set expectedImports, - Set expectedDevImports) { + Set expectedDevImports, + EventHandler eventHandler) { var actualDevImports = ImmutableSet.copyOf(rootUsage.getDevImports()); var actualImports = rootUsage.getImports().values().stream() @@ -243,7 +241,7 @@ private static ImmutableList generateFixupMessage( && importsToRemove.isEmpty() && devImportsToAdd.isEmpty() && devImportsToRemove.isEmpty()) { - return ImmutableList.of(); + return Optional.empty(); } var message = @@ -343,8 +341,8 @@ private static ImmutableList 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 makeUseRepoCommand( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixupEvent.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixup.java similarity index 79% rename from src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixupEvent.java rename to src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixup.java index 6163127c2bfb62..2d3376489dcf0a 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixupEvent.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixup.java @@ -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 buildozerCommands; private final ModuleExtensionUsage usage; - public RootModuleFileFixupEvent(List buildozerCommands, ModuleExtensionUsage usage) { + public RootModuleFileFixup(List buildozerCommands, ModuleExtensionUsage usage) { this.buildozerCommands = ImmutableList.copyOf(buildozerCommands); this.usage = usage; } @@ -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; - } } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalValue.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalValue.java index 761ea61398524a..4d11575550e0b9 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalValue.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalValue.java @@ -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 @@ -49,26 +49,35 @@ public abstract class SingleExtensionEvalValue implements SkyValue { public abstract ImmutableBiMap 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 getLockFileInfo(); + + /** + * Returns the buildozer fixup for incorrect use_repo declarations by the root module (if any). + */ + public abstract Optional getFixup(); @AutoCodec.Instantiator public static SingleExtensionEvalValue create( ImmutableMap generatedRepoSpecs, ImmutableBiMap canonicalRepoNameToInternalNames, - @Nullable LockFileModuleExtension.WithFactors lockFileInfo) { + Optional lockFileInfo, + Optional 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); } diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUnvalidatedEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUnvalidatedEvalFunction.java index 264aa9c508cd2a..70513a9687054d 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUnvalidatedEvalFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionUnvalidatedEvalFunction.java @@ -210,32 +210,33 @@ public SkyValue compute(SkyKey skyKey, Environment env) } } - LockFileModuleExtension.WithFactors lockFileInfo; + Optional lockFileInfo; // At this point the extension has been evaluated successfully, but SingleExtensionEvalFunction // may still fail if imported repositories were not generated. However, since imports do not // influence the evaluation of the extension and the validation also runs when the extension - // result is taken from the lockfile, we can already post the update event. This is necessary to - // prevent the extension from rerunning when only the imports change. + // result is taken from the lockfile, we can already populate the lockfile info. This is + // necessary to prevent the extension from rerunning when only the imports change. if (lockfileMode.equals(LockfileMode.UPDATE)) { lockFileInfo = - new LockFileModuleExtension.WithFactors( - extension.getEvalFactors(), - LockFileModuleExtension.builder() - .setBzlTransitiveDigest(extension.getBzlTransitiveDigest()) - .setUsagesDigest( - ModuleExtensionUsage.hashForEvaluation( - GsonTypeAdapterUtil.createModuleExtensionUsagesHashGson(), - usagesValue.getExtensionUsages())) - .setRecordedFileInputs(moduleExtensionResult.getRecordedFileInputs()) - .setRecordedDirentsInputs(moduleExtensionResult.getRecordedDirentsInputs()) - .setEnvVariables(extension.getEnvVars()) - .setGeneratedRepoSpecs(generatedRepoSpecs) - .setModuleExtensionMetadata(moduleExtensionMetadata) - .setRecordedRepoMappingEntries( - moduleExtensionResult.getRecordedRepoMappingEntries()) - .build()); + Optional.of( + new LockFileModuleExtension.WithFactors( + extension.getEvalFactors(), + LockFileModuleExtension.builder() + .setBzlTransitiveDigest(extension.getBzlTransitiveDigest()) + .setUsagesDigest( + ModuleExtensionUsage.hashForEvaluation( + GsonTypeAdapterUtil.createModuleExtensionUsagesHashGson(), + usagesValue.getExtensionUsages())) + .setRecordedFileInputs(moduleExtensionResult.getRecordedFileInputs()) + .setRecordedDirentsInputs(moduleExtensionResult.getRecordedDirentsInputs()) + .setEnvVariables(extension.getEnvVars()) + .setGeneratedRepoSpecs(generatedRepoSpecs) + .setModuleExtensionMetadata(moduleExtensionMetadata) + .setRecordedRepoMappingEntries( + moduleExtensionResult.getRecordedRepoMappingEntries()) + .build())); } else { - lockFileInfo = null; + lockFileInfo = Optional.empty(); } return createSingleExtensionEvalValue( generatedRepoSpecs, moduleExtensionMetadata, extensionId, usagesValue, lockFileInfo, env); @@ -311,7 +312,7 @@ private SingleExtensionEvalValue tryGettingValueFromLockFile( lockedExtension.getModuleExtensionMetadata(), extensionId, usagesValue, - new LockFileModuleExtension.WithFactors(evalFactors, lockedExtension), + Optional.of(new LockFileModuleExtension.WithFactors(evalFactors, lockedExtension)), env); } if (lockfileMode.equals(LockfileMode.ERROR)) { @@ -425,21 +426,23 @@ private SingleExtensionEvalValue createSingleExtensionEvalValue( Optional moduleExtensionMetadata, ModuleExtensionId extensionId, SingleExtensionUsagesValue usagesValue, - @Nullable LockFileModuleExtension.WithFactors lockFileInfo, + Optional lockFileInfo, Environment env) throws SingleExtensionEvalFunctionException { // Evaluate the metadata before failing on invalid imports so that fixup warning are still // emitted in case of an error. + Optional fixup = Optional.empty(); if (moduleExtensionMetadata.isPresent()) { try { // TODO: ModuleExtensionMetadata#evaluate should throw ExternalDepsException instead of // EvalException. - moduleExtensionMetadata - .get() - .evaluate( - usagesValue.getExtensionUsages().values(), - generatedRepoSpecs.keySet(), - env.getListener()); + fixup = + moduleExtensionMetadata + .get() + .generateFixupMessage( + usagesValue.getExtensionUsages().values(), + generatedRepoSpecs.keySet(), + env.getListener()); } catch (EvalException e) { env.getListener().handle(Event.error(e.getMessageWithStack())); throw new SingleExtensionEvalFunctionException( @@ -461,7 +464,8 @@ private SingleExtensionEvalValue createSingleExtensionEvalValue( RepositoryName.createUnvalidated( usagesValue.getExtensionUniqueName() + "~" + e), Function.identity())), - lockFileInfo); + lockFileInfo, + fixup); } private BzlLoadValue loadBzlFile( diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD index 5c074c83ccabbc..a12cc7e49ad3f8 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/BUILD @@ -33,10 +33,10 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:common", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension", - "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_file_fixup_event", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:root_module_file_fixup", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:tidy", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod/modcommand", "//src/main/java/com/google/devtools/build/lib/bazel/repository", diff --git a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java index 4d40a27628cf29..8b27efe86419bc 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/commands/ModCommand.java @@ -41,7 +41,7 @@ import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileFunction; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue; import com.google.devtools.build.lib.bazel.bzlmod.ModuleKey; -import com.google.devtools.build.lib.bazel.bzlmod.RootModuleFileFixupEvent; +import com.google.devtools.build.lib.bazel.bzlmod.RootModuleFileFixup; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ExtensionArg; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.ExtensionArg.ExtensionArgConverter; import com.google.devtools.build.lib.bazel.bzlmod.modcommand.InvalidArgumentException; @@ -91,7 +91,6 @@ import java.io.IOException; import java.io.OutputStreamWriter; import java.io.Writer; -import java.util.ArrayList; import java.util.Collection; import java.util.List; import java.util.Map; @@ -566,14 +565,8 @@ private BlazeCommandResult execInternal(CommandEnvironment env, OptionsParsingRe } private static class TidyEventRecorder { - final List fixupEvents = new ArrayList<>(); @Nullable BazelModuleResolutionEvent bazelModuleResolutionEvent; - @Subscribe - public void fixupGenerated(RootModuleFileFixupEvent event) { - fixupEvents.add(event); - } - @Subscribe public void bazelModuleResolved(BazelModuleResolutionEvent event) { bazelModuleResolutionEvent = event; @@ -588,8 +581,8 @@ private BlazeCommandResult runTidy( .addArg(modTidyValue.buildozer().getPathString()) .addArgs( Stream.concat( - eventRecorder.fixupEvents.stream() - .map(RootModuleFileFixupEvent::getBuildozerCommands) + modTidyValue.fixups().stream() + .map(RootModuleFileFixup::getBuildozerCommands) .flatMap(Collection::stream), Stream.of("format")) .collect(toImmutableList())) @@ -613,7 +606,7 @@ private BlazeCommandResult runTidy( Code.BUILDOZER_FAILED); } - for (RootModuleFileFixupEvent fixupEvent : eventRecorder.fixupEvents) { + for (RootModuleFileFixup fixupEvent : modTidyValue.fixups()) { env.getReporter().handle(Event.info(fixupEvent.getSuccessMessage())); } diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD index 66f893176b9587..77f71773641ece 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/BUILD @@ -38,11 +38,11 @@ java_library( "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:inspection_impl", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_extension", - "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:module_file_fixup_event", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:registry", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:repo_rule_value", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution", "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:resolution_impl", + "//src/main/java/com/google/devtools/build/lib/bazel/bzlmod:root_module_file_fixup", "//src/main/java/com/google/devtools/build/lib/bazel/repository:repository_options", "//src/main/java/com/google/devtools/build/lib/bazel/repository/cache", "//src/main/java/com/google/devtools/build/lib/bazel/repository/downloader", diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java index b2cdddcac000e7..9d8f8e01784977 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/ModuleExtensionResolutionTest.java @@ -118,15 +118,15 @@ public class ModuleExtensionResolutionTest extends FoundationTestCase { private static class EventRecorder { // Keep in deterministic order even though events are posted in Skyframe evaluation order. - private final SortedSet fixupEvents = - new TreeSet<>(comparing(RootModuleFileFixupEvent::getSuccessMessage)); + private final SortedSet fixupEvents = + new TreeSet<>(comparing(RootModuleFileFixup::getSuccessMessage)); @Subscribe - public void onFixupEvent(RootModuleFileFixupEvent fixupEvent) { + public void onFixupEvent(RootModuleFileFixup fixupEvent) { fixupEvents.add(fixupEvent); } - public List fixupEvents() { + public List fixupEvents() { return ImmutableList.copyOf(fixupEvents); } } @@ -362,14 +362,6 @@ public void simpleExtension() throws Exception { throw result.getError().getException(); } assertThat(result.get(skyKey).getModule().getGlobal("data")).isEqualTo("foo:fu bar:ba"); - - // Verify that Bzlmod events do not end up being retained on regular build nodes unexpectedly. - // TODO: Get rid of BazelModuleResolutionEvent as well. - NodeEntry nodeEntry = evaluator.getExistingEntryAtCurrentlyEvaluatingVersion(skyKey); - assertThat( - ValueWithMetadata.getEvents(nodeEntry.getValueMaybeWithMetadata()).toList().stream() - .map(Object::getClass)) - .containsExactly(BazelModuleResolutionEvent.class); } @Test