diff options
5 files changed, 132 insertions, 42 deletions
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 868834db81..0bcfb7fb3f 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.ImmutableSet; 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.HashSet; 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 class CppConfiguration extends BuildConfiguration.Fragment { 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 class CppConfiguration extends BuildConfiguration.Fragment { + "' 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 class CppConfiguration extends BuildConfiguration.Fragment { 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 ca6c26ff26..c6e4a81586 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.NoSuchThingException; 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 class CppConfigurationLoader implements ConfigurationFragmentFactory { 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 class CppConfigurationLoader implements ConfigurationFragmentFactory { this.crosstoolTop = crosstoolTop; this.ccToolchainLabel = ccToolchainLabel; this.stlLabel = stlLabel; + this.libcTop = libcTop; } } @@ -221,7 +226,19 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { "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 5ef1dd0ac2..19af5fd0b8 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.analysis.config.PerLabelOptions; 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 class CppOptions extends FragmentOptions { 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 class CppOptions extends FragmentOptions { 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 class CppOptions extends FragmentOptions { 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 class CppOptions extends FragmentOptions { // 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 class CppOptions extends FragmentOptions { 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 class CppOptions extends FragmentOptions { 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 2c3ed33245..024956fc07 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 @@ public class AppleCrosstoolTransition implements PatchTransition { // 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 fce9586918..1fd7a011ad 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 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { + " 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 class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { + " 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 class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { + " 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()); |