Don't show fixup warnings during bazel mod tidy

The warnings tell the user to run `bazel mod tidy`, which is very confusing. Fixes #24495 Closes #24729. PiperOrigin-RevId: 708007222 Change-Id: I60dc889281a776bbf08a7f9537272d7692cce1d8
bazelbuild · Dec 19, 2024 · 455ddb7 · 455ddb7
1 parent 0ce2d7d
commit 455ddb7
Show file tree

Hide file tree

Showing 6 changed files with 23 additions and 25 deletions.
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
@@ -458,6 +458,7 @@ java_library(
     ],
     deps = [
         ":module_extension",
+        "//src/main/java/com/google/devtools/build/lib/events",
         "//src/main/java/com/google/devtools/build/lib/vfs:pathfragment",
         "//third_party:guava",
     ],

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
@@ -26,7 +26,6 @@
 import com.google.devtools.build.lib.bazel.bzlmod.ModuleExtensionUsage.Proxy;
 import com.google.devtools.build.lib.cmdline.RepositoryName;
 import com.google.devtools.build.lib.events.Event;
-import com.google.devtools.build.lib.events.EventHandler;
 import com.google.devtools.build.lib.vfs.PathFragment;
 import com.ryanharter.auto.value.gson.GenerateTypeAdapter;
 import java.util.ArrayList;
@@ -171,8 +170,7 @@ static ModuleExtensionMetadata create(
   }
 
   public Optional<RootModuleFileFixup> generateFixup(
-      ModuleExtensionUsage rootUsage, Set<String> allRepos, EventHandler eventHandler)
-      throws EvalException {
+      ModuleExtensionUsage rootUsage, Set<String> allRepos) throws EvalException {
     var rootModuleDirectDevDeps = getRootModuleDirectDevDeps(allRepos);
     var rootModuleDirectDeps = getRootModuleDirectDeps(allRepos);
     if (rootModuleDirectDevDeps.isEmpty() && rootModuleDirectDeps.isEmpty()) {
@@ -193,19 +191,14 @@ public Optional<RootModuleFileFixup> generateFixup(
     }
 
     return generateFixup(
-        rootUsage,
-        allRepos,
-        rootModuleDirectDeps.get(),
-        rootModuleDirectDevDeps.get(),
-        eventHandler);
+        rootUsage, allRepos, rootModuleDirectDeps.get(), rootModuleDirectDevDeps.get());
   }
 
   private static Optional<RootModuleFileFixup> generateFixup(
       ModuleExtensionUsage rootUsage,
       Set<String> allRepos,
       Set<String> expectedImports,
-      Set<String> expectedDevImports,
-      EventHandler eventHandler) {
+      Set<String> expectedDevImports) {
     var actualDevImports =
         rootUsage.getProxies().stream()
             .filter(p -> p.isDevDependency())
@@ -329,8 +322,11 @@ private static Optional<RootModuleFileFixup> generateFixup(
       }
     }
 
-    eventHandler.handle(Event.warn(rootUsage.getProxies().getFirst().getLocation(), message));
-    return Optional.of(new RootModuleFileFixup(moduleFilePathToCommandsBuilder.build(), rootUsage));
+    return Optional.of(
+        new RootModuleFileFixup(
+            moduleFilePathToCommandsBuilder.build(),
+            rootUsage,
+            Event.warn(rootUsage.getProxies().getFirst().getLocation(), message)));
   }
 
   private static String makeUseRepoCommand(String cmd, String proxyName, Collection<String> repos) {

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixup.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/RootModuleFileFixup.java
@@ -16,6 +16,7 @@
 package com.google.devtools.build.lib.bazel.bzlmod;
 
 import com.google.common.collect.ImmutableListMultimap;
+import com.google.devtools.build.lib.events.Event;
 import com.google.devtools.build.lib.vfs.PathFragment;
 
 /**
@@ -29,7 +30,8 @@
  */
 public record RootModuleFileFixup(
     ImmutableListMultimap<PathFragment, String> moduleFilePathToBuildozerCommands,
-    ModuleExtensionUsage usage) {
+    ModuleExtensionUsage usage,
+    Event warning) {
 
   /** A human-readable message describing the fixup after it has been applied. */
   public String getSuccessMessage() {

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionEvalFunction.java
@@ -459,8 +459,7 @@ private SingleExtensionValue createSingleExtensionValue(
                 .get()
                 .generateFixup(
                     usagesValue.getExtensionUsages().get(ModuleKey.ROOT),
-                    generatedRepoSpecs.keySet(),
-                    env.getListener());
+                    generatedRepoSpecs.keySet());
       } catch (EvalException e) {
         env.getListener().handle(Event.error(e.getInnermostLocation(), e.getMessageWithStack()));
         throw new SingleExtensionEvalFunctionException(

diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/SingleExtensionFunction.java
@@ -48,6 +48,10 @@ public SkyValue compute(SkyKey skyKey, Environment env)
       return null;
     }
 
+    // SingleExtensionEvalFunction doesn't handle the fixup warning so that bazel mod tidy doesn't
+    // show it.
+    evalOnlyValue.fixup().ifPresent(fixup -> env.getListener().handle(fixup.warning()));
+
     // Check that all imported repos have actually been generated.
     for (ModuleExtensionUsage usage : usagesValue.getExtensionUsages().values()) {
       for (ModuleExtensionUsage.Proxy proxy : usage.getProxies()) {

diff --git a/src/test/py/bazel/bzlmod/mod_command_test.py b/src/test/py/bazel/bzlmod/mod_command_test.py
@@ -735,13 +735,13 @@ def testModTidy(self):
     # The extensions should not be reevaluated by the command.
     self.assertNotIn('ext1 is being evaluated', stderr)
     self.assertNotIn('ext2 is being evaluated', stderr)
-    # The fixup warnings should be shown again due to Skyframe replaying.
-    self.assertIn(
+    # bazel mod tidy doesn't show fixup warnings.
+    self.assertNotIn(
         'Not imported, but reported as direct dependencies by the extension'
         ' (may cause the build to fail):\nmissing_dep',
         stderr,
     )
-    self.assertIn(
+    self.assertNotIn(
         'Imported, but reported as indirect dependencies by the'
         ' extension:\nindirect_dep',
         stderr,
@@ -1085,13 +1085,13 @@ def testModTidyKeepGoing(self):
     # The passing extension should not be reevaluated by the command.
     self.assertNotIn('ext1 is being evaluated', stderr)
     self.assertIn('ext2 is being evaluated', stderr)
-    # The fixup warnings should be shown again due to Skyframe replaying.
-    self.assertIn(
+    # baze mod tidy doesn't show fixup warnings.
+    self.assertNotIn(
         'Not imported, but reported as direct dependencies by the extension'
         ' (may cause the build to fail):\nmissing_dep',
         stderr,
     )
-    self.assertIn(
+    self.assertNotIn(
         'Imported, but reported as indirect dependencies by the'
         ' extension:\nindirect_dep',
         stderr,
@@ -1178,10 +1178,6 @@ def testModTidyFixesInvalidImport(self):
     # extension fails after evaluation.
     _, _, stderr = self.RunBazel(['mod', 'tidy'])
     stderr = '\n'.join(stderr)
-    self.assertIn(
-        'ext defined in @//:extension.bzl reported incorrect imports', stderr
-    )
-    self.assertIn('invalid_dep', stderr)
     self.assertIn(
         'INFO: Updated use_repo calls for @//:extension.bzl%ext', stderr
     )