diff options
author | 2017-05-09 04:53:37 -0400 | |
---|---|---|
committer | 2017-05-09 10:54:18 -0400 | |
commit | cbbb423663b154d82e3dfa5e9a56839583987999 (patch) | |
tree | 59e7c03030c010d82ede7d4f5087e958a1ebe2d7 /src | |
parent | da10d78d723bf2cffa6a1721d840b1125a7cddd1 (diff) |
RELNOTES: Effectively remove sysroot from CppConfiguration and allow it to use select statements.
PiperOrigin-RevId: 155480011
Diffstat (limited to 'src')
16 files changed, 289 insertions, 290 deletions
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 ca98d7bf61..7e15d9e462 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,8 +523,7 @@ public abstract class CcBinary implements RuleConfiguredTargetFactory { private static final boolean dashStaticInLinkopts(List<String> linkopts, CppConfiguration cppConfiguration) { - return linkopts.contains("-static") - || cppConfiguration.getLinkOptions().contains("-static"); + return linkopts.contains("-static") || cppConfiguration.hasStaticLinkOption(); } 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 6fc4c92938..da27ada49e 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,6 +36,7 @@ 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; @@ -284,6 +285,15 @@ public class CcToolchain implements RuleConfiguredTargetFactory { coverage = crosstool; } + PathFragment sysroot = calculateSysroot(ruleContext); + + ImmutableList<PathFragment> builtInIncludeDirectories = null; + try { + builtInIncludeDirectories = cppConfiguration.getBuiltInIncludeDirectories(sysroot); + } catch (InvalidConfigurationException e) { + ruleContext.ruleError(e.getMessage()); + } + CcToolchainProvider ccProvider = new CcToolchainProvider( cppConfiguration, @@ -310,9 +320,8 @@ public class CcToolchain implements RuleConfiguredTargetFactory { coverageEnvironment.build(), ruleContext.getPrerequisiteArtifact("$link_dynamic_library_tool", Mode.HOST), getEnvironment(ruleContext), - cppConfiguration.getBuiltInIncludeDirectories()); - - PathFragment sysroot = cppConfiguration.getSysroot(); + builtInIncludeDirectories, + sysroot); MakeVariableProvider makeVariableProvider = createMakeVariableProvider(cppConfiguration, sysroot); @@ -481,4 +490,16 @@ public class CcToolchain implements RuleConfiguredTargetFactory { protected ImmutableMap<String, String> getEnvironment(RuleContext ruleContext) { return ImmutableMap.<String, String>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 26d4c38b67..89beafe21d 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,7 +75,8 @@ public final class CcToolchainProvider NestedSetBuilder.<Pair<String, String>>emptySet(Order.COMPILE_ORDER), null, ImmutableMap.<String, String>of(), - ImmutableList.<PathFragment>of()); + ImmutableList.<PathFragment>of(), + null); @Nullable private final CppConfiguration cppConfiguration; private final NestedSet<Artifact> crosstool; @@ -102,6 +103,7 @@ public final class CcToolchainProvider @Nullable private final Artifact linkDynamicLibraryTool; private final ImmutableMap<String, String> environment; private final ImmutableList<PathFragment> builtInIncludeDirectories; + @Nullable private final PathFragment sysroot; public CcToolchainProvider( @Nullable CppConfiguration cppConfiguration, @@ -128,7 +130,8 @@ public final class CcToolchainProvider NestedSet<Pair<String, String>> coverageEnvironment, Artifact linkDynamicLibraryTool, ImmutableMap<String, String> environment, - ImmutableList<PathFragment> builtInIncludeDirectories) { + ImmutableList<PathFragment> builtInIncludeDirectories, + @Nullable PathFragment sysroot) { super(SKYLARK_CONSTRUCTOR, ImmutableMap.<String, Object>of()); this.cppConfiguration = cppConfiguration; this.crosstool = Preconditions.checkNotNull(crosstool); @@ -155,6 +158,7 @@ public final class CcToolchainProvider this.linkDynamicLibraryTool = linkDynamicLibraryTool; this.environment = environment; this.builtInIncludeDirectories = builtInIncludeDirectories; + this.sysroot = sysroot; } @SkylarkCallable( @@ -352,7 +356,7 @@ public final class CcToolchainProvider + "this method returns <code>None</code>." ) public PathFragment getSysroot() { - return cppConfiguration.getSysroot(); + return sysroot; } @SkylarkCallable( @@ -362,7 +366,7 @@ public final class CcToolchainProvider + "rules. These should be appended to the command line after filtering." ) public ImmutableList<String> getUnfilteredCompilerOptions(Iterable<String> features) { - return cppConfiguration.getUnfilteredCompilerOptions(features); + return cppConfiguration.getUnfilteredCompilerOptions(features, sysroot); } @SkylarkCallable( @@ -373,6 +377,6 @@ public final class CcToolchainProvider + "inferred from the command-line options." ) public ImmutableList<String> getLinkOptions() { - return cppConfiguration.getLinkOptions(); + return cppConfiguration.getLinkOptions(sysroot); } } 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 a1922b8553..8b7dbe92ff 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,9 +51,8 @@ public final class CcToolchainRule implements RuleDefinition { private static final LateBoundLabel<BuildConfiguration> LIBC_TOP = new LateBoundLabel<BuildConfiguration>(CppConfiguration.class) { @Override - public Label resolve( - Rule rule, AttributeMap attributes, BuildConfiguration configuration) { - return configuration.getFragment(CppConfiguration.class).getLibcLabel(); + public Label resolve(Rule rule, AttributeMap attributes, BuildConfiguration configuration) { + return configuration.getFragment(CppConfiguration.class).getSysrootLabel(); } }; @@ -88,6 +87,7 @@ 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 f9fc4c3f6b..4310cef570 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,6 +45,7 @@ public final class CompileCommandLine { @VisibleForTesting public final CcToolchainFeatures.Variables variables; private final String actionName; private final CppConfiguration cppConfiguration; + private final ImmutableList<String> unfilteredCompilerOptions; public CompileCommandLine( Artifact sourceFile, @@ -57,7 +58,8 @@ public final class CompileCommandLine { CppConfiguration cppConfiguration, CcToolchainFeatures.Variables variables, String actionName, - DotdFile dotdFile) { + DotdFile dotdFile, + ImmutableList<String> unfilteredCompilerOptions) { this.sourceFile = Preconditions.checkNotNull(sourceFile); this.outputFile = Preconditions.checkNotNull(outputFile); this.sourceLabel = Preconditions.checkNotNull(sourceLabel); @@ -69,6 +71,7 @@ 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. */ @@ -161,7 +164,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(toolchain.getUnfilteredCompilerOptions(features)); + options.addAll(unfilteredCompilerOptions); } // 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 1b1884ce8d..20fe8e030d 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,6 +217,8 @@ public class CppCompileAction extends AbstractAction /** Whether this action needs to discover inputs. */ private final boolean discoversInputs; + private final ImmutableList<PathFragment> builtInIncludeDirectories; + /** * Set when the action prepares for execution. Used to preserve state between preparation and * execution. @@ -271,6 +273,8 @@ 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, @@ -308,7 +312,9 @@ public class CppCompileAction extends AbstractAction ImmutableMap<String, String> environment, String actionName, Iterable<Artifact> builtinIncludeFiles, - CppSemantics cppSemantics) { + CppSemantics cppSemantics, + ImmutableList<String> unfilteredCompilerOptions, + ImmutableList<PathFragment> builtInIncludeDirectories) { super( owner, allInputs, @@ -349,7 +355,8 @@ public class CppCompileAction extends AbstractAction cppConfiguration, variables, actionName, - dotdFile); + dotdFile, + unfilteredCompilerOptions); this.actionContext = actionContext; this.lipoScannables = lipoScannables; this.actionClassId = actionClassId; @@ -363,6 +370,7 @@ public class CppCompileAction extends AbstractAction this.builtinIncludeFiles = ImmutableList.copyOf(builtinIncludeFiles); this.cppSemantics = cppSemantics; this.additionalIncludeScannables = ImmutableList.copyOf(additionalIncludeScannables); + this.builtInIncludeDirectories = builtInIncludeDirectories; } /** @@ -381,7 +389,7 @@ public class CppCompileAction extends AbstractAction @Override public List<PathFragment> getBuiltInIncludeDirectories() { - return cppConfiguration.getBuiltInIncludeDirectories(); + return builtInIncludeDirectories; } @Nullable @@ -835,9 +843,10 @@ public class CppCompileAction extends AbstractAction if (optionalSourceFile != null) { allowedIncludes.add(optionalSourceFile); } - Iterable<PathFragment> ignoreDirs = cppConfiguration.isStrictSystemIncludes() - ? cppConfiguration.getBuiltInIncludeDirectories() - : getValidationIgnoredDirs(); + Iterable<PathFragment> ignoreDirs = + cppConfiguration.isStrictSystemIncludes() + ? getBuiltInIncludeDirectories() + : getValidationIgnoredDirs(); // Copy the sets to hash sets for fast contains checking. // Avoid immutable sets here to limit memory churn. @@ -906,7 +915,7 @@ public class CppCompileAction extends AbstractAction } Iterable<PathFragment> getValidationIgnoredDirs() { - List<PathFragment> cxxSystemIncludeDirs = cppConfiguration.getBuiltInIncludeDirectories(); + List<PathFragment> cxxSystemIncludeDirs = getBuiltInIncludeDirectories(); return Iterables.concat( cxxSystemIncludeDirs, context.getSystemIncludeDirs()); } @@ -1259,9 +1268,8 @@ public class CppCompileAction extends AbstractAction } public List<Path> getPermittedSystemIncludePrefixes(Path execRoot) { - CppConfiguration toolchain = cppConfiguration; List<Path> systemIncludePrefixes = new ArrayList<>(); - for (PathFragment includePath : toolchain.getBuiltInIncludeDirectories()) { + for (PathFragment includePath : 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 333361f193..e4bf2b3b4c 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,6 +386,8 @@ public class CppCompileActionBuilder { getLipoScannables(realMandatoryInputs), ccToolchain.getBuiltinIncludeFiles(), cppSemantics, + ccToolchain.getUnfilteredCompilerOptions(features), + ccToolchain.getBuiltInIncludeDirectories(), ImmutableMap.copyOf(executionInfo)); } else { return new CppCompileAction( @@ -422,7 +424,9 @@ public class CppCompileActionBuilder { ImmutableMap.copyOf(environment), getActionName(), ccToolchain.getBuiltinIncludeFiles(), - cppSemantics); + cppSemantics, + ccToolchain.getUnfilteredCompilerOptions(features), + ccToolchain.getBuiltInIncludeDirectories()); } } 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 4388571311..765f4d6f6d 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,12 +25,10 @@ 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; @@ -60,7 +58,6 @@ 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; @@ -162,127 +159,6 @@ 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()}. @@ -381,15 +257,17 @@ 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; - private final PathFragment sysroot; + // 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 runtimeSysroot; - private final ImmutableList<PathFragment> builtInIncludeDirectories; + private final ImmutableList<String> rawBuiltInIncludeDirectories; private final Map<String, PathFragment> toolPaths; private final PathFragment ldExecutable; @@ -614,29 +492,14 @@ public class CppConfiguration extends BuildConfiguration.Fragment { this.abiGlibcVersion = toolchain.getAbiLibcVersion(); // The default value for optional string attributes is the empty string. - 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.defaultSysroot = computeDefaultSysroot(toolchain); - LibcTop libcTop = params.libcTop; - if ((libcTop != null) && (libcTop.getLabel() != null)) { - libcLabel = libcTop.getLabel(); - } else { - libcLabel = null; - } + this.sysrootLabel = params.sysrootLabel; + this.nonConfiguredSysroot = + params.sysrootLabel == null ? defaultSysroot : params.sysrootLabel.getPackageFragment(); - ImmutableList.Builder<PathFragment> builtInIncludeDirectoriesBuilder - = ImmutableList.builder(); - sysroot = libcTop == null ? defaultSysroot : libcTop.getSysroot(); - for (String s : toolchain.getCxxBuiltinIncludeDirectoryList()) { - builtInIncludeDirectoriesBuilder.add( - resolveIncludeDir(s, sysroot, crosstoolTopPathFragment)); - } - builtInIncludeDirectories = builtInIncludeDirectoriesBuilder.build(); + rawBuiltInIncludeDirectories = + ImmutableList.copyOf(toolchain.getCxxBuiltinIncludeDirectoryList()); // 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 @@ -644,17 +507,8 @@ public class CppConfiguration extends BuildConfiguration.Fragment { // and libc versions, you must always choose compatible ones. runtimeSysroot = defaultSysroot; - String sysrootFlag; - if (sysroot != null) { - sysrootFlag = "--sysroot=" + sysroot; - } else { - sysrootFlag = null; - } - ImmutableList.Builder<String> unfilteredCoptsBuilder = ImmutableList.builder(); - if (sysrootFlag != null) { - unfilteredCoptsBuilder.add(sysrootFlag); - } + unfilteredCoptsBuilder.addAll(toolchain.getUnfilteredCxxFlagList()); unfilteredCompilerFlags = new FlagList( unfilteredCoptsBuilder.build(), @@ -666,9 +520,6 @@ 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<String> ltoindexoptsBuilder = ImmutableList.builder(); @@ -730,12 +581,6 @@ 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); } @@ -1278,14 +1123,6 @@ public class CppConfiguration extends BuildConfiguration.Fragment { } /** - * 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 * target architecture. @@ -1433,17 +1270,30 @@ 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<PathFragment> getBuiltInIncludeDirectories() { - return builtInIncludeDirectories; + @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<PathFragment> getBuiltInIncludeDirectories() + throws InvalidConfigurationException { + return getBuiltInIncludeDirectories(nonConfiguredSysroot); + } + + public ImmutableList<PathFragment> getBuiltInIncludeDirectories(PathFragment sysroot) + throws InvalidConfigurationException { + ImmutableList.Builder<PathFragment> builtInIncludeDirectoriesBuilder = ImmutableList.builder(); + for (String s : rawBuiltInIncludeDirectories) { + builtInIncludeDirectoriesBuilder.add(resolveIncludeDir(s, sysroot, crosstoolTopPathFragment)); + } + return builtInIncludeDirectoriesBuilder.build(); } /** @@ -1456,7 +1306,11 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + "different sysroots, or the sysroot is the same as the default sysroot, then " + "this method returns <code>None</code>.") public PathFragment getSysroot() { - return sysroot; + return nonConfiguredSysroot; + } + + public Label getSysrootLabel() { + return sysrootLabel; } /** @@ -1516,8 +1370,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", @@ -1526,21 +1380,56 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + "rules. These should be appended to the command line after filtering." ) public ImmutableList<String> getUnfilteredCompilerOptions(Iterable<String> features) { - return unfilteredCompilerFlags.evaluate(features); + return getUnfilteredCompilerOptions(features, nonConfiguredSysroot); + } + + public ImmutableList<String> getUnfilteredCompilerOptions( + Iterable<String> features, PathFragment sysroot) { + if (sysroot == null) { + return unfilteredCompilerFlags.evaluate(features); + } else { + return ImmutableList.<String>builder() + .add("--sysroot=" + sysroot) + .addAll(unfilteredCompilerFlags.evaluate(features)) + .build(); + } } /** - * 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<String> getLinkOptions() { - return linkOptions; + return getLinkOptions(nonConfiguredSysroot); + } + + public ImmutableList<String> getLinkOptions(PathFragment sysroot) { + if (sysroot == null) { + return linkOptions; + } else { + return ImmutableList.<String>builder() + .addAll(linkOptions) + .add("--sysroot=" + sysroot) + .build(); + } + } + + public boolean hasStaticLinkOption() { + return linkOptions.contains("-static"); + } + + public boolean hasSharedLinkOption() { + return linkOptions.contains("-shared"); } /** Returns the set of command-line LTO indexing options. */ @@ -2170,6 +2059,8 @@ 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()); @@ -2252,6 +2143,22 @@ 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 9a98181724..b334b10923 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,10 +36,11 @@ 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; /** @@ -89,7 +90,7 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { protected final Label ccToolchainLabel; protected final Label stlLabel; protected final Path fdoZip; - protected final LibcTop libcTop; + protected final Label sysrootLabel; CppConfigurationParameters( CrosstoolConfig.CToolchain toolchain, @@ -99,7 +100,7 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { Label crosstoolTop, Label ccToolchainLabel, Label stlLabel, - LibcTop libcTop) { + Label sysrootLabel) { this.toolchain = toolchain; this.cacheKeySuffix = cacheKeySuffix; this.commonOptions = buildOptions.get(BuildConfiguration.Options.class); @@ -108,7 +109,7 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { this.crosstoolTop = crosstoolTop; this.ccToolchainLabel = ccToolchainLabel; this.stlLabel = stlLabel; - this.libcTop = libcTop; + this.sysrootLabel = sysrootLabel; } } @@ -219,10 +220,7 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { "The label '%s' is not a cc_toolchain rule", ccToolchainLabel)); } - LibcTop.Result libcTopResult = LibcTop.createLibcTop(cppOptions, env, toolchain); - if (libcTopResult.valuesMissing()) { - return null; - } + Label sysrootLabel = getSysrootLabel(toolchain, cppOptions.libcTopLabel); return new CppConfigurationParameters( toolchain, @@ -232,6 +230,34 @@ public class CppConfigurationLoader implements ConfigurationFragmentFactory { crosstoolTopLabel, ccToolchainLabel, stlLabel, - libcTopResult.getLibcTop()); + 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; } } 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 aa719c80ed..ea3fd42ddd 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).getLinkOptions().contains("-static")) { + if (ruleContext.getFragment(CppConfiguration.class).hasStaticLinkOption()) { 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 0c640ff818..18d7102092 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<String> argv = new ArrayList<>(); - argv.addAll(cppConfiguration.getLinkOptions()); + argv.addAll(toolchain.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.getLinkOptions().contains("-shared"); + || cppConfig.hasSharedLinkOption(); 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 f35954589d..1844609dd1 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,6 +82,8 @@ public class FakeCppCompileAction extends CppCompileAction { Iterable<IncludeScannable> lipoScannables, Iterable<Artifact> builtinIncludeFiles, CppSemantics cppSemantics, + ImmutableList<String> unfilteredCompilerOptions, + ImmutableList<PathFragment> builtInIncludeDirectories, ImmutableMap<String, String> executionInfo) { super( owner, @@ -124,7 +126,9 @@ public class FakeCppCompileAction extends CppCompileAction { ImmutableMap.<String, String>of(), CppCompileAction.CPP_COMPILE, builtinIncludeFiles, - cppSemantics); + cppSemantics, + unfilteredCompilerOptions, + builtInIncludeDirectories); 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 bbb3b4ddb8..491e481322 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,6 +74,7 @@ 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; @@ -98,7 +99,8 @@ public final class LinkCommandLine extends CommandLine { boolean useTestOnlyFlags, @Nullable Artifact paramFile, CcToolchainFeatures.Variables variables, - @Nullable FeatureConfiguration featureConfiguration) { + @Nullable FeatureConfiguration featureConfiguration, + CcToolchainProvider ccProvider) { this.actionName = actionName; this.toolPath = toolPath; @@ -125,6 +127,7 @@ public final class LinkCommandLine extends CommandLine { this.nativeDeps = nativeDeps; this.useTestOnlyFlags = useTestOnlyFlags; this.paramFile = paramFile; + this.ccProvider = ccProvider; } @Nullable @@ -322,7 +325,7 @@ public final class LinkCommandLine extends CommandLine { boolean sharedLinkopts = linkTargetType == LinkTargetType.DYNAMIC_LIBRARY || linkopts.contains("-shared") - || cppConfiguration.getLinkOptions().contains("-shared"); + || cppConfiguration.hasSharedLinkOption(); List<String> toolchainFlags = new ArrayList<>(); @@ -353,7 +356,7 @@ public final class LinkCommandLine extends CommandLine { toolchainFlags.addAll(cppConfiguration.getTestOnlyLinkOptions()); } - toolchainFlags.addAll(cppConfiguration.getLinkOptions()); + toolchainFlags.addAll(ccProvider.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 @@ -572,7 +575,7 @@ public final class LinkCommandLine extends CommandLine { optionList.add("-I."); // Add sysroot. - PathFragment sysroot = cppConfiguration.getSysroot(); + PathFragment sysroot = ccProvider.getSysroot(); if (sysroot != null) { optionList.add("--sysroot=" + sysroot.getPathString()); } @@ -580,7 +583,7 @@ public final class LinkCommandLine extends CommandLine { // Add toolchain compiler options. optionList.addAll(cppConfiguration.getCompilerOptions(features)); optionList.addAll(cppConfiguration.getCOptions()); - optionList.addAll(cppConfiguration.getUnfilteredCompilerOptions(features)); + optionList.addAll(ccProvider.getUnfilteredCompilerOptions(features)); if (CppFileTypes.CPP_SOURCE.matches(linkstamp.getKey().getExecPath())) { optionList.addAll(cppConfiguration.getCxxOptions(features)); } @@ -634,7 +637,7 @@ public final class LinkCommandLine extends CommandLine { private final BuildConfiguration configuration; private final ActionOwner owner; - @Nullable private final RuleContext ruleContext; + private final RuleContext ruleContext; @Nullable private String toolPath; @Nullable private Artifact output; @@ -651,7 +654,7 @@ public final class LinkCommandLine extends CommandLine { private boolean nativeDeps; private boolean useTestOnlyFlags; @Nullable private Artifact paramFile; - @Nullable private CcToolchainProvider toolchain; + private CcToolchainProvider toolchain; private FdoSupport fdoSupport; private Variables variables; private FeatureConfiguration featureConfiguration; @@ -659,8 +662,7 @@ 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, @Nullable RuleContext ruleContext) { + public Builder(BuildConfiguration configuration, ActionOwner owner, RuleContext ruleContext) { this.configuration = configuration; this.owner = owner; this.ruleContext = ruleContext; @@ -689,17 +691,16 @@ 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) { - if (toolchain != null) { - featureConfiguration = - CcCommon.configureFeatures( - ruleContext, toolchain, CcLibraryHelper.SourceCategory.CC); - } else { - CcToolchainProvider ccToolchain = CppHelper.getToolchain(ruleContext, ":cc_toolchain"); - featureConfiguration = CcCommon.configureFeatures(ruleContext, ccToolchain); - } + featureConfiguration = + CcCommon.configureFeatures(ruleContext, toolchain, CcLibraryHelper.SourceCategory.CC); } if (fdoSupport == null) { @@ -734,7 +735,8 @@ public final class LinkCommandLine extends CommandLine { useTestOnlyFlags, paramFile, variables, - featureConfiguration); + featureConfiguration, + toolchain); } /** diff --git a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java index 5b49663020..7f88ad3241 100644 --- a/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/analysis/util/AnalysisTestCase.java @@ -13,6 +13,7 @@ // limitations under the License. package com.google.devtools.build.lib.analysis.util; + import com.google.common.base.Predicates; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; @@ -126,6 +127,7 @@ public abstract class AnalysisTestCase extends FoundationTestCase { protected MockToolsConfig mockToolsConfig; protected AnalysisMock analysisMock; + protected BuildOptions buildOptions; private OptionsParser optionsParser; protected PackageManager packageManager; private LoadingPhaseRunner loadingPhaseRunner; @@ -246,6 +248,8 @@ public abstract class AnalysisTestCase extends FoundationTestCase { InvocationPolicyEnforcer optionsPolicyEnforcer = analysisMock.getInvocationPolicyEnforcer(); optionsPolicyEnforcer.enforce(optionsParser); + + buildOptions = ruleClassProvider.createBuildOptions(optionsParser); } protected FlagBuilder defaultFlags() { @@ -321,8 +325,6 @@ public abstract class AnalysisTestCase extends FoundationTestCase { viewOptions.keepGoing = flags.contains(Flag.KEEP_GOING); viewOptions.loadingPhaseThreads = LOADING_PHASE_THREADS; - BuildOptions buildOptions = ruleClassProvider.createBuildOptions(optionsParser); - PackageCacheOptions packageCacheOptions = optionsParser.getOptions(PackageCacheOptions.class); PathPackageLocator pathPackageLocator = PathPackageLocator.create( outputBase, packageCacheOptions.packagePath, reporter, rootDirectory, rootDirectory); 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 30b5af1d95..7c0c8ad8e2 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 @@ -25,28 +25,27 @@ import static org.junit.Assert.fail; import com.google.common.base.Functions; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; -import com.google.devtools.build.lib.analysis.config.BuildOptions; +import com.google.devtools.build.lib.analysis.ConfiguredTarget; 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.FragmentOptions; import com.google.devtools.build.lib.analysis.config.InvalidConfigurationException; import com.google.devtools.build.lib.analysis.util.AnalysisTestCase; import com.google.devtools.build.lib.cmdline.Label; import com.google.devtools.build.lib.cmdline.LabelSyntaxException; import com.google.devtools.build.lib.cmdline.PackageIdentifier; -import com.google.devtools.build.lib.flags.InvocationPolicyEnforcer; import com.google.devtools.build.lib.packages.util.MockCcSupport; +import com.google.devtools.build.lib.rules.MakeVariableProvider; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool; import com.google.devtools.build.lib.testutil.TestConstants; import com.google.devtools.build.lib.testutil.TestRuleClassProvider; +import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; -import com.google.devtools.common.options.OptionsParser; -import com.google.devtools.common.options.OptionsParsingException; import java.io.IOException; import java.util.Arrays; import java.util.Collection; import java.util.Collections; +import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; @@ -58,25 +57,12 @@ import org.junit.runners.JUnit4; public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { private static final Collection<String> NO_FEATURES = Collections.emptySet(); - private BuildOptions createBuildOptionsForTest(String... args) { - ImmutableList<Class<? extends FragmentOptions>> testFragments = - TestRuleClassProvider.getRuleClassProvider().getConfigurationOptions(); - OptionsParser optionsParser = OptionsParser.newOptionsParser(testFragments); - try { - optionsParser.parse(args); - InvocationPolicyEnforcer optionsPolicyEnforcer = analysisMock.getInvocationPolicyEnforcer(); - optionsPolicyEnforcer.enforce(optionsParser); - } catch (OptionsParsingException e) { - throw new IllegalStateException(e); - } - return BuildOptions.applyStaticConfigOverride(BuildOptions.of(testFragments, optionsParser)); - } - private CppConfiguration create(CppConfigurationLoader loader, String... args) throws Exception { + useConfiguration(args); ConfigurationEnvironment env = new ConfigurationEnvironment.TargetProviderEnvironment( skyframeExecutor.getPackageManager(), reporter, directories); - return loader.create(env, createBuildOptionsForTest(args)); + return loader.create(env, buildOptions); } private CppConfigurationLoader loader(String crosstoolFileContents) throws IOException { @@ -84,6 +70,11 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { return new CppConfigurationLoader(Functions.<String>identity()); } + @Before + public void setupTests() throws Exception { + useRuleClassProvider(TestRuleClassProvider.getRuleClassProvider()); + } + private CppConfigurationLoader loaderWithOptionalTool(String optionalTool) throws IOException { return loader( "major_version: \"12\"" @@ -167,6 +158,18 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { + "}"); } + private ConfiguredTarget getCcToolchainTarget(CppConfiguration cppConfiguration) + throws Exception { + update(cppConfiguration.getCcToolchainRuleLabel().toString()); + return Preconditions.checkNotNull( + getConfiguredTarget(cppConfiguration.getCcToolchainRuleLabel().toString())); + } + + private CcToolchainProvider getCcToolchainProvider(CppConfiguration cppConfiguration) + throws Exception { + return getCcToolchainTarget(cppConfiguration).getProvider(CcToolchainProvider.class); + } + /** * Checks that we do not accidentally change the proto format in incompatible * ways. Do not modify the configuration file in this test, except if you are @@ -176,7 +179,10 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { public void testSimpleCompleteConfiguration() throws Exception { CppConfigurationLoader loader = loaderWithOptionalTool(""); - CppConfiguration toolchain = create(loader, "--cpu=cpu"); + // Need to clear out the android cpu options to avoid this split transition in Bazel. + CppConfiguration toolchain = + create(loader, "--cpu=cpu", "--host_cpu=cpu", "--android_cpu=", "--fat_apk_cpu="); + CcToolchainProvider ccProvider = getCcToolchainProvider(toolchain); assertEquals("toolchain-identifier", toolchain.getToolchainIdentifier()); assertEquals("host-system-name", toolchain.getHostSystemName()); @@ -197,17 +203,16 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { assertFalse(toolchain.toolchainNeedsPic()); assertTrue(toolchain.supportsFission()); - assertEquals( - ImmutableList.of(getToolPath("/system-include-dir")), - toolchain.getBuiltInIncludeDirectories()); - assertNull(toolchain.getSysroot()); + assertThat(ccProvider.getBuiltInIncludeDirectories()) + .containsExactly(getToolPath("/system-include-dir")); + assertNull(ccProvider.getSysroot()); assertEquals(Arrays.asList("c", "fastbuild"), toolchain.getCompilerOptions(NO_FEATURES)); assertEquals(Arrays.<String>asList(), toolchain.getCOptions()); assertEquals(Arrays.asList("cxx", "cxx-fastbuild"), toolchain.getCxxOptions(NO_FEATURES)); - assertEquals(Arrays.asList("unfiltered"), toolchain.getUnfilteredCompilerOptions(NO_FEATURES)); + assertEquals(Arrays.asList("unfiltered"), ccProvider.getUnfilteredCompilerOptions(NO_FEATURES)); - assertEquals(Arrays.<String>asList(), toolchain.getLinkOptions()); + assertEquals(Arrays.<String>asList(), ccProvider.getLinkOptions()); assertEquals( Arrays.asList("linker", "linker-fastbuild", "fully static"), toolchain.getFullyStaticLinkOptions(NO_FEATURES, false)); @@ -482,7 +487,12 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { "licenses(['unencumbered'])", "filegroup(name = 'everything')"); - CppConfiguration toolchainA = create(loader, "--cpu=piii"); + // Need to clear out the android cpu options to avoid this split transition in Bazel. + CppConfiguration toolchainA = + create(loader, "--cpu=piii", "--host_cpu=piii", "--android_cpu=", "--fat_apk_cpu="); + ConfiguredTarget ccToolchainA = getCcToolchainTarget(toolchainA); + CcToolchainProvider ccProviderA = ccToolchainA.getProvider(CcToolchainProvider.class); + MakeVariableProvider makeProviderA = ccToolchainA.getProvider(MakeVariableProvider.class); assertEquals("toolchain-identifier-A", toolchainA.getToolchainIdentifier()); assertEquals("host-system-name-A", toolchainA.getHostSystemName()); assertEquals("target-system-name-A", toolchainA.getTargetGnuSystemName()); @@ -515,7 +525,7 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { toolchainA.getCxxOptions(NO_FEATURES)); assertEquals( Arrays.asList("--sysroot=some", "unfiltered-flag-A-1", "unfiltered-flag-A-2"), - toolchainA.getUnfilteredCompilerOptions(NO_FEATURES)); + ccProviderA.getUnfilteredCompilerOptions(NO_FEATURES)); assertEquals( Arrays.asList( "linker-flag-A-1", @@ -584,7 +594,7 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { toolchainA.getLdOptionsForEmbedding()); assertEquals(Arrays.asList("ar-flag-A"), toolchainA.getArFlags()); - assertThat(toolchainA.getAdditionalMakeVariables().entrySet()) + assertThat(makeProviderA.getMakeVariables().entrySet()) .containsExactlyElementsIn( ImmutableMap.<String, String>builder() .put("SOME_MAKE_VARIABLE-A-1", "make-variable-value-A-1") @@ -596,8 +606,8 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { assertEquals( Arrays.asList( getToolPath("/system-include-dir-A-1"), getToolPath("/system-include-dir-A-2")), - toolchainA.getBuiltInIncludeDirectories()); - assertEquals(PathFragment.create("some"), toolchainA.getSysroot()); + ccProviderA.getBuiltInIncludeDirectories()); + assertEquals(PathFragment.create("some"), ccProviderA.getSysroot()); // Cursory testing of the "B" toolchain only; assume that if none of // toolchain B bled through into toolchain A, the reverse also didn't occur. And @@ -607,8 +617,17 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { // Make sure nothing bled through to the nearly-empty "C" toolchain. This is also testing for // all the defaults. + // Need to clear out the android cpu options to avoid this split transition in Bazel. CppConfiguration toolchainC = - create(loader, "--compiler=compiler-C", "--glibc=target-libc-C", "--cpu=piii"); + create( + loader, + "--compiler=compiler-C", + "--glibc=target-libc-C", + "--cpu=piii", + "--host_cpu=piii", + "--android_cpu=", + "--fat_apk_cpu="); + CcToolchainProvider ccProviderC = getCcToolchainProvider(toolchainC); assertEquals("toolchain-identifier-C", toolchainC.getToolchainIdentifier()); assertEquals("host-system-name-C", toolchainC.getHostSystemName()); assertEquals("target-system-name-C", toolchainC.getTargetGnuSystemName()); @@ -628,7 +647,7 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { assertThat(toolchainC.getCompilerOptions(NO_FEATURES)).isEmpty(); assertThat(toolchainC.getCOptions()).isEmpty(); assertThat(toolchainC.getCxxOptions(NO_FEATURES)).isEmpty(); - assertThat(toolchainC.getUnfilteredCompilerOptions(NO_FEATURES)).isEmpty(); + assertThat(ccProviderC.getUnfilteredCompilerOptions(NO_FEATURES)).isEmpty(); assertEquals(Collections.EMPTY_LIST, toolchainC.getDynamicLinkOptions(NO_FEATURES, true)); assertEquals( Collections.EMPTY_LIST, @@ -661,8 +680,8 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { .put("STACK_FRAME_UNLIMITED", "") .build() .entrySet()); - assertThat(toolchainC.getBuiltInIncludeDirectories()).isEmpty(); - assertNull(toolchainC.getSysroot()); + assertThat(ccProviderC.getBuiltInIncludeDirectories()).isEmpty(); + assertNull(ccProviderC.getSysroot()); } protected PathFragment getToolPath(String path) throws LabelSyntaxException { diff --git a/src/test/shell/integration/discard_graph_edges_test.sh b/src/test/shell/integration/discard_graph_edges_test.sh index 3da36d66a9..9650df0a4a 100755 --- a/src/test/shell/integration/discard_graph_edges_test.sh +++ b/src/test/shell/integration/discard_graph_edges_test.sh @@ -170,7 +170,7 @@ function test_packages_cleared() { local histo_file="$(prepare_histogram "--nodiscard_analysis_cache")" local package_count="$(extract_histogram_count "$histo_file" \ 'devtools\.build\.lib\..*\.Package$')" - [[ "$package_count" -ge 10 ]] \ + [[ "$package_count" -ge 9 ]] \ || fail "package count $package_count too low: did you move/rename the class?" local glob_count="$(extract_histogram_count "$histo_file" "GlobValue")" [[ "$glob_count" -ge 8 ]] \ |