From 4cb09f7bf011e11434377b664d4584c3f221999e Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 28 Jul 2023 10:25:40 -0700 Subject: [PATCH] Merge `use_repo` buildifier fixups into a single command Users only have to copy&paste a single command this way. Closes #19119. PiperOrigin-RevId: 551887901 Change-Id: Id285563e83a18ac9a4db3b2d4e8d731d5b3f5813 --- .../bazel/bzlmod/ModuleExtensionMetadata.java | 16 +++---- .../bzlmod/ModuleExtensionResolutionTest.java | 44 ++++++++----------- 2 files changed, 26 insertions(+), 34 deletions(-) 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 f71986f509c8c9..a161cf0cff2e59 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 @@ -16,6 +16,7 @@ import static com.google.common.collect.ImmutableList.toImmutableList; import static com.google.common.collect.ImmutableSet.toImmutableSet; +import static java.util.stream.Collectors.joining; import com.google.common.base.Preconditions; import com.google.common.collect.ImmutableSet; @@ -31,7 +32,6 @@ import java.util.LinkedHashSet; import java.util.Optional; import java.util.Set; -import java.util.stream.Collectors; import java.util.stream.Stream; import javax.annotation.Nullable; import net.starlark.java.annot.StarlarkBuiltin; @@ -285,7 +285,7 @@ private static Optional generateFixupMessage( String.join(", ", indirectDepImports)); } - var fixupCommands = + var fixupCommand = Stream.of( makeUseRepoCommand( "use_repo_add", @@ -315,19 +315,18 @@ private static Optional generateFixupMessage( extensionBzlFile, extensionName, rootUsage.getIsolationKey())) - .flatMap(Optional::stream); + .flatMap(Optional::stream) + .collect(joining(" ", "buildozer ", " //MODULE.bazel:all")); return Optional.of( Event.warn( location, message + String.format( - "%s ** You can use the following buildozer command(s) to fix these" + "%s ** You can use the following buildozer command to fix these" + " issues:%s\n\n" + "%s", - "\033[35m\033[1m", - "\033[0m", - fixupCommands.collect(Collectors.joining("\n"))))); + "\033[35m\033[1m", "\033[0m", fixupCommand))); } private static Optional makeUseRepoCommand( @@ -354,8 +353,7 @@ private static Optional makeUseRepoCommand( commandParts.add(extensionName); } commandParts.addAll(repos); - return Optional.of( - String.format("buildozer '%s' //MODULE.bazel:all", String.join(" ", commandParts))); + return Optional.of(commandParts.stream().collect(joining(" ", "'", "'"))); } private Optional> getRootModuleDirectDeps(Set allRepos) 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 675a9ce4f7816c..5bd81dac41e5eb 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 @@ -1841,16 +1841,13 @@ public void extensionMetadata() throws Exception { + "Imported, but reported as indirect dependencies by the extension:\n" + " indirect_dep, indirect_dev_dep\n" + "\n" - + "\033[35m\033[1m ** You can use the following buildozer command(s) to fix these" + + "\033[35m\033[1m ** You can use the following buildozer command to fix these" + " issues:\033[0m\n" + "\n" + "buildozer 'use_repo_add @ext//:defs.bzl ext missing_direct_dep non_dev_as_dev_dep'" - + " //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove @ext//:defs.bzl ext dev_as_non_dev_dep" - + " indirect_dep invalid_dep' //MODULE.bazel:all\n" - + "buildozer 'use_repo_add dev @ext//:defs.bzl ext dev_as_non_dev_dep" - + " missing_direct_dev_dep' //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove dev @ext//:defs.bzl ext indirect_dev_dep invalid_dev_dep" + + " 'use_repo_remove @ext//:defs.bzl ext dev_as_non_dev_dep indirect_dep invalid_dep'" + + " 'use_repo_add dev @ext//:defs.bzl ext dev_as_non_dev_dep missing_direct_dev_dep'" + + " 'use_repo_remove dev @ext//:defs.bzl ext indirect_dev_dep invalid_dev_dep" + " non_dev_as_dev_dep' //MODULE.bazel:all", ImmutableSet.of(EventKind.WARNING)); } @@ -1925,14 +1922,13 @@ public void extensionMetadata_all() throws Exception { + " extension (may cause the build to fail when used by other modules):\n" + " direct_dev_dep, indirect_dev_dep\n" + "\n" - + "\033[35m\033[1m ** You can use the following buildozer command(s) to fix these" + + "\033[35m\033[1m ** You can use the following buildozer command to fix these" + " issues:\033[0m\n" + "\n" + "buildozer 'use_repo_add @ext//:defs.bzl ext direct_dev_dep indirect_dev_dep" - + " missing_direct_dep missing_direct_dev_dep' //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove @ext//:defs.bzl ext invalid_dep' //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove dev @ext//:defs.bzl ext direct_dev_dep indirect_dev_dep" - + " invalid_dev_dep' //MODULE.bazel:all", + + " missing_direct_dep missing_direct_dev_dep' 'use_repo_remove @ext//:defs.bzl ext" + + " invalid_dep' 'use_repo_remove dev @ext//:defs.bzl ext direct_dev_dep" + + " indirect_dev_dep invalid_dev_dep' //MODULE.bazel:all", ImmutableSet.of(EventKind.WARNING)); } @@ -2008,14 +2004,12 @@ public void extensionMetadata_allDev() throws Exception { + " extension (may cause the build to fail when used by other modules):\n" + " direct_dep, indirect_dep\n" + "\n" - + "\033[35m\033[1m ** You can use the following buildozer command(s) to fix these" + + "\033[35m\033[1m ** You can use the following buildozer command to fix these" + " issues:\033[0m\n" + "\n" + "buildozer 'use_repo_remove @ext//:defs.bzl ext direct_dep indirect_dep invalid_dep'" - + " //MODULE.bazel:all\n" - + "buildozer 'use_repo_add dev @ext//:defs.bzl ext direct_dep indirect_dep" - + " missing_direct_dep missing_direct_dev_dep' //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove dev @ext//:defs.bzl ext invalid_dev_dep'" + + " 'use_repo_add dev @ext//:defs.bzl ext direct_dep indirect_dep missing_direct_dep" + + " missing_direct_dev_dep' 'use_repo_remove dev @ext//:defs.bzl ext invalid_dev_dep'" + " //MODULE.bazel:all", ImmutableSet.of(EventKind.WARNING)); } @@ -2129,11 +2123,11 @@ public void extensionMetadata_isolated() throws Exception { + "Imported, but reported as indirect dependencies by the extension:\n" + " indirect_dep\n" + "\n" - + "\033[35m\033[1m ** You can use the following buildozer command(s) to fix these" + + "\033[35m\033[1m ** You can use the following buildozer command to fix these" + " issues:\033[0m\n" + "\n" - + "buildozer 'use_repo_add ext1 direct_dep missing_direct_dep' //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove ext1 indirect_dep' //MODULE.bazel:all", + + "buildozer 'use_repo_add ext1 direct_dep missing_direct_dep' 'use_repo_remove ext1" + + " indirect_dep' //MODULE.bazel:all", ImmutableSet.of(EventKind.WARNING)); assertContainsEvent( "WARNING /ws/MODULE.bazel:8:21: The module extension ext defined in @ext//:defs.bzl" @@ -2143,7 +2137,7 @@ public void extensionMetadata_isolated() throws Exception { + " build to fail):\n" + " missing_direct_dep\n" + "\n" - + "\033[35m\033[1m ** You can use the following buildozer command(s) to fix these" + + "\033[35m\033[1m ** You can use the following buildozer command to fix these" + " issues:\033[0m\n" + "\n" + "buildozer 'use_repo_add ext2 missing_direct_dep' //MODULE.bazel:all", @@ -2213,11 +2207,11 @@ public void extensionMetadata_isolatedDev() throws Exception { + "Imported, but reported as indirect dependencies by the extension:\n" + " indirect_dep\n" + "\n" - + "\033[35m\033[1m ** You can use the following buildozer command(s) to fix these" + + "\033[35m\033[1m ** You can use the following buildozer command to fix these" + " issues:\033[0m\n" + "\n" - + "buildozer 'use_repo_add ext1 direct_dep missing_direct_dep' //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove ext1 indirect_dep' //MODULE.bazel:all", + + "buildozer 'use_repo_add ext1 direct_dep missing_direct_dep' 'use_repo_remove ext1" + + " indirect_dep' //MODULE.bazel:all", ImmutableSet.of(EventKind.WARNING)); assertContainsEvent( "WARNING /ws/MODULE.bazel:8:21: The module extension ext defined in @ext//:defs.bzl" @@ -2227,7 +2221,7 @@ public void extensionMetadata_isolatedDev() throws Exception { + " build to fail):\n" + " missing_direct_dep\n" + "\n" - + "\033[35m\033[1m ** You can use the following buildozer command(s) to fix these" + + "\033[35m\033[1m ** You can use the following buildozer command to fix these" + " issues:\033[0m\n" + "\n" + "buildozer 'use_repo_add ext2 missing_direct_dep' //MODULE.bazel:all",