From 4bf016763cdab044dc09b6cdbebd8638b4e4579a Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Wed, 12 Jul 2023 13:44:07 -0700 Subject: [PATCH] Report dev/non-dev deps imported via non-dev/dev usages If a repository generated by a module extension corresponds to a dev tag but is imported on a non-dev usage proxy (or vice versa), the build can fail if the module is used by other modules. `ModuleExtensionMetadata` already generated fixup commands for this case, but now also explains the situation and impact. Closes #18832. PiperOrigin-RevId: 547589845 Change-Id: I58a63b1aa6b60849a350202097515c7840540800 --- .../bazel/bzlmod/ModuleExtensionMetadata.java | 22 +++++++++++ .../bzlmod/ModuleExtensionResolutionTest.java | 38 +++++++++++++++---- 2 files changed, 52 insertions(+), 8 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 e11c1356feee1f..f71986f509c8c9 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 @@ -253,6 +253,28 @@ private static Optional generateFixupMessage( String.join(", ", missingImports)); } + var nonDevImportsOfDevDeps = + ImmutableSortedSet.copyOf(Sets.intersection(expectedDevImports, actualImports)); + if (!nonDevImportsOfDevDeps.isEmpty()) { + message += + String.format( + "Imported as a regular dependency, but reported as a dev dependency by the " + + "extension (may cause the build to fail when used by other modules):\n" + + " %s\n\n", + String.join(", ", nonDevImportsOfDevDeps)); + } + + var devImportsOfNonDevDeps = + ImmutableSortedSet.copyOf(Sets.intersection(expectedImports, actualDevImports)); + if (!devImportsOfNonDevDeps.isEmpty()) { + message += + String.format( + "Imported as a dev dependency, but reported as a regular dependency by the " + + "extension (may cause the build to fail when used by other modules):\n" + + " %s\n\n", + String.join(", ", devImportsOfNonDevDeps)); + } + var indirectDepImports = ImmutableSortedSet.copyOf( Sets.difference(Sets.intersection(allActualImports, allRepos), allExpectedImports)); 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 d1ba4be42133a9..acecdf09bfa002 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 @@ -1765,6 +1765,7 @@ public void extensionMetadata() throws Exception { " ext,", " 'indirect_dep',", " 'invalid_dep',", + " 'dev_as_non_dev_dep',", " my_direct_dep = 'direct_dep',", ")", "ext_dev = use_extension('@ext//:defs.bzl', 'ext', dev_dependency = True)", @@ -1772,6 +1773,7 @@ public void extensionMetadata() throws Exception { " ext_dev,", " 'indirect_dev_dep',", " 'invalid_dev_dep',", + " 'non_dev_as_dev_dep',", " my_direct_dev_dep = 'direct_dev_dep',", ")"); scratch.file(workspaceRoot.getRelative("BUILD").getPathString()); @@ -1800,9 +1802,12 @@ public void extensionMetadata() throws Exception { " data_repo(name='missing_direct_dev_dep')", " data_repo(name='indirect_dep')", " data_repo(name='indirect_dev_dep')", + " data_repo(name='dev_as_non_dev_dep')", + " data_repo(name='non_dev_as_dev_dep')", " return ctx.extension_metadata(", - " root_module_direct_deps=['direct_dep', 'missing_direct_dep'],", - " root_module_direct_dev_deps=['direct_dev_dep', 'missing_direct_dev_dep'],", + " root_module_direct_deps=['direct_dep', 'missing_direct_dep', 'non_dev_as_dev_dep'],", + " root_module_direct_dev_deps=['direct_dev_dep', 'missing_direct_dev_dep'," + + " 'dev_as_non_dev_dep'],", " )", "ext=module_extension(implementation=_ext_impl)"); @@ -1826,19 +1831,28 @@ public void extensionMetadata() throws Exception { + " build to fail):\n" + " missing_direct_dep, missing_direct_dev_dep\n" + "\n" + + "Imported as a regular dependency, but reported as a dev dependency by the" + + " extension (may cause the build to fail when used by other modules):\n" + + " dev_as_non_dev_dep\n" + + "\n" + + "Imported as a dev dependency, but reported as a regular dependency by the" + + " extension (may cause the build to fail when used by other modules):\n" + + " non_dev_as_dev_dep\n" + + "\n" + "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" + " issues:\033[0m\n" + "\n" - + "buildozer 'use_repo_add @ext//:defs.bzl ext missing_direct_dep' //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove @ext//:defs.bzl ext indirect_dep invalid_dep'" - + " //MODULE.bazel:all\n" - + "buildozer 'use_repo_add dev @ext//:defs.bzl ext missing_direct_dev_dep'" + + "buildozer 'use_repo_add @ext//:defs.bzl ext missing_direct_dep non_dev_as_dev_dep'" + " //MODULE.bazel:all\n" - + "buildozer 'use_repo_remove dev @ext//:defs.bzl ext indirect_dev_dep invalid_dev_dep'" - + " //MODULE.bazel:all", + + "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" + + " non_dev_as_dev_dep' //MODULE.bazel:all", ImmutableSet.of(EventKind.WARNING)); } @@ -1908,6 +1922,10 @@ public void extensionMetadata_all() throws Exception { + " build to fail):\n" + " missing_direct_dep, missing_direct_dev_dep\n" + "\n" + + "Imported as a dev dependency, but reported as a regular dependency by the" + + " 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" + " issues:\033[0m\n" + "\n" @@ -1987,6 +2005,10 @@ public void extensionMetadata_allDev() throws Exception { + " build to fail):\n" + " missing_direct_dep, missing_direct_dev_dep\n" + "\n" + + "Imported as a regular dependency, but reported as a dev dependency by the" + + " 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" + " issues:\033[0m\n" + "\n"