Report dev/non-dev deps imported via non-dev/dev usages

fmeum · copybara-github · commit 4bf016763cda · 2023-07-12T13:46:14.000-07:00
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
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
@@ -253,6 +253,28 @@ private static Optional<Event> 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
@@ -1765,13 +1765,15 @@ 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)",
         "use_repo(",
         "  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"
