aboutsummaryrefslogtreecommitdiffhomepage
path: root/src/main/java/com/google/devtools
diff options
context:
space:
mode:
authorGravatar ccalvarin <ccalvarin@google.com>2017-11-20 13:12:58 -0800
committerGravatar Copybara-Service <copybara-piper@google.com>2017-11-20 13:15:14 -0800
commitbff599bba6ace59104dbc5a5de17164b5fe25d1c (patch)
tree8181aee93d00c64f00a4a5eb9c7953a2c61d86df /src/main/java/com/google/devtools
parentc5e9a4790288a24b22bc72761994775c029a676e (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.java34
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));
}
}
}