diff options
author | 2017-11-20 13:12:58 -0800 | |
---|---|---|
committer | 2017-11-20 13:15:14 -0800 | |
commit | bff599bba6ace59104dbc5a5de17164b5fe25d1c (patch) | |
tree | 8181aee93d00c64f00a4a5eb9c7953a2c61d86df /src/main/java/com/google/devtools | |
parent | c5e9a4790288a24b22bc72761994775c029a676e (diff) |
Add warning for long chain of recursive --configs.
Configs are recursive, but let's be reasonable. If there's an absurdly long list of configs inheritance, warn about it. It is probably unnecessary, and might very well be unintentional and surprising to the user.
RELNOTES: None.
PiperOrigin-RevId: 176405183
Diffstat (limited to 'src/main/java/com/google/devtools')
-rw-r--r-- | src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java | 34 |
1 files changed, 31 insertions, 3 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java index a2ff946635..f2510a447e 100644 --- a/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java +++ b/src/main/java/com/google/devtools/build/lib/runtime/BlazeOptionHandler.java @@ -466,7 +466,24 @@ public abstract class BlazeOptionHandler { throws OptionsParsingException { LinkedHashSet<String> configAncestorSet = new LinkedHashSet<>(); configAncestorSet.add(configToExpand); - return getExpansion(eventHandler, commandToRcArgs, configAncestorSet, configToExpand); + List<String> longestChain = new ArrayList<>(); + List<String> finalExpansion = + getExpansion( + eventHandler, commandToRcArgs, configAncestorSet, configToExpand, longestChain); + + // In order to prevent warning about a long chain of 13 configs at the 10, 11, 12, and 13 + // point, we identify the longest chain for this 'high-level' --config found and only warn + // about it once. This may mean we missed a fork where each branch was independently long + // enough to warn, but the single warning should convey the message reasonably. + if (longestChain.size() >= 10) { + eventHandler.handle( + Event.warn( + String.format( + "There is a recursive chain of configs %s configs long: %s. This seems " + + "excessive, and might be hiding errors.", + longestChain.size(), longestChain))); + } + return finalExpansion; } /** @@ -477,12 +494,14 @@ public abstract class BlazeOptionHandler { * when we find another 'foo' to expand. However, {@code build:foo --config=bar}, {@code * build:foo --config=bar} is not a cycle just because bar is expanded twice, and the 1st * bar should not be in the parents list of the second bar. + * @param longestChain will be populated with the longest inheritance chain of configs. */ private List<String> getExpansion( EventHandler eventHandler, ListMultimap<String, RcChunkOfArgs> commandToRcArgs, LinkedHashSet<String> configAncestorSet, - String configToExpand) + String configToExpand, + List<String> longestChain) throws OptionsParsingException { List<String> expansion = new ArrayList<>(); boolean foundDefinition = false; @@ -518,9 +537,18 @@ public abstract class BlazeOptionHandler { + "see inheritance chain %s", newConfigValue, extendedConfigAncestorSet)); } + if (extendedConfigAncestorSet.size() > longestChain.size()) { + longestChain.clear(); + longestChain.addAll(extendedConfigAncestorSet); + } + expansion.addAll( getExpansion( - eventHandler, commandToRcArgs, extendedConfigAncestorSet, newConfigValue)); + eventHandler, + commandToRcArgs, + extendedConfigAncestorSet, + newConfigValue, + longestChain)); } } } |