From d4131db93ca7eb5fcba60f504bcf5dfcf9abe52e Mon Sep 17 00:00:00 2001 From: Fabian Meumertzheim Date: Fri, 15 Sep 2023 13:11:18 -0700 Subject: [PATCH] Print dep chain leading to a module that is in error When a module file has an error, this now results in an error such as: ``` ERROR: Error computing the main repository mapping: in module dependency chain -> grpc@1.48.1.bcr.1 -> re2@2021-09-01: the MODULE.bazel file of re2@2021-09-01 declares a different version (2021-09-02) ``` Closes #19535. PiperOrigin-RevId: 565759770 Change-Id: I8ec788de4e83f59cc5ff5dc6f9fac19322181c08 --- .../bzlmod/BazelModuleResolutionFunction.java | 2 ++ .../build/lib/bazel/bzlmod/Discovery.java | 34 +++++++++++++++++-- .../build/lib/bazel/bzlmod/DiscoveryTest.java | 8 ++++- .../py/bazel/bzlmod/bazel_lockfile_test.py | 5 +-- 4 files changed, 44 insertions(+), 5 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java index dc9f79692bdf44..249f9f1578bd48 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/BazelModuleResolutionFunction.java @@ -120,6 +120,8 @@ private static Selection.Result discoverAndSelect( ImmutableMap initialDepGraph; try (SilentCloseable c = Profiler.instance().profile(ProfilerTask.BZLMOD, "discovery")) { initialDepGraph = Discovery.run(env, root); + } catch (ExternalDepsException e) { + throw new BazelModuleResolutionFunctionException(e, Transience.PERSISTENT); } if (initialDepGraph == null) { return null; diff --git a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java index d041c3ca0cda97..32f7d0a020c1c9 100644 --- a/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java +++ b/src/main/java/com/google/devtools/build/lib/bazel/bzlmod/Discovery.java @@ -15,15 +15,21 @@ package com.google.devtools.build.lib.bazel.bzlmod; +import static java.util.stream.Collectors.joining; + import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.bazel.bzlmod.InterimModule.DepSpec; import com.google.devtools.build.lib.bazel.bzlmod.ModuleFileValue.RootModuleFileValue; +import com.google.devtools.build.lib.server.FailureDetails; import com.google.devtools.build.skyframe.SkyFunction.Environment; import com.google.devtools.build.skyframe.SkyKey; import com.google.devtools.build.skyframe.SkyframeLookupResult; import java.util.ArrayDeque; +import java.util.ArrayList; +import java.util.Collections; import java.util.HashMap; import java.util.HashSet; +import java.util.List; import java.util.Map; import java.util.Queue; import java.util.Set; @@ -43,12 +49,14 @@ private Discovery() {} */ @Nullable public static ImmutableMap run( - Environment env, RootModuleFileValue root) throws InterruptedException { + Environment env, RootModuleFileValue root) + throws InterruptedException, ExternalDepsException { String rootModuleName = root.getModule().getName(); ImmutableMap overrides = root.getOverrides(); Map depGraph = new HashMap<>(); depGraph.put(ModuleKey.ROOT, rewriteDepSpecs(root.getModule(), overrides, rootModuleName)); Queue unexpanded = new ArrayDeque<>(); + Map predecessors = new HashMap<>(); unexpanded.add(ModuleKey.ROOT); while (!unexpanded.isEmpty()) { Set unexpandedSkyKeys = new HashSet<>(); @@ -58,6 +66,7 @@ public static ImmutableMap run( if (depGraph.containsKey(depSpec.toModuleKey())) { continue; } + predecessors.putIfAbsent(depSpec.toModuleKey(), module.getKey()); unexpandedSkyKeys.add( ModuleFileValue.key(depSpec.toModuleKey(), overrides.get(depSpec.getName()))); } @@ -65,7 +74,28 @@ public static ImmutableMap run( SkyframeLookupResult result = env.getValuesAndExceptions(unexpandedSkyKeys); for (SkyKey skyKey : unexpandedSkyKeys) { ModuleKey depKey = ((ModuleFileValue.Key) skyKey).getModuleKey(); - ModuleFileValue moduleFileValue = (ModuleFileValue) result.get(skyKey); + ModuleFileValue moduleFileValue; + try { + moduleFileValue = + (ModuleFileValue) result.getOrThrow(skyKey, ExternalDepsException.class); + } catch (ExternalDepsException e) { + // Trace back a dependency chain to the root module. There can be multiple paths to the + // failing module, but any of those is useful for debugging. + List depChain = new ArrayList<>(); + depChain.add(depKey); + ModuleKey predecessor = depKey; + while ((predecessor = predecessors.get(predecessor)) != null) { + depChain.add(predecessor); + } + Collections.reverse(depChain); + String depChainString = + depChain.stream().map(ModuleKey::toString).collect(joining(" -> ")); + throw ExternalDepsException.withCauseAndMessage( + FailureDetails.ExternalDeps.Code.BAD_MODULE, + e, + "in module dependency chain %s", + depChainString); + } if (moduleFileValue == null) { // Don't return yet. Try to expand any other unexpanded nodes before returning. depGraph.put(depKey, null); diff --git a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java index d8d885cd5c465b..99c03bd3854b29 100644 --- a/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java +++ b/src/test/java/com/google/devtools/build/lib/bazel/bzlmod/DiscoveryTest.java @@ -108,7 +108,13 @@ public SkyValue compute(SkyKey skyKey, Environment env) if (root == null) { return null; } - ImmutableMap depGraph = Discovery.run(env, root); + ImmutableMap depGraph; + try { + depGraph = Discovery.run(env, root); + } catch (ExternalDepsException e) { + throw new BazelModuleResolutionFunction.BazelModuleResolutionFunctionException( + e, SkyFunctionException.Transience.PERSISTENT); + } return depGraph == null ? null : DiscoveryValue.create(depGraph); } } diff --git a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py index 9d267d93ada69d..6cf2db2ba606bc 100644 --- a/src/test/py/bazel/bzlmod/bazel_lockfile_test.py +++ b/src/test/py/bazel/bzlmod/bazel_lockfile_test.py @@ -102,8 +102,9 @@ def testChangeModuleInRegistryWithoutLockfile(self): self.AssertExitCode(exit_code, 48, stderr) self.assertIn( ( - 'ERROR: Error computing the main repository mapping: error parsing' - ' MODULE.bazel file for sss@1.3' + 'ERROR: Error computing the main repository mapping: in module ' + 'dependency chain -> sss@1.3: error parsing MODULE.bazel ' + 'file for sss@1.3' ), stderr, )