Skip to content

Commit

Permalink
Report dev/non-dev deps imported via non-dev/dev usages
Browse files Browse the repository at this point in the history
If a repository generated by a module extension corresponds to a dev tag
but is imported on non-dev usage (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.
  • Loading branch information
fmeum committed Jul 3, 2023
1 parent 815b6c9 commit 95e790e
Show file tree
Hide file tree
Showing 2 changed files with 51 additions and 8 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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());
Expand Down Expand Up @@ -1800,9 +1802,11 @@ 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)");

Expand All @@ -1826,19 +1830,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));
}

Expand Down Expand Up @@ -1908,6 +1921,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"
Expand Down Expand Up @@ -1987,6 +2004,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"
Expand Down

0 comments on commit 95e790e

Please sign in to comment.