RELNOTES[NEW]: If grte_top is a label, it can now follow non-configur…

…able redirects.

--
PiperOrigin-RevId: 1506473
MOS_MIGRATED_REVID=150647389

src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java

@@ -24,10 +24,12 @@
 import com.google.common.collect.Iterables;
 import com.google.common.collect.ListMultimap;
 import com.google.common.collect.Maps;
+import com.google.devtools.build.lib.analysis.RedirectChaser;
 import com.google.devtools.build.lib.analysis.RuleContext;
 import com.google.devtools.build.lib.analysis.config.BuildConfiguration;
 import com.google.devtools.build.lib.analysis.config.BuildOptions;
 import com.google.devtools.build.lib.analysis.config.CompilationMode;
+import com.google.devtools.build.lib.analysis.config.ConfigurationEnvironment;
 import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException;
 import com.google.devtools.build.lib.analysis.config.PerLabelOptions;
 import com.google.devtools.build.lib.cmdline.Label;
@@ -63,6 +65,7 @@
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import javax.annotation.Nullable;
 
 /**
  * This class represents the C/C++ parts of the {@link BuildConfiguration}, including the host
@@ -156,7 +159,89 @@ public String toString() {
   public static class LibcTop implements Serializable {
     private final Label label;
 
-    LibcTop(Label label) {
+    /**
+     * Result of trying to create LibcTop. Bundles whether or not it still needs values that aren't
+     * loaded yet.
+     */
+    public static class Result {
+      private final LibcTop libcTop;
+      private final boolean valuesMissing;
+
+      Result(LibcTop libcTop, boolean valuesMissing) {
+        this.libcTop = libcTop;
+        this.valuesMissing = valuesMissing;
+      }
+
+      @Nullable
+      public LibcTop getLibcTop() {
+        return libcTop;
+      }
+
+      public boolean valuesMissing() {
+        return valuesMissing;
+      }
+    };
+
+    /** Tries to create a LibcTop object and returns whether or not all were any missing values. */
+    public static LibcTop.Result createLibcTop(
+        CppOptions cppOptions, ConfigurationEnvironment env, CrosstoolConfig.CToolchain toolchain)
+        throws InterruptedException, InvalidConfigurationException {
+      Preconditions.checkArgument(env != null);
+
+      PathFragment defaultSysroot =
+          toolchain.getBuiltinSysroot().length() == 0
+              ? null
+              : new PathFragment(toolchain.getBuiltinSysroot());
+      if ((defaultSysroot != null) && !defaultSysroot.isNormalized()) {
+        throw new InvalidConfigurationException(
+            "The built-in sysroot '" + defaultSysroot + "' is not normalized.");
+      }
+
+      if ((cppOptions.libcTopLabel != null) && (defaultSysroot == null)) {
+        throw new InvalidConfigurationException(
+            "The selected toolchain "
+                + toolchain.getToolchainIdentifier()
+                + " does not support setting --grte_top.");
+      }
+
+      LibcTop libcTop = null;
+      if (cppOptions.libcTopLabel != null) {
+        libcTop = createLibcTop(cppOptions.libcTopLabel, env);
+        if (libcTop == null) {
+          return new Result(libcTop, true);
+        }
+      } else if (!toolchain.getDefaultGrteTop().isEmpty()) {
+        Label grteTopLabel = null;
+        try {
+          grteTopLabel =
+              new CppOptions.LibcTopLabelConverter().convert(toolchain.getDefaultGrteTop());
+        } catch (OptionsParsingException e) {
+          throw new InvalidConfigurationException(e.getMessage(), e);
+        }
+
+        libcTop = createLibcTop(grteTopLabel, env);
+        if (libcTop == null) {
+          return new Result(libcTop, true);
+        }
+      }
+      return new Result(libcTop, false);
+    }
+
+    @Nullable
+    private static LibcTop createLibcTop(Label label, ConfigurationEnvironment env)
+        throws InvalidConfigurationException, InterruptedException {
+      Preconditions.checkArgument(label != null);
+      Label trueLabel = label;
+      // Allow the sysroot to follow any redirects.
+      trueLabel = RedirectChaser.followRedirects(env, label, "libc_top");
+      if (trueLabel == null) {
+        // Happens if there's a reference to package that hasn't been loaded yet.
+        return null;
+      }
+      return new LibcTop(trueLabel);
+    }
+
+    private LibcTop(Label label) {
       Preconditions.checkArgument(label != null);
       this.label = label;
     }
@@ -526,18 +611,7 @@ public boolean apply(Tool tool) {
           + "' is not normalized.");
     }
 
-    if ((cppOptions.libcTop != null) && (defaultSysroot == null)) {
-      throw new InvalidConfigurationException("The selected toolchain " + toolchainIdentifier
-          + " does not support setting --grte_top.");
-    }
-    LibcTop libcTop = cppOptions.libcTop;
-    if ((libcTop == null) && !toolchain.getDefaultGrteTop().isEmpty()) {
-      try {
-        libcTop = new CppOptions.LibcTopConverter().convert(toolchain.getDefaultGrteTop());
-      } catch (OptionsParsingException e) {
-        throw new InvalidConfigurationException(e.getMessage(), e);
-      }
-    }
+    LibcTop libcTop = params.libcTop;
     if ((libcTop != null) && (libcTop.getLabel() != null)) {
       libcLabel = libcTop.getLabel();
     } else {
@@ -1780,10 +1854,6 @@ public boolean getParseHeadersVerifiesModules() {
     return cppOptions.parseHeadersVerifiesModules;
   }
 
-  public LibcTop getLibcTop() {
-    return cppOptions.libcTop;
-  }
-
   public boolean getUseInterfaceSharedObjects() {
     return cppOptions.useInterfaceSharedObjects;
   }

src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java

@@ -36,6 +36,7 @@
 import com.google.devtools.build.lib.packages.NonconfigurableAttributeMapper;
 import com.google.devtools.build.lib.packages.Rule;
 import com.google.devtools.build.lib.packages.Target;
+import com.google.devtools.build.lib.rules.cpp.CppConfiguration.LibcTop;
 import com.google.devtools.build.lib.vfs.FileSystemUtils;
 import com.google.devtools.build.lib.vfs.Path;
 import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig;
@@ -95,14 +96,17 @@ public static class CppConfigurationParameters {
     protected final Label ccToolchainLabel;
     protected final Label stlLabel;
     protected final Path fdoZip;
+    protected final LibcTop libcTop;
 
-    CppConfigurationParameters(CrosstoolConfig.CToolchain toolchain,
+    CppConfigurationParameters(
+        CrosstoolConfig.CToolchain toolchain,
         String cacheKeySuffix,
         BuildOptions buildOptions,
         Path fdoZip,
         Label crosstoolTop,
         Label ccToolchainLabel,
-        Label stlLabel) {
+        Label stlLabel,
+        LibcTop libcTop) {
       this.toolchain = toolchain;
       this.cacheKeySuffix = cacheKeySuffix;
       this.commonOptions = buildOptions.get(BuildConfiguration.Options.class);
@@ -111,6 +115,7 @@ public static class CppConfigurationParameters {
       this.crosstoolTop = crosstoolTop;
       this.ccToolchainLabel = ccToolchainLabel;
       this.stlLabel = stlLabel;
+      this.libcTop = libcTop;
     }
   }
 
@@ -221,7 +226,19 @@ protected CppConfigurationParameters createParameters(
           "The label '%s' is not a cc_toolchain rule", ccToolchainLabel));
     }
 
-    return new CppConfigurationParameters(toolchain, file.getMd5(), options,
-        fdoZip, crosstoolTopLabel, ccToolchainLabel, stlLabel);
+    LibcTop.Result libcTopResult = LibcTop.createLibcTop(cppOptions, env, toolchain);
+    if (libcTopResult.valuesMissing()) {
+      return null;
+    }
+
+    return new CppConfigurationParameters(
+        toolchain,
+        file.getMd5(),
+        options,
+        fdoZip,
+        crosstoolTopLabel,
+        ccToolchainLabel,
+        stlLabel,
+        libcTopResult.getLibcTop());
   }
 }

src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java

@@ -26,7 +26,6 @@
 import com.google.devtools.build.lib.cmdline.Label;
 import com.google.devtools.build.lib.cmdline.LabelSyntaxException;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.DynamicMode;
-import com.google.devtools.build.lib.rules.cpp.CppConfiguration.LibcTop;
 import com.google.devtools.build.lib.rules.cpp.CppConfiguration.StripMode;
 import com.google.devtools.build.lib.util.OptionsUtils;
 import com.google.devtools.build.lib.vfs.PathFragment;
@@ -90,12 +89,11 @@ public StripModeConverter() {
   private static final String LIBC_RELATIVE_LABEL = ":everything";
 
   /**
-   * Converts a String, which is an absolute path or label into a LibcTop
-   * object.
+   * Converts a String, which is a package label into a label that can be used for a LibcTop object.
    */
-  public static class LibcTopConverter implements Converter<LibcTop> {
+  public static class LibcTopLabelConverter implements Converter<Label> {
     @Override
-    public LibcTop convert(String input) throws OptionsParsingException {
+    public Label convert(String input) throws OptionsParsingException {
       if (input.equals("default")) {
         // This is needed for defining config_setting() values, the syntactic form
         // of which must be a String, to match absence of a --grte_top option.
@@ -107,8 +105,7 @@ public LibcTop convert(String input) throws OptionsParsingException {
         throw new OptionsParsingException("Not a label");
       }
       try {
-        Label label = Label.parseAbsolute(input).getRelative(LIBC_RELATIVE_LABEL);
-        return new LibcTop(label);
+        return Label.parseAbsolute(input).getRelative(LIBC_RELATIVE_LABEL);
       } catch (LabelSyntaxException e) {
         throw new OptionsParsingException(e.getMessage());
       }
@@ -508,23 +505,23 @@ public LipoModeConverter() {
     name = "grte_top",
     defaultValue = "null", // The default value is chosen by the toolchain.
     category = "version",
-    converter = LibcTopConverter.class,
+    converter = LibcTopLabelConverter.class,
     help =
         "A label to a checked-in libc library. The default value is selected by the crosstool "
             + "toolchain, and you almost never need to override it."
   )
-  public LibcTop libcTop;
+  public Label libcTopLabel;
 
   @Option(
     name = "host_grte_top",
     defaultValue = "null", // The default value is chosen by the toolchain.
     category = "version",
-    converter = LibcTopConverter.class,
+    converter = LibcTopLabelConverter.class,
     help =
         "If specified, this setting overrides the libc top-level directory (--grte_top) "
             + "for the host configuration."
   )
-  public LibcTop hostLibcTop;
+  public Label hostLibcTopLabel;
 
   @Option(
     name = "output_symbol_counts",
@@ -629,7 +626,7 @@ public FragmentOptions getHost(boolean fallback) {
     // Only an explicit command-line option will change it.
     // The default is whatever the host's crosstool (which might have been specified
     // by --host_crosstool_top, or --crosstool_top as a fallback) says it should be.
-    host.libcTop = hostLibcTop;
+    host.libcTopLabel = hostLibcTopLabel;
 
     // -g0 is the default, but allowMultiple options cannot have default values so we just pass
     // -g0 first and let the user options override it.
@@ -652,14 +649,14 @@ public void addAllLabels(Multimap<String, Label> labelMap) {
       labelMap.put("crosstool", hostCrosstoolTop);
     }
 
-    if (libcTop != null) {
-      Label libcLabel = libcTop.getLabel();
+    if (libcTopLabel != null) {
+      Label libcLabel = libcTopLabel;
       if (libcLabel != null) {
         labelMap.put("crosstool", libcLabel);
       }
     }
-    if (hostLibcTop != null) {
-      Label libcLabel = hostLibcTop.getLabel();
+    if (hostLibcTopLabel != null) {
+      Label libcLabel = hostLibcTopLabel;
       if (libcLabel != null) {
         labelMap.put("crosstool", libcLabel);
       }
@@ -687,8 +684,8 @@ public Map<String, Set<Label>> getDefaultsLabels(BuildConfiguration.Options comm
       crosstoolLabels.add(hostCrosstoolTop);
     }
 
-    if (libcTop != null) {
-      Label libcLabel = libcTop.getLabel();
+    if (libcTopLabel != null) {
+      Label libcLabel = libcTopLabel;
       if (libcLabel != null) {
         crosstoolLabels.add(libcLabel);
       }

src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java

@@ -78,7 +78,7 @@ public static void setAppleCrosstoolTransitionConfiguration(BuildOptions from,
 
     // OSX toolchains always use the runtime of the platform they are targeting (i.e. we do not
     // support custom production environments).
-    to.get(CppOptions.class).libcTop = null;
+    to.get(CppOptions.class).libcTopLabel = null;
     to.get(CppOptions.class).glibc = null;
 
     // OSX toolchains do not support fission.

src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java

@@ -342,7 +342,7 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
                 + "  builtin_sysroot: \"builtin-sysroot-A\"\n"
                 + "  default_python_top: \"python-top-A\"\n"
                 + "  default_python_version: \"python-version-A\"\n"
-                + "  default_grte_top: \"//some:labelA\""
+                + "  default_grte_top: \"//some\""
                 + "  debian_extra_requires: \"a\""
                 + "  debian_extra_requires: \"b\""
                 + "}\n"
@@ -446,7 +446,7 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
                 + "  builtin_sysroot: \"builtin-sysroot-B\"\n"
                 + "  default_python_top: \"python-top-B\"\n"
                 + "  default_python_version: \"python-version-B\"\n"
-                + "  default_grte_top: \"//some:labelB\"\n"
+                + "  default_grte_top: \"//some\"\n"
                 + "  debian_extra_requires: \"c\""
                 + "  debian_extra_requires: \"d\""
                 + "}\n"
@@ -476,6 +476,12 @@ public void testComprehensiveCompleteConfiguration() throws Exception {
                 + "  tool_path { name: \"dwp\" path: \"path/to/dwp\" }\n"
                 + "}");
 
+    mockToolsConfig.create(
+        "some/BUILD",
+        "package(default_visibility=['//visibility:public'])",
+        "licenses(['unencumbered'])",
+        "filegroup(name = 'everything')");
+
     CppConfiguration toolchainA = create(loader, "--cpu=piii");
     assertEquals("toolchain-identifier-A", toolchainA.getToolchainIdentifier());
     assertEquals("host-system-name-A", toolchainA.getHostSystemName());