commit d2a2461e69acef66401aa8dbcf00b06eb960de44
author Florian Weikert <fwe@google.com> Mon Sep 07 14:35:23 2015 +0000
committer Damien Martin-Guillerez <dmarting@google.com> Tue Sep 08 09:04:33 2015 +0000
tree 5d0f6cb235946e8bbed5266929b63c48c2971db0
parent 1540c7b1e06bc1d2b2b0fb5d65059204cc24d86f

Improved error message when accessing undeclared configuration fragments.

Users will get an error when rules access configuration fragments without having declared them as required fragments.
The error message now contains more details on the cause of the error and may also include a possible Skylark fix, if applicable.

--
MOS_MIGRATED_REVID=102497559

src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java

diff --git a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
index b6affca..16982f0 100644
--- a/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
+++ b/src/main/java/com/google/devtools/build/lib/analysis/RuleContext.java
@@ -259,12 +259,20 @@
    */
   @Nullable
   public <T extends Fragment> T getFragment(Class<T> fragment, ConfigurationTransition config) {
+    return getFragment(fragment, fragment.getSimpleName(), "", config);
+  }
+
+  @Nullable
+  protected <T extends Fragment> T getFragment(Class<T> fragment, String name,
+      String additionalErrorMessage, ConfigurationTransition config) {
     // TODO(bazel-team): The fragments can also be accessed directly through BuildConfiguration.
     // Can we lock that down somehow?
     Preconditions.checkArgument(isLegalFragment(fragment, config),
-        "%s does not have access to '%s' in %s configuration", rule.getRuleClass(),
-        fragment.getSimpleName(), FragmentCollection.getConfigurationName(config));
-    return getConfiguration(config).getFragment(fragment);
+        "%s has to declare '%s' as a required fragment "
+        + "in %s configuration in order to access it.%s",
+        rule.getRuleClass(), name, FragmentCollection.getConfigurationName(config),
+        additionalErrorMessage);
+    return getConfiguration().getFragment(fragment);
   }
 
   @Nullable
@@ -275,9 +283,16 @@
 
   @Nullable
   public Fragment getSkylarkFragment(String name, ConfigurationTransition config) {
-    Class<? extends Fragment> fragmentClass =
-        getConfiguration(config).getSkylarkFragmentByName(name);
-    return (fragmentClass == null) ? null : getFragment(fragmentClass, config);
+    Class<? extends Fragment> fragmentClass = getConfiguration().getSkylarkFragmentByName(name);
+    if (fragmentClass == null) {
+      return null;
+    }
+    return getFragment(fragmentClass, name,
+        String.format(
+            " Please update the '%1$sfragments' argument of the rule definition "
+            + "(for example: %1$sfragments = [\"%2$s\"])",
+            (config == ConfigurationTransition.HOST) ? "host_" : "", name),
+        config);
   }
 
   public ImmutableCollection<String> getSkylarkFragmentNames(ConfigurationTransition config) {
@@ -293,7 +308,7 @@
     // NONE means target configuration.
     return isLegalFragment(fragment, ConfigurationTransition.NONE);
   }
-  
+
   protected BuildConfiguration getConfiguration(ConfigurationTransition config) {
     return config.equals(ConfigurationTransition.HOST) ? hostConfiguration : getConfiguration();
   }