Skip to content

Commit

Permalink
[7.5.0] Don't show fixup warnings during bazel mod tidy (#24846)
Browse files Browse the repository at this point in the history
The warnings tell the user to run `bazel mod tidy`, which is very
confusing.

Fixes #24495

Closes #24729.

PiperOrigin-RevId: 708007222
Change-Id: I60dc889281a776bbf08a7f9537272d7692cce1d8 
(cherry picked from commit 455ddb7)

Fixes #24740
  • Loading branch information
fmeum authored Jan 8, 2025
1 parent b9c7111 commit 09c29e3
Show file tree
Hide file tree
Showing 6 changed files with 20 additions and 22 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -459,6 +459,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",
],
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down Expand Up @@ -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()) {
Expand All @@ -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())
Expand Down Expand Up @@ -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) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

/**
Expand All @@ -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() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -470,8 +470,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(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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.getFixup().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()) {
Expand Down
10 changes: 3 additions & 7 deletions src/test/py/bazel/bzlmod/mod_command_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -682,13 +682,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,
Expand Down Expand Up @@ -984,10 +984,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
)
Expand Down

0 comments on commit 09c29e3

Please sign in to comment.