RELNOTES[NEW]: If grte_top is a label, it can now follow non-configurable redirects.
--
PiperOrigin-RevId: 150647389
MOS_MIGRATED_REVID=150647389
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
index 868834d..0bcfb7f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfiguration.java
+++ b/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 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 @@
+ "' 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 @@
return cppOptions.parseHeadersVerifiesModules;
}
- public LibcTop getLibcTop() {
- return cppOptions.libcTop;
- }
-
public boolean getUseInterfaceSharedObjects() {
return cppOptions.useInterfaceSharedObjects;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java
index ca6c26f..c6e4a81 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppConfigurationLoader.java
+++ b/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 @@
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 @@
this.crosstoolTop = crosstoolTop;
this.ccToolchainLabel = ccToolchainLabel;
this.stlLabel = stlLabel;
+ this.libcTop = libcTop;
}
}
@@ -221,7 +226,19 @@
"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());
}
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
index 5ef1dd0..19af5fd 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppOptions.java
+++ b/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 @@
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 @@
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 @@
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 @@
// 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 @@
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 @@
crosstoolLabels.add(hostCrosstoolTop);
}
- if (libcTop != null) {
- Label libcLabel = libcTop.getLabel();
+ if (libcTopLabel != null) {
+ Label libcLabel = libcTopLabel;
if (libcLabel != null) {
crosstoolLabels.add(libcLabel);
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
index 2c3ed33..024956f 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleCrosstoolTransition.java
@@ -78,7 +78,7 @@
// 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.
diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
index fce9586..1fd7a01 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigurationLoaderTest.java
@@ -342,7 +342,7 @@
+ " 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 @@
+ " 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 @@
+ " 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());
