From 6879d7ceff0e118fdecb0cabe5134979030b7cb8 Mon Sep 17 00:00:00 2001 From: Googler Date: Fri, 7 Apr 2017 16:57:21 -0400 Subject: Automated g4 rollback of commit cbbb423663b154d82e3dfa5e9a56839583987999. *** Reason for rollback *** Need to roll this back as part of http://b/38171368 *** Original change description *** RELNOTES: Effectively remove sysroot from CppConfiguration and allow it to use select statements. PiperOrigin-RevId: 155547813 --- .../devtools/build/lib/rules/cpp/CcBinary.java | 3 +- .../devtools/build/lib/rules/cpp/CcToolchain.java | 27 +- .../build/lib/rules/cpp/CcToolchainProvider.java | 14 +- .../build/lib/rules/cpp/CcToolchainRule.java | 6 +- .../build/lib/rules/cpp/CompileCommandLine.java | 7 +- .../build/lib/rules/cpp/CppCompileAction.java | 26 +- .../lib/rules/cpp/CppCompileActionBuilder.java | 6 +- .../build/lib/rules/cpp/CppConfiguration.java | 297 ++++++++++++++------- .../lib/rules/cpp/CppConfigurationLoader.java | 44 +-- .../devtools/build/lib/rules/cpp/CppHelper.java | 2 +- .../build/lib/rules/cpp/CppLinkActionBuilder.java | 4 +- .../build/lib/rules/cpp/FakeCppCompileAction.java | 6 +- .../build/lib/rules/cpp/LinkCommandLine.java | 38 ++- 13 files changed, 251 insertions(+), 229 deletions(-) (limited to 'src/main/java/com') diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java index 7e15d9e462..ca98d7bf61 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcBinary.java @@ -523,7 +523,8 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { private static final boolean dashStaticInLinkopts(List linkopts, CppConfiguration cppConfiguration) { - return linkopts.contains("-static") || cppConfiguration.hasStaticLinkOption(); + return linkopts.contains("-static") + || cppConfiguration.getLinkOptions().contains("-static"); } private static final LinkStaticness getLinkStaticness(RuleContext context, diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java index da27ada49e..6fc4c92938 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchain.java @@ -36,7 +36,6 @@ import com.google.devtools.build.lib.analysis.TransitiveInfoCollection; import com.google.devtools.build.lib.analysis.actions.SpawnAction; import com.google.devtools.build.lib.analysis.actions.SymlinkAction; import com.google.devtools.build.lib.analysis.config.CompilationMode; -import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; @@ -285,15 +284,6 @@ public class CcToolchain implements RuleConfiguredTargetFactory { coverage = crosstool; } - PathFragment sysroot = calculateSysroot(ruleContext); - - ImmutableList builtInIncludeDirectories = null; - try { - builtInIncludeDirectories = cppConfiguration.getBuiltInIncludeDirectories(sysroot); - } catch (InvalidConfigurationException e) { - ruleContext.ruleError(e.getMessage()); - } - CcToolchainProvider ccProvider = new CcToolchainProvider( cppConfiguration, @@ -320,8 +310,9 @@ public class CcToolchain implements RuleConfiguredTargetFactory { coverageEnvironment.build(), ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST), getEnvironment(ruleContext), - builtInIncludeDirectories, - sysroot); + cppConfiguration.getBuiltInIncludeDirectories()); + + PathFragment sysroot = cppConfiguration.getSysroot(); MakeVariableProvider makeVariableProvider = createMakeVariableProvider(cppConfiguration, sysroot); @@ -490,16 +481,4 @@ public class CcToolchain implements RuleConfiguredTargetFactory { protected ImmutableMap getEnvironment(RuleContext ruleContext) { return ImmutableMap.of(); } - - private PathFragment calculateSysroot(RuleContext ruleContext) { - - TransitiveInfoCollection sysrootTarget = ruleContext.getPrerequisite(":sysroot", Mode.TARGET); - if (sysrootTarget == null) { - CppConfiguration cppConfiguration = - Preconditions.checkNotNull(ruleContext.getFragment(CppConfiguration.class)); - return cppConfiguration.getDefaultSysroot(); - } - - return sysrootTarget.getLabel().getPackageFragment(); - } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java index 89beafe21d..26d4c38b67 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainProvider.java @@ -75,8 +75,7 @@ public final class CcToolchainProvider NestedSetBuilder.>emptySet(Order.COMPILE_ORDER), null, ImmutableMap.of(), - ImmutableList.of(), - null); + ImmutableList.of()); @Nullable private final CppConfiguration cppConfiguration; private final NestedSet crosstool; @@ -103,7 +102,6 @@ public final class CcToolchainProvider @Nullable private final Artifact linkDynamicLibraryTool; private final ImmutableMap environment; private final ImmutableList builtInIncludeDirectories; - @Nullable private final PathFragment sysroot; public CcToolchainProvider( @Nullable CppConfiguration cppConfiguration, @@ -130,8 +128,7 @@ public final class CcToolchainProvider NestedSet> coverageEnvironment, Artifact linkDynamicLibraryTool, ImmutableMap environment, - ImmutableList builtInIncludeDirectories, - @Nullable PathFragment sysroot) { + ImmutableList builtInIncludeDirectories) { super(SKYLARK_CONSTRUCTOR, ImmutableMap.of()); this.cppConfiguration = cppConfiguration; this.crosstool = Preconditions.checkNotNull(crosstool); @@ -158,7 +155,6 @@ public final class CcToolchainProvider this.linkDynamicLibraryTool = linkDynamicLibraryTool; this.environment = environment; this.builtInIncludeDirectories = builtInIncludeDirectories; - this.sysroot = sysroot; } @SkylarkCallable( @@ -356,7 +352,7 @@ public final class CcToolchainProvider + "this method returns None." ) public PathFragment getSysroot() { - return sysroot; + return cppConfiguration.getSysroot(); } @SkylarkCallable( @@ -366,7 +362,7 @@ public final class CcToolchainProvider + "rules. These should be appended to the command line after filtering." ) public ImmutableList getUnfilteredCompilerOptions(Iterable features) { - return cppConfiguration.getUnfilteredCompilerOptions(features, sysroot); + return cppConfiguration.getUnfilteredCompilerOptions(features); } @SkylarkCallable( @@ -377,6 +373,6 @@ public final class CcToolchainProvider + "inferred from the command-line options." ) public ImmutableList getLinkOptions() { - return cppConfiguration.getLinkOptions(sysroot); + return cppConfiguration.getLinkOptions(); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java index 8b7dbe92ff..a1922b8553 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainRule.java @@ -51,8 +51,9 @@ public final class CcToolchainRule implements RuleDefinition { private static final LateBoundLabel LIBC_TOP = new LateBoundLabel(CppConfiguration.class) { @Override - public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(CppConfiguration.class).getSysrootLabel(); + public Label resolve( + Rule rule, AttributeMap attributes, BuildConfiguration configuration) { + return configuration.getFragment(CppConfiguration.class).getLibcLabel(); } }; @@ -87,7 +88,6 @@ public final class CcToolchainRule implements RuleDefinition { .value(env.getToolsLabel("//tools/cpp:link_dynamic_library"))) // TODO(bazel-team): Should be using the TARGET configuration. .add(attr(":libc_top", LABEL).cfg(HOST).value(LIBC_TOP)) - .add(attr(":sysroot", LABEL).value(LIBC_TOP)) .add( attr(":lipo_context_collector", LABEL) .cfg(LipoTransition.LIPO_COLLECTOR) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java index 4310cef570..f9fc4c3f6b 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileCommandLine.java @@ -45,7 +45,6 @@ public final class CompileCommandLine { @VisibleForTesting public final CcToolchainFeatures.Variables variables; private final String actionName; private final CppConfiguration cppConfiguration; - private final ImmutableList unfilteredCompilerOptions; public CompileCommandLine( Artifact sourceFile, @@ -58,8 +57,7 @@ public final class CompileCommandLine { CppConfiguration cppConfiguration, CcToolchainFeatures.Variables variables, String actionName, - DotdFile dotdFile, - ImmutableList unfilteredCompilerOptions) { + DotdFile dotdFile) { this.sourceFile = Preconditions.checkNotNull(sourceFile); this.outputFile = Preconditions.checkNotNull(outputFile); this.sourceLabel = Preconditions.checkNotNull(sourceLabel); @@ -71,7 +69,6 @@ public final class CompileCommandLine { this.variables = variables; this.actionName = actionName; this.dotdFile = isGenerateDotdFile(sourceFile) ? Preconditions.checkNotNull(dotdFile) : null; - this.unfilteredCompilerOptions = unfilteredCompilerOptions; } /** Returns true if Dotd file should be generated. */ @@ -164,7 +161,7 @@ public final class CompileCommandLine { // the user provided options, otherwise users adding include paths will not pick up their // own include paths first. if (!isObjcCompile(actionName)) { - options.addAll(unfilteredCompilerOptions); + options.addAll(toolchain.getUnfilteredCompilerOptions(features)); } // Add the options of --per_file_copt, if the label or the base name of the source file diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java index 20fe8e030d..1b1884ce8d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileAction.java @@ -217,8 +217,6 @@ public class CppCompileAction extends AbstractAction /** Whether this action needs to discover inputs. */ private final boolean discoversInputs; - private final ImmutableList builtInIncludeDirectories; - /** * Set when the action prepares for execution. Used to preserve state between preparation and * execution. @@ -273,8 +271,6 @@ public class CppCompileAction extends AbstractAction * @param actionName a string giving the name of this action for the purpose of toolchain * evaluation * @param cppSemantics C++ compilation semantics - * @param unfilteredCompilerOptions - Compile options that will always be added. - * @param builtInIncludeDirectories - Directories to search for system includes from. */ protected CppCompileAction( ActionOwner owner, @@ -312,9 +308,7 @@ public class CppCompileAction extends AbstractAction ImmutableMap environment, String actionName, Iterable builtinIncludeFiles, - CppSemantics cppSemantics, - ImmutableList unfilteredCompilerOptions, - ImmutableList builtInIncludeDirectories) { + CppSemantics cppSemantics) { super( owner, allInputs, @@ -355,8 +349,7 @@ public class CppCompileAction extends AbstractAction cppConfiguration, variables, actionName, - dotdFile, - unfilteredCompilerOptions); + dotdFile); this.actionContext = actionContext; this.lipoScannables = lipoScannables; this.actionClassId = actionClassId; @@ -370,7 +363,6 @@ public class CppCompileAction extends AbstractAction this.builtinIncludeFiles = ImmutableList.copyOf(builtinIncludeFiles); this.cppSemantics = cppSemantics; this.additionalIncludeScannables = ImmutableList.copyOf(additionalIncludeScannables); - this.builtInIncludeDirectories = builtInIncludeDirectories; } /** @@ -389,7 +381,7 @@ public class CppCompileAction extends AbstractAction @Override public List getBuiltInIncludeDirectories() { - return builtInIncludeDirectories; + return cppConfiguration.getBuiltInIncludeDirectories(); } @Nullable @@ -843,10 +835,9 @@ public class CppCompileAction extends AbstractAction if (optionalSourceFile != null) { allowedIncludes.add(optionalSourceFile); } - Iterable ignoreDirs = - cppConfiguration.isStrictSystemIncludes() - ? getBuiltInIncludeDirectories() - : getValidationIgnoredDirs(); + Iterable ignoreDirs = cppConfiguration.isStrictSystemIncludes() + ? cppConfiguration.getBuiltInIncludeDirectories() + : getValidationIgnoredDirs(); // Copy the sets to hash sets for fast contains checking. // Avoid immutable sets here to limit memory churn. @@ -915,7 +906,7 @@ public class CppCompileAction extends AbstractAction } Iterable getValidationIgnoredDirs() { - List cxxSystemIncludeDirs = getBuiltInIncludeDirectories(); + List cxxSystemIncludeDirs = cppConfiguration.getBuiltInIncludeDirectories(); return Iterables.concat( cxxSystemIncludeDirs, context.getSystemIncludeDirs()); } @@ -1268,8 +1259,9 @@ public class CppCompileAction extends AbstractAction } public List getPermittedSystemIncludePrefixes(Path execRoot) { + CppConfiguration toolchain = cppConfiguration; List systemIncludePrefixes = new ArrayList<>(); - for (PathFragment includePath : getBuiltInIncludeDirectories()) { + for (PathFragment includePath : toolchain.getBuiltInIncludeDirectories()) { if (includePath.isAbsolute()) { systemIncludePrefixes.add(execRoot.getFileSystem().getPath(includePath)); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java index e4bf2b3b4c..333361f193 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppCompileActionBuilder.java @@ -386,8 +386,6 @@ public class CppCompileActionBuilder { getLipoScannables(realMandatoryInputs), ccToolchain.getBuiltinIncludeFiles(), cppSemantics, - ccToolchain.getUnfilteredCompilerOptions(features), - ccToolchain.getBuiltInIncludeDirectories(), ImmutableMap.copyOf(executionInfo)); } else { return new CppCompileAction( @@ -424,9 +422,7 @@ public class CppCompileActionBuilder { ImmutableMap.copyOf(environment), getActionName(), ccToolchain.getBuiltinIncludeFiles(), - cppSemantics, - ccToolchain.getUnfilteredCompilerOptions(features), - ccToolchain.getBuiltInIncludeDirectories()); + cppSemantics); } } 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 765f4d6f6d..4388571311 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 @@ -25,10 +25,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.PatchTransition; import com.google.devtools.build.lib.analysis.config.PerLabelOptions; @@ -58,6 +60,7 @@ import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CTool import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LinkingModeFlags; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.ToolPath; +import com.google.devtools.common.options.OptionsParsingException; import com.google.protobuf.TextFormat; import com.google.protobuf.TextFormat.ParseException; import java.io.Serializable; @@ -159,6 +162,127 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } } + /** Storage for the libc label, if given. */ + public static class LibcTop implements Serializable { + private final 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 + : PathFragment.create(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; + } + + public Label getLabel() { + return label; + } + + public PathFragment getSysroot() { + return label.getPackageFragment(); + } + + @Override + public String toString() { + return label.toString(); + } + + @Override + public boolean equals(Object other) { + if (this == other) { + return true; + } else if (other instanceof LibcTop) { + return label.equals(((LibcTop) other).label); + } else { + return false; + } + } + + @Override + public int hashCode() { + return label.hashCode(); + } + } + /** * This macro will be passed as a command-line parameter (eg. -DBUILD_FDO_TYPE="LIPO"). * For possible values see {@code CppModel.getFdoBuildStamp()}. @@ -257,17 +381,15 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // TODO(bazel-team): All these labels (except for ccCompilerRuleLabel) can be removed once the // transition to the cc_compiler rule is complete. + private final Label libcLabel; private final Label staticRuntimeLibsLabel; private final Label dynamicRuntimeLibsLabel; private final Label ccToolchainLabel; private final Label stlLabel; - // TODO(kmensah): This is temporary until all the Skylark functions that need this can be removed. - private final PathFragment nonConfiguredSysroot; - private final Label sysrootLabel; - private final PathFragment defaultSysroot; + private final PathFragment sysroot; private final PathFragment runtimeSysroot; - private final ImmutableList rawBuiltInIncludeDirectories; + private final ImmutableList builtInIncludeDirectories; private final Map toolPaths; private final PathFragment ldExecutable; @@ -492,14 +614,29 @@ public class CppConfiguration extends BuildConfiguration.Fragment { this.abiGlibcVersion = toolchain.getAbiLibcVersion(); // The default value for optional string attributes is the empty string. - this.defaultSysroot = computeDefaultSysroot(toolchain); + PathFragment defaultSysroot = toolchain.getBuiltinSysroot().length() == 0 + ? null + : PathFragment.create(toolchain.getBuiltinSysroot()); + if ((defaultSysroot != null) && !defaultSysroot.isNormalized()) { + throw new IllegalArgumentException("The built-in sysroot '" + defaultSysroot + + "' is not normalized."); + } - this.sysrootLabel = params.sysrootLabel; - this.nonConfiguredSysroot = - params.sysrootLabel == null ? defaultSysroot : params.sysrootLabel.getPackageFragment(); + LibcTop libcTop = params.libcTop; + if ((libcTop != null) && (libcTop.getLabel() != null)) { + libcLabel = libcTop.getLabel(); + } else { + libcLabel = null; + } - rawBuiltInIncludeDirectories = - ImmutableList.copyOf(toolchain.getCxxBuiltinIncludeDirectoryList()); + ImmutableList.Builder builtInIncludeDirectoriesBuilder + = ImmutableList.builder(); + sysroot = libcTop == null ? defaultSysroot : libcTop.getSysroot(); + for (String s : toolchain.getCxxBuiltinIncludeDirectoryList()) { + builtInIncludeDirectoriesBuilder.add( + resolveIncludeDir(s, sysroot, crosstoolTopPathFragment)); + } + builtInIncludeDirectories = builtInIncludeDirectoriesBuilder.build(); // The runtime sysroot should really be set from --grte_top. However, currently libc has no // way to set the sysroot. The CROSSTOOL file does set the runtime sysroot, in the @@ -507,8 +644,17 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // and libc versions, you must always choose compatible ones. runtimeSysroot = defaultSysroot; - ImmutableList.Builder unfilteredCoptsBuilder = ImmutableList.builder(); + String sysrootFlag; + if (sysroot != null) { + sysrootFlag = "--sysroot=" + sysroot; + } else { + sysrootFlag = null; + } + ImmutableList.Builder unfilteredCoptsBuilder = ImmutableList.builder(); + if (sysrootFlag != null) { + unfilteredCoptsBuilder.add(sysrootFlag); + } unfilteredCoptsBuilder.addAll(toolchain.getUnfilteredCxxFlagList()); unfilteredCompilerFlags = new FlagList( unfilteredCoptsBuilder.build(), @@ -520,6 +666,9 @@ public class CppConfiguration extends BuildConfiguration.Fragment { if (cppOptions.experimentalOmitfp) { linkoptsBuilder.add("-Wl,--eh-frame-hdr"); } + if (sysrootFlag != null) { + linkoptsBuilder.add(sysrootFlag); + } this.linkOptions = linkoptsBuilder.build(); ImmutableList.Builder ltoindexoptsBuilder = ImmutableList.builder(); @@ -581,6 +730,12 @@ public class CppConfiguration extends BuildConfiguration.Fragment { for (CrosstoolConfig.MakeVariable variable : toolchain.getMakeVariableList()) { makeVariablesBuilder.put(variable.getName(), variable.getValue()); } + // TODO(kmensah): Remove once targets can depend on the cc_toolchain in skylark. + if (sysrootFlag != null) { + String ccFlags = makeVariablesBuilder.get("CC_FLAGS"); + ccFlags = ccFlags.isEmpty() ? sysrootFlag : ccFlags + " " + sysrootFlag; + makeVariablesBuilder.put("CC_FLAGS", ccFlags); + } this.additionalMakeVariables = ImmutableMap.copyOf(makeVariablesBuilder); } @@ -1122,6 +1277,14 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return toolPaths.get(tool.getNamePart()); } + /** + * Returns a label that forms a dependency to the files required for the + * sysroot that is used. + */ + public Label getLibcLabel() { + return libcLabel; + } + /** * Returns a label that references the library files needed to statically * link the C++ runtime (i.e. libgcc.a, libgcc_eh.a, libstdc++.a) for the @@ -1270,30 +1433,17 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } /** - * Returns the built-in list of system include paths for the toolchain compiler. All paths in this - * list should be relative to the exec directory. They may be absolute if they are also installed - * on the remote build nodes or for local compilation. + * Returns the built-in list of system include paths for the toolchain + * compiler. All paths in this list should be relative to the exec directory. + * They may be absolute if they are also installed on the remote build nodes or + * for local compilation. */ - @SkylarkCallable( - name = "built_in_include_directories", - structField = true, - doc = - "Built-in system include paths for the toolchain compiler. All paths in this list" - + " should be relative to the exec directory. They may be absolute if they are also" - + " installed on the remote build nodes or for local compilation." - ) - public ImmutableList getBuiltInIncludeDirectories() - throws InvalidConfigurationException { - return getBuiltInIncludeDirectories(nonConfiguredSysroot); - } - - public ImmutableList getBuiltInIncludeDirectories(PathFragment sysroot) - throws InvalidConfigurationException { - ImmutableList.Builder builtInIncludeDirectoriesBuilder = ImmutableList.builder(); - for (String s : rawBuiltInIncludeDirectories) { - builtInIncludeDirectoriesBuilder.add(resolveIncludeDir(s, sysroot, crosstoolTopPathFragment)); - } - return builtInIncludeDirectoriesBuilder.build(); + @SkylarkCallable(name = "built_in_include_directories", structField = true, + doc = "Built-in system include paths for the toolchain compiler. All paths in this list" + + " should be relative to the exec directory. They may be absolute if they are also installed" + + " on the remote build nodes or for local compilation.") + public ImmutableList getBuiltInIncludeDirectories() { + return builtInIncludeDirectories; } /** @@ -1306,11 +1456,7 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + "different sysroots, or the sysroot is the same as the default sysroot, then " + "this method returns None.") public PathFragment getSysroot() { - return nonConfiguredSysroot; - } - - public Label getSysrootLabel() { - return sysrootLabel; + return sysroot; } /** @@ -1370,8 +1516,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } /** - * Returns the default list of options which cannot be filtered by BUILD rules. These should be - * appended to the command line after filtering. + * Returns the default list of options which cannot be filtered by BUILD + * rules. These should be appended to the command line after filtering. */ @SkylarkCallable( name = "unfiltered_compiler_options", @@ -1380,56 +1526,21 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + "rules. These should be appended to the command line after filtering." ) public ImmutableList getUnfilteredCompilerOptions(Iterable features) { - return getUnfilteredCompilerOptions(features, nonConfiguredSysroot); - } - - public ImmutableList getUnfilteredCompilerOptions( - Iterable features, PathFragment sysroot) { - if (sysroot == null) { - return unfilteredCompilerFlags.evaluate(features); - } else { - return ImmutableList.builder() - .add("--sysroot=" + sysroot) - .addAll(unfilteredCompilerFlags.evaluate(features)) - .build(); - } + return unfilteredCompilerFlags.evaluate(features); } /** - * Returns the set of command-line linker options, including any flags inferred from the - * command-line options. + * Returns the set of command-line linker options, including any flags + * inferred from the command-line options. * * @see Link */ // TODO(bazel-team): Clean up the linker options computation! - @SkylarkCallable( - name = "link_options", - structField = true, - doc = - "Returns the set of command-line linker options, including any flags " - + "inferred from the command-line options." - ) + @SkylarkCallable(name = "link_options", structField = true, + doc = "Returns the set of command-line linker options, including any flags " + + "inferred from the command-line options.") public ImmutableList getLinkOptions() { - return getLinkOptions(nonConfiguredSysroot); - } - - public ImmutableList getLinkOptions(PathFragment sysroot) { - if (sysroot == null) { - return linkOptions; - } else { - return ImmutableList.builder() - .addAll(linkOptions) - .add("--sysroot=" + sysroot) - .build(); - } - } - - public boolean hasStaticLinkOption() { - return linkOptions.contains("-static"); - } - - public boolean hasSharedLinkOption() { - return linkOptions.contains("-shared"); + return linkOptions; } /** Returns the set of command-line LTO indexing options. */ @@ -2059,8 +2170,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // TODO(bazel-team): delete all of these. globalMakeEnvBuilder.put("CROSSTOOLTOP", crosstoolTopPathFragment.getPathString()); - // TODO(kmensah): Remove when skylark dependencies can be updated to rely on - // CcToolchainProvider. globalMakeEnvBuilder.putAll(getAdditionalMakeVariables()); globalMakeEnvBuilder.put("ABI_GLIBC_VERSION", getAbiGlibcVersion()); @@ -2143,22 +2252,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { return requestedFeatures.build(); } - public static PathFragment computeDefaultSysroot(CToolchain toolchain) { - PathFragment defaultSysroot = - toolchain.getBuiltinSysroot().length() == 0 - ? null - : PathFragment.create(toolchain.getBuiltinSysroot()); - if ((defaultSysroot != null) && !defaultSysroot.isNormalized()) { - throw new IllegalArgumentException( - "The built-in sysroot '" + defaultSysroot + "' is not normalized."); - } - return defaultSysroot; - } - - public PathFragment getDefaultSysroot() { - return defaultSysroot; - } - @Override public PatchTransition getArtifactOwnerTransition() { return isLipoContextCollector() ? ContextCollectorOwnerTransition.INSTANCE : null; 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 b334b10923..9a98181724 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,11 +36,10 @@ 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.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig; -import com.google.devtools.common.options.OptionsParsingException; import javax.annotation.Nullable; /** @@ -90,7 +89,7 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { protected final Label ccToolchainLabel; protected final Label stlLabel; protected final Path fdoZip; - protected final Label sysrootLabel; + protected final LibcTop libcTop; CppConfigurationParameters( CrosstoolConfig.CToolchain toolchain, @@ -100,7 +99,7 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { Label crosstoolTop, Label ccToolchainLabel, Label stlLabel, - Label sysrootLabel) { + LibcTop libcTop) { this.toolchain = toolchain; this.cacheKeySuffix = cacheKeySuffix; this.commonOptions = buildOptions.get(BuildConfiguration.Options.class); @@ -109,7 +108,7 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { this.crosstoolTop = crosstoolTop; this.ccToolchainLabel = ccToolchainLabel; this.stlLabel = stlLabel; - this.sysrootLabel = sysrootLabel; + this.libcTop = libcTop; } } @@ -220,7 +219,10 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { "The label '%s' is not a cc_toolchain rule", ccToolchainLabel)); } - Label sysrootLabel = getSysrootLabel(toolchain, cppOptions.libcTopLabel); + LibcTop.Result libcTopResult = LibcTop.createLibcTop(cppOptions, env, toolchain); + if (libcTopResult.valuesMissing()) { + return null; + } return new CppConfigurationParameters( toolchain, @@ -230,34 +232,6 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { crosstoolTopLabel, ccToolchainLabel, stlLabel, - sysrootLabel); - } - - @Nullable - public static Label getSysrootLabel(CrosstoolConfig.CToolchain toolchain, Label libcTopLabel) - throws InvalidConfigurationException { - PathFragment defaultSysroot = CppConfiguration.computeDefaultSysroot(toolchain); - - if ((libcTopLabel != null) && (defaultSysroot == null)) { - throw new InvalidConfigurationException( - "The selected toolchain " - + toolchain.getToolchainIdentifier() - + " does not support setting --grte_top."); - } - - if (libcTopLabel != null) { - return libcTopLabel; - } - - if (!toolchain.getDefaultGrteTop().isEmpty()) { - try { - Label grteTopLabel = - new CppOptions.LibcTopLabelConverter().convert(toolchain.getDefaultGrteTop()); - return grteTopLabel; - } catch (OptionsParsingException e) { - throw new InvalidConfigurationException(e.getMessage(), e); - } - } - return null; + libcTopResult.getLibcTop()); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java index ea3fd42ddd..aa719c80ed 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppHelper.java @@ -592,7 +592,7 @@ public class CppHelper { public static void maybeAddStaticLinkMarkerProvider(RuleConfiguredTargetBuilder builder, RuleContext ruleContext) { boolean staticallyLinked = false; - if (ruleContext.getFragment(CppConfiguration.class).hasStaticLinkOption()) { + if (ruleContext.getFragment(CppConfiguration.class).getLinkOptions().contains("-static")) { staticallyLinked = true; } else if (ruleContext.attributes().has("linkopts", Type.STRING_LIST) && ruleContext.attributes().get("linkopts", Type.STRING_LIST).contains("-static")) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java index 18d7102092..0c640ff818 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkActionBuilder.java @@ -813,7 +813,7 @@ public class CppLinkActionBuilder { } else if (isLTOIndexing && allLTOArtifacts != null) { for (LTOBackendArtifacts a : allLTOArtifacts) { List argv = new ArrayList<>(); - argv.addAll(toolchain.getLinkOptions()); + argv.addAll(cppConfiguration.getLinkOptions()); argv.addAll(cppConfiguration.getCompilerOptions(features)); a.setCommandLine(argv); @@ -914,7 +914,7 @@ public class CppLinkActionBuilder { boolean sharedLinkopts = type == LinkTargetType.DYNAMIC_LIBRARY || linkopts.contains("-shared") - || cppConfig.hasSharedLinkOption(); + || cppConfig.getLinkOptions().contains("-shared"); return (isNativeDeps || cppConfig.legacyWholeArchive()) && (fullyStatic || mostlyStatic) && sharedLinkopts; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java index 1844609dd1..f35954589d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FakeCppCompileAction.java @@ -82,8 +82,6 @@ public class FakeCppCompileAction extends CppCompileAction { Iterable lipoScannables, Iterable builtinIncludeFiles, CppSemantics cppSemantics, - ImmutableList unfilteredCompilerOptions, - ImmutableList builtInIncludeDirectories, ImmutableMap executionInfo) { super( owner, @@ -126,9 +124,7 @@ public class FakeCppCompileAction extends CppCompileAction { ImmutableMap.of(), CppCompileAction.CPP_COMPILE, builtinIncludeFiles, - cppSemantics, - unfilteredCompilerOptions, - builtInIncludeDirectories); + cppSemantics); this.tempOutputFile = Preconditions.checkNotNull(tempOutputFile); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java index 491e481322..bbb3b4ddb8 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/LinkCommandLine.java @@ -74,7 +74,6 @@ public final class LinkCommandLine extends CommandLine { @Nullable private final PathFragment runtimeSolibDir; private final boolean nativeDeps; private final boolean useTestOnlyFlags; - private final CcToolchainProvider ccProvider; @Nullable private final Artifact paramFile; @@ -99,8 +98,7 @@ public final class LinkCommandLine extends CommandLine { boolean useTestOnlyFlags, @Nullable Artifact paramFile, CcToolchainFeatures.Variables variables, - @Nullable FeatureConfiguration featureConfiguration, - CcToolchainProvider ccProvider) { + @Nullable FeatureConfiguration featureConfiguration) { this.actionName = actionName; this.toolPath = toolPath; @@ -127,7 +125,6 @@ public final class LinkCommandLine extends CommandLine { this.nativeDeps = nativeDeps; this.useTestOnlyFlags = useTestOnlyFlags; this.paramFile = paramFile; - this.ccProvider = ccProvider; } @Nullable @@ -325,7 +322,7 @@ public final class LinkCommandLine extends CommandLine { boolean sharedLinkopts = linkTargetType == LinkTargetType.DYNAMIC_LIBRARY || linkopts.contains("-shared") - || cppConfiguration.hasSharedLinkOption(); + || cppConfiguration.getLinkOptions().contains("-shared"); List toolchainFlags = new ArrayList<>(); @@ -356,7 +353,7 @@ public final class LinkCommandLine extends CommandLine { toolchainFlags.addAll(cppConfiguration.getTestOnlyLinkOptions()); } - toolchainFlags.addAll(ccProvider.getLinkOptions()); + toolchainFlags.addAll(cppConfiguration.getLinkOptions()); // -pie is not compatible with shared and should be // removed when the latter is part of the link command. Should we need to further @@ -575,7 +572,7 @@ public final class LinkCommandLine extends CommandLine { optionList.add("-I."); // Add sysroot. - PathFragment sysroot = ccProvider.getSysroot(); + PathFragment sysroot = cppConfiguration.getSysroot(); if (sysroot != null) { optionList.add("--sysroot=" + sysroot.getPathString()); } @@ -583,7 +580,7 @@ public final class LinkCommandLine extends CommandLine { // Add toolchain compiler options. optionList.addAll(cppConfiguration.getCompilerOptions(features)); optionList.addAll(cppConfiguration.getCOptions()); - optionList.addAll(ccProvider.getUnfilteredCompilerOptions(features)); + optionList.addAll(cppConfiguration.getUnfilteredCompilerOptions(features)); if (CppFileTypes.CPP_SOURCE.matches(linkstamp.getKey().getExecPath())) { optionList.addAll(cppConfiguration.getCxxOptions(features)); } @@ -637,7 +634,7 @@ public final class LinkCommandLine extends CommandLine { private final BuildConfiguration configuration; private final ActionOwner owner; - private final RuleContext ruleContext; + @Nullable private final RuleContext ruleContext; @Nullable private String toolPath; @Nullable private Artifact output; @@ -654,7 +651,7 @@ public final class LinkCommandLine extends CommandLine { private boolean nativeDeps; private boolean useTestOnlyFlags; @Nullable private Artifact paramFile; - private CcToolchainProvider toolchain; + @Nullable private CcToolchainProvider toolchain; private FdoSupport fdoSupport; private Variables variables; private FeatureConfiguration featureConfiguration; @@ -662,7 +659,8 @@ public final class LinkCommandLine extends CommandLine { // This interface is needed to support tests that don't create a // ruleContext, in which case the configuration and action owner // cannot be accessed off of the give ruleContext. - public Builder(BuildConfiguration configuration, ActionOwner owner, RuleContext ruleContext) { + public Builder( + BuildConfiguration configuration, ActionOwner owner, @Nullable RuleContext ruleContext) { this.configuration = configuration; this.owner = owner; this.ruleContext = ruleContext; @@ -691,16 +689,17 @@ public final class LinkCommandLine extends CommandLine { Iterables.concat(DEFAULT_LINKSTAMP_OPTIONS, linkstampCompileOptions)); } - if (toolchain == null) { - toolchain = - Preconditions.checkNotNull(CppHelper.getToolchain(ruleContext, ":cc_toolchain")); - } - // The ruleContext can be null for some tests. if (ruleContext != null) { if (featureConfiguration == null) { - featureConfiguration = - CcCommon.configureFeatures(ruleContext, toolchain, CcLibraryHelper.SourceCategory.CC); + if (toolchain != null) { + featureConfiguration = + CcCommon.configureFeatures( + ruleContext, toolchain, CcLibraryHelper.SourceCategory.CC); + } else { + CcToolchainProvider ccToolchain = CppHelper.getToolchain(ruleContext, ":cc_toolchain"); + featureConfiguration = CcCommon.configureFeatures(ruleContext, ccToolchain); + } } if (fdoSupport == null) { @@ -735,8 +734,7 @@ public final class LinkCommandLine extends CommandLine { useTestOnlyFlags, paramFile, variables, - featureConfiguration, - toolchain); + featureConfiguration); } /** -- cgit v1.2.3