diff options
Diffstat (limited to 'src')
14 files changed, 116 insertions, 350 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 49c88e280a..ac2c429fd6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -226,7 +226,6 @@ public final class CcCompilationHelper { private final FeatureConfiguration featureConfiguration; private final CcToolchainProvider ccToolchain; private final FdoSupportProvider fdoSupport; - private final ImmutableSet<String> features; private boolean useDeps = true; private boolean generateModuleMap = true; private String purpose = null; @@ -291,7 +290,6 @@ public final class CcCompilationHelper { this.configuration = Preconditions.checkNotNull(configuration); this.cppConfiguration = Preconditions.checkNotNull(ruleContext.getFragment(CppConfiguration.class)); - this.features = ruleContext.getFeatures(); setGenerateNoPicAction( !CppHelper.usePicForDynamicLibraries(ruleContext, ccToolchain) || !CppHelper.usePicForBinaries(ruleContext, ccToolchain)); @@ -1526,8 +1524,7 @@ public final class CcCompilationHelper { CppHelper.getFdoBuildStamp(ruleContext, fdoSupport.getFdoSupport()), dotdFileExecPath, ImmutableList.copyOf(variablesExtensions), - allAdditionalBuildVariables.build(), - features); + allAdditionalBuildVariables.build()); } /** 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 fd7b607c51..e5b853ba6a 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 @@ -38,7 +38,6 @@ import com.google.devtools.build.lib.skylarkinterface.SkylarkModule; import com.google.devtools.build.lib.util.Pair; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain.OptionalFlag; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import java.util.Map; import javax.annotation.Nullable; @@ -623,14 +622,15 @@ public final class CcToolchainProvider extends ToolchainInfo { name = "unfiltered_compiler_options", doc = "Returns the default list of options which cannot be filtered by BUILD " - + "rules. These should be appended to the command line after filtering." - ) - public ImmutableList<String> getUnfilteredCompilerOptionsWithSysroot(Iterable<String> features) { - return toolchainInfo.getUnfilteredCompilerOptions(features, sysroot); + + "rules. These should be appended to the command line after filtering.") + // TODO(b/24373706): Remove this method once new C++ toolchain API is available + public ImmutableList<String> getUnfilteredCompilerOptionsWithSysroot( + Iterable<String> featuresNotUsedAnymore) { + return toolchainInfo.getUnfilteredCompilerOptions(sysroot); } - public ImmutableList<String> getUnfilteredCompilerOptions(Iterable<String> features) { - return toolchainInfo.getUnfilteredCompilerOptions(features, /* sysroot= */ null); + public ImmutableList<String> getUnfilteredCompilerOptions() { + return toolchainInfo.getUnfilteredCompilerOptions(/* sysroot= */ null); } /** @@ -693,8 +693,8 @@ public final class CcToolchainProvider extends ToolchainInfo { * Returns link options for the specified flag list, combined with universal options for all * shared libraries (regardless of link staticness). */ - ImmutableList<String> getSharedLibraryLinkOptions(FlagList flags, Iterable<String> features) { - return toolchainInfo.getSharedLibraryLinkOptions(flags, features); + ImmutableList<String> getSharedLibraryLinkOptions(FlagList flags) { + return toolchainInfo.getSharedLibraryLinkOptions(flags); } /** Returns compiler flags arising from the {@link CToolchain}. */ @@ -735,16 +735,10 @@ public final class CcToolchainProvider extends ToolchainInfo { return toolchainInfo.getLipoCxxFlags(); } - /** Returns optional compiler flags arising from the {@link CToolchain}. */ - ImmutableList<OptionalFlag> getOptionalCompilerFlags() { - return toolchainInfo.getOptionalCompilerFlags(); - } - /** Returns linker flags for fully statically linked outputs. */ FlagList getFullyStaticLinkFlags(CompilationMode compilationMode, LipoMode lipoMode) { return new FlagList( configureLinkerOptions(compilationMode, lipoMode, LinkingMode.FULLY_STATIC), - ImmutableList.of(), ImmutableList.<String>of()); } @@ -752,7 +746,6 @@ public final class CcToolchainProvider extends ToolchainInfo { FlagList getMostlyStaticLinkFlags(CompilationMode compilationMode, LipoMode lipoMode) { return new FlagList( configureLinkerOptions(compilationMode, lipoMode, LinkingMode.MOSTLY_STATIC), - ImmutableList.of(), ImmutableList.<String>of()); } @@ -760,7 +753,6 @@ public final class CcToolchainProvider extends ToolchainInfo { FlagList getMostlyStaticSharedLinkFlags(CompilationMode compilationMode, LipoMode lipoMode) { return new FlagList( configureLinkerOptions(compilationMode, lipoMode, LinkingMode.MOSTLY_STATIC_LIBRARIES), - ImmutableList.of(), ImmutableList.<String>of()); } @@ -768,7 +760,6 @@ public final class CcToolchainProvider extends ToolchainInfo { FlagList getDynamicLinkFlags(CompilationMode compilationMode, LipoMode lipoMode) { return new FlagList( configureLinkerOptions(compilationMode, lipoMode, LinkingMode.DYNAMIC), - ImmutableList.of(), ImmutableList.of()); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java index 155b070dcc..eeb6da6454 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CompileBuildVariables.java @@ -120,9 +120,7 @@ public enum CompileBuildVariables { String fdoStamp, String dotdFileExecPath, ImmutableList<VariablesExtension> variablesExtensions, - ImmutableMap<String, String> additionalBuildVariables, - // TODO(b/76449614): Remove use of optional_*_flag from CROSSTOOL and get rid of this param - ImmutableSet<String> features) { + ImmutableMap<String, String> additionalBuildVariables) { Variables.Builder buildVariables = new Variables.Builder(ccToolchainProvider.getBuildVariables()); @@ -135,16 +133,13 @@ public enum CompileBuildVariables { buildVariables.addLazyStringSequenceVariable( LEGACY_COMPILE_FLAGS.getVariableName(), getLegacyCompileFlagsSupplier( - ruleContext.getFragment(CppConfiguration.class), - ccToolchainProvider, - sourceFilename, - features)); + ruleContext.getFragment(CppConfiguration.class), ccToolchainProvider, sourceFilename)); if (!CppFileTypes.OBJC_SOURCE.matches(sourceFilename) && !CppFileTypes.OBJCPP_SOURCE.matches(sourceFilename)) { buildVariables.addLazyStringSequenceVariable( UNFILTERED_COMPILE_FLAGS.getVariableName(), - getUnfilteredCompileFlagsSupplier(ccToolchainProvider, features)); + getUnfilteredCompileFlagsSupplier(ccToolchainProvider)); } // TODO(b/76195763): Remove once blaze with cl/189769259 is released and crosstools are updated. @@ -261,20 +256,15 @@ public enum CompileBuildVariables { * to arguments (to prevent accidental capture of enclosing instance which could regress memory). */ private static Supplier<ImmutableList<String>> getLegacyCompileFlagsSupplier( - CppConfiguration cppConfiguration, - CcToolchainProvider toolchain, - String sourceFilename, - ImmutableSet<String> features) { + CppConfiguration cppConfiguration, CcToolchainProvider toolchain, String sourceFilename) { return () -> { ImmutableList.Builder<String> legacyCompileFlags = ImmutableList.builder(); - legacyCompileFlags.addAll( - CppHelper.getCrosstoolCompilerOptions(cppConfiguration, toolchain, features)); + legacyCompileFlags.addAll(CppHelper.getCrosstoolCompilerOptions(cppConfiguration, toolchain)); if (CppFileTypes.CPP_SOURCE.matches(sourceFilename) || CppFileTypes.CPP_HEADER.matches(sourceFilename) || CppFileTypes.CPP_MODULE_MAP.matches(sourceFilename) || CppFileTypes.CLIF_INPUT_PROTO.matches(sourceFilename)) { - legacyCompileFlags.addAll( - CppHelper.getCrosstoolCxxOptions(cppConfiguration, toolchain, features)); + legacyCompileFlags.addAll(CppHelper.getCrosstoolCxxOptions(cppConfiguration, toolchain)); } return legacyCompileFlags.build(); }; @@ -287,8 +277,8 @@ public enum CompileBuildVariables { * to arguments (to prevent accidental capture of enclosing instance which could regress memory). */ private static Supplier<ImmutableList<String>> getUnfilteredCompileFlagsSupplier( - CcToolchainProvider ccToolchain, ImmutableSet<String> features) { - return () -> ccToolchain.getUnfilteredCompilerOptions(features); + CcToolchainProvider ccToolchain) { + return () -> ccToolchain.getUnfilteredCompilerOptions(); } public String getVariableName() { 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 5806ac5d8a..c21f8056fb 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 @@ -277,36 +277,29 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { params.sysrootLabel, new FlagList( coptsBuilder.build(), - FlagList.convertOptionalOptions(toolchain.getOptionalCompilerFlagList()), ImmutableList.copyOf(cppOptions.coptList)), new FlagList( cxxOptsBuilder.build(), - ImmutableList.of(), ImmutableList.copyOf(cppOptions.cxxoptList)), new FlagList( ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()), - ImmutableList.of(), ImmutableList.of()), ImmutableList.copyOf(cppOptions.conlyoptList), new FlagList( cppToolchainInfo.configureLinkerOptions( compilationMode, cppOptions.getLipoMode(), LinkingMode.FULLY_STATIC), - ImmutableList.of(), ImmutableList.of()), new FlagList( cppToolchainInfo.configureLinkerOptions( compilationMode, cppOptions.getLipoMode(), LinkingMode.MOSTLY_STATIC), - ImmutableList.of(), ImmutableList.of()), new FlagList( cppToolchainInfo.configureLinkerOptions( compilationMode, cppOptions.getLipoMode(), LinkingMode.MOSTLY_STATIC_LIBRARIES), - ImmutableList.of(), ImmutableList.of()), new FlagList( cppToolchainInfo.configureLinkerOptions( compilationMode, cppOptions.getLipoMode(), LinkingMode.DYNAMIC), - ImmutableList.of(), ImmutableList.of()), ImmutableList.copyOf(cppOptions.coptList), ImmutableList.copyOf(cppOptions.cxxoptList), @@ -593,19 +586,22 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { + "in addition to the ones returned by this method" ) @Deprecated - public ImmutableList<String> getCompilerOptions(Iterable<String> features) { - return compilerFlags.evaluate(features); + public ImmutableList<String> getCompilerOptions(Iterable<String> featuresNotUsedAnymore) { + return compilerFlags.evaluate(); } /** - * Returns the list of additional C-specific options to use for compiling - * C. These should be go on the command line after the common options - * returned by {@link #getCompilerOptions}. + * Returns the list of additional C-specific options to use for compiling C. These should be go on + * the command line after the common options returned by {@link #getCompilerOptions}. */ - @SkylarkCallable(name = "c_options", structField = true, - doc = "Returns the list of additional C-specific options to use for compiling C. " - + "These should be go on the command line after the common options returned by " - + "<code>compiler_options</code>") + // TODO(b/64384912): Migrate skylark callers and remove. + @SkylarkCallable( + name = "c_options", + structField = true, + doc = + "Returns the list of additional C-specific options to use for compiling C. " + + "These should be go on the command line after the common options returned by " + + "<code>compiler_options</code>") public ImmutableList<String> getCOptions() { return cOptions; } @@ -625,17 +621,17 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { + "<code>compiler_options</code>" ) @Deprecated - public ImmutableList<String> getCxxOptions(Iterable<String> features) { - return cxxFlags.evaluate(features); + public ImmutableList<String> getCxxOptions(Iterable<String> featuresNotUsedAnymore) { + return cxxFlags.evaluate(); } /** * Returns the default list of options which cannot be filtered by BUILD rules. These should be * appended to the command line after filtering. * - * @deprecated since it uses nonconfigured sysroot. Use - * {@link CcToolchainProvider#getUnfilteredCompilerOptionsWithSysroot(Iterable)} if you *really* - * need to. + * @deprecated since it uses nonconfigured sysroot. Use {@link + * CcToolchainProvider#getUnfilteredCompilerOptionsWithSysroot(Iterable)} if you *really* need + * to. */ // TODO(b/65401585): Migrate existing uses to cc_toolchain and cleanup here. @Deprecated @@ -646,25 +642,23 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { + "rules. These should be appended to the command line after filtering." ) public ImmutableList<String> getUnfilteredCompilerOptionsWithLegacySysroot( - Iterable<String> features) { - return getUnfilteredCompilerOptionsDoNotUse(features, nonConfiguredSysroot); + Iterable<String> featuresNotUsedAnymore) { + return getUnfilteredCompilerOptionsDoNotUse(nonConfiguredSysroot); } /** - * @deprecated since it hardcodes --sysroot flag. Use - * {@link com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration} - * instead. + * @deprecated since it hardcodes --sysroot flag. Use {@link + * com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration} instead. */ // TODO(b/65401585): Migrate existing uses to cc_toolchain and cleanup here. @Deprecated - ImmutableList<String> getUnfilteredCompilerOptionsDoNotUse( - Iterable<String> features, @Nullable PathFragment sysroot) { + ImmutableList<String> getUnfilteredCompilerOptionsDoNotUse(@Nullable PathFragment sysroot) { if (sysroot == null) { - return unfilteredCompilerFlags.evaluate(features); + return unfilteredCompilerFlags.evaluate(); } return ImmutableList.<String>builder() .add("--sysroot=" + sysroot) - .addAll(unfilteredCompilerFlags.evaluate(features)) + .addAll(unfilteredCompilerFlags.evaluate()) .build(); } @@ -732,10 +726,10 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { * Returns the immutable list of linker options for fully statically linked outputs. Does not * include command-line options passed via --linkopt or --linkopts. * - * @param features default settings affecting this link + * @param featuresNotUsedAnymore * @param sharedLib true if the output is a shared lib, false if it's an executable * <p>Deprecated: Use {@link CppHelper#getFullyStaticLinkOptions(CppConfiguration, - * CcToolchainProvider, Iterable, Boolean)} + * CcToolchainProvider, Boolean)} */ // TODO(b/64384912): Migrate skylark users to cc_common and remove. @SkylarkCallable( @@ -747,11 +741,11 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { ) @Deprecated public ImmutableList<String> getFullyStaticLinkOptions( - Iterable<String> features, Boolean sharedLib) { + Iterable<String> featuresNotUsedAnymore, Boolean sharedLib) { if (sharedLib) { - return getSharedLibraryLinkOptions(mostlyStaticLinkFlags, features); + return getSharedLibraryLinkOptions(mostlyStaticLinkFlags); } else { - return fullyStaticLinkFlags.evaluate(features); + return fullyStaticLinkFlags.evaluate(); } } @@ -759,10 +753,10 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { * Returns the immutable list of linker options for mostly statically linked outputs. Does not * include command-line options passed via --linkopt or --linkopts. * - * @param features default settings affecting this link + * @param featuresNotUsedAnymore * @param sharedLib true if the output is a shared lib, false if it's an executable * <p>Deprecated: Use {@link CppHelper#getMostlyStaticLinkOptions(CppConfiguration, - * CcToolchainProvider, Iterable, Boolean)} + * CcToolchainProvider, Boolean)} */ // TODO(b/64384912): Migrate skylark users to cc_common and remove. @SkylarkCallable( @@ -774,15 +768,14 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { ) @Deprecated public ImmutableList<String> getMostlyStaticLinkOptions( - Iterable<String> features, Boolean sharedLib) { + Iterable<String> featuresNotUsedAnymore, Boolean sharedLib) { if (sharedLib) { return getSharedLibraryLinkOptions( cppToolchainInfo.supportsEmbeddedRuntimes() ? mostlyStaticSharedLinkFlags - : dynamicLinkFlags, - features); + : dynamicLinkFlags); } else { - return mostlyStaticLinkFlags.evaluate(features); + return mostlyStaticLinkFlags.evaluate(); } } @@ -790,10 +783,10 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { * Returns the immutable list of linker options for artifacts that are not fully or mostly * statically linked. Does not include command-line options passed via --linkopt or --linkopts. * - * @param features default settings affecting this link + * @param featuresNotUsedAnymore * @param sharedLib true if the output is a shared lib, false if it's an executable * <p>Deprecated: Use {@link CppHelper#getDynamicLinkOptions(CppConfiguration, - * CcToolchainProvider, Iterable, Boolean)} + * CcToolchainProvider, Boolean)} */ // TODO(b/64384912): Migrate skylark users to cc_common and remove. @SkylarkCallable( @@ -804,11 +797,12 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { + "passed via --linkopt or --linkopts." ) @Deprecated - public ImmutableList<String> getDynamicLinkOptions(Iterable<String> features, Boolean sharedLib) { + public ImmutableList<String> getDynamicLinkOptions( + Iterable<String> featuresNotUsedAnymore, Boolean sharedLib) { if (sharedLib) { - return getSharedLibraryLinkOptions(dynamicLinkFlags, features); + return getSharedLibraryLinkOptions(dynamicLinkFlags); } else { - return dynamicLinkFlags.evaluate(features); + return dynamicLinkFlags.evaluate(); } } @@ -819,9 +813,8 @@ public final class CppConfiguration extends BuildConfiguration.Fragment { * <p>Deprecated: Use {@link CcToolchainProvider#getSharedLibraryLinkOptions} */ // TODO(b/64384912): Migrate skylark dependants and delete. - private ImmutableList<String> getSharedLibraryLinkOptions( - FlagList flags, Iterable<String> features) { - return cppToolchainInfo.getSharedLibraryLinkOptions(flags, features); + private ImmutableList<String> getSharedLibraryLinkOptions(FlagList flags) { + return cppToolchainInfo.getSharedLibraryLinkOptions(flags); } /** 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 d793a52833..a146b951b1 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 @@ -254,7 +254,7 @@ public class CppHelper { */ // TODO(b/70784100): Figure out if these methods can be moved to CcToolchainProvider. public static ImmutableList<String> getCrosstoolCompilerOptions( - CppConfiguration config, CcToolchainProvider toolchain, Iterable<String> features) { + CppConfiguration config, CcToolchainProvider toolchain) { ImmutableList.Builder<String> coptsBuilder = ImmutableList.<String>builder() .addAll(toolchain.getToolchainCompilerFlags()) @@ -270,10 +270,9 @@ public class CppHelper { FlagList compilerFlags = new FlagList( coptsBuilder.build(), - FlagList.convertOptionalOptions(toolchain.getOptionalCompilerFlags()), ImmutableList.of()); - return compilerFlags.evaluate(features); + return compilerFlags.evaluate(); } /** @@ -282,9 +281,9 @@ public class CppHelper { * C++-specific options that should be used, in addition to the ones returned by this method. */ public static ImmutableList<String> getCompilerOptions( - CppConfiguration config, CcToolchainProvider toolchain, Iterable<String> features) { + CppConfiguration config, CcToolchainProvider toolchain) { return ImmutableList.<String>builder() - .addAll(getCrosstoolCompilerOptions(config, toolchain, features)) + .addAll(getCrosstoolCompilerOptions(config, toolchain)) .addAll(config.getCopts()) .build(); } @@ -295,17 +294,16 @@ public class CppHelper { * returned by {@link #getCompilerOptions}. */ public static ImmutableList<String> getCrosstoolCxxOptions( - CppConfiguration config, CcToolchainProvider toolchain, Iterable<String> features) { + CppConfiguration config, CcToolchainProvider toolchain) { ImmutableList.Builder<String> cxxOptsBuilder = ImmutableList.<String>builder() .addAll(toolchain.getToolchainCxxFlags()) .addAll(toolchain.getCxxFlagsByCompilationMode().get(config.getCompilationMode())) .addAll(toolchain.getLipoCxxFlags().get(config.getLipoMode())); - FlagList cxxFlags = - new FlagList(cxxOptsBuilder.build(), ImmutableList.of(), ImmutableList.of()); + FlagList cxxFlags = new FlagList(cxxOptsBuilder.build(), ImmutableList.of()); - return cxxFlags.evaluate(features); + return cxxFlags.evaluate(); } /** @@ -313,9 +311,9 @@ public class CppHelper { * go on the command line after the common options returned by {@link #getCompilerOptions}. */ public static ImmutableList<String> getCxxOptions( - CppConfiguration config, CcToolchainProvider toolchain, Iterable<String> features) { + CppConfiguration config, CcToolchainProvider toolchain) { return ImmutableList.<String>builder() - .addAll(getCrosstoolCxxOptions(config, toolchain, features)) + .addAll(getCrosstoolCxxOptions(config, toolchain)) .addAll(config.getCxxopts()) .build(); } @@ -326,22 +324,19 @@ public class CppHelper { * * @param config the CppConfiguration for this build * @param toolchain the c++ toolchain - * @param features default settings affecting this link * @param sharedLib true if the output is a shared lib, false if it's an executable */ public static ImmutableList<String> getFullyStaticLinkOptions( CppConfiguration config, CcToolchainProvider toolchain, - Iterable<String> features, Boolean sharedLib) { if (sharedLib) { return toolchain.getSharedLibraryLinkOptions( - toolchain.getMostlyStaticLinkFlags(config.getCompilationMode(), config.getLipoMode()), - features); + toolchain.getMostlyStaticLinkFlags(config.getCompilationMode(), config.getLipoMode())); } else { return toolchain .getFullyStaticLinkFlags(config.getCompilationMode(), config.getLipoMode()) - .evaluate(features); + .evaluate(); } } @@ -351,25 +346,22 @@ public class CppHelper { * * @param config the CppConfiguration for this build * @param toolchain the c++ toolchain - * @param features default settings affecting this link * @param sharedLib true if the output is a shared lib, false if it's an executable */ public static ImmutableList<String> getMostlyStaticLinkOptions( CppConfiguration config, CcToolchainProvider toolchain, - Iterable<String> features, Boolean sharedLib) { if (sharedLib) { return toolchain.getSharedLibraryLinkOptions( toolchain.supportsEmbeddedRuntimes() ? toolchain.getMostlyStaticSharedLinkFlags( config.getCompilationMode(), config.getLipoMode()) - : toolchain.getDynamicLinkFlags(config.getCompilationMode(), config.getLipoMode()), - features); + : toolchain.getDynamicLinkFlags(config.getCompilationMode(), config.getLipoMode())); } else { return toolchain .getMostlyStaticLinkFlags(config.getCompilationMode(), config.getLipoMode()) - .evaluate(features); + .evaluate(); } } @@ -379,22 +371,19 @@ public class CppHelper { * * @param config the CppConfiguration for this build * @param toolchain the c++ toolchain - * @param features default settings affecting this link * @param sharedLib true if the output is a shared lib, false if it's an executable */ public static ImmutableList<String> getDynamicLinkOptions( CppConfiguration config, CcToolchainProvider toolchain, - Iterable<String> features, Boolean sharedLib) { if (sharedLib) { return toolchain.getSharedLibraryLinkOptions( - toolchain.getDynamicLinkFlags(config.getCompilationMode(), config.getLipoMode()), - features); + toolchain.getDynamicLinkFlags(config.getCompilationMode(), config.getLipoMode())); } else { return toolchain .getDynamicLinkFlags(config.getCompilationMode(), config.getLipoMode()) - .evaluate(features); + .evaluate(); } } 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 3560f6b2d8..756b6a987b 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 @@ -520,10 +520,10 @@ public class CppLinkActionBuilder { .collect(ImmutableList.toImmutableList()); } - private List<String> getLtoBackendCommandLineOptions(ImmutableSet<String> features) { + private List<String> getLtoBackendCommandLineOptions() { List<String> argv = new ArrayList<>(); argv.addAll(toolchain.getLinkOptions()); - argv.addAll(CppHelper.getCompilerOptions(cppConfiguration, toolchain, features)); + argv.addAll(CppHelper.getCompilerOptions(cppConfiguration, toolchain)); argv.addAll(cppConfiguration.getLtoBackendOptions()); return argv; } @@ -532,8 +532,7 @@ public class CppLinkActionBuilder { PathFragment ltoOutputRootPrefix, NestedSet<LibraryToLink> uniqueLibraries, boolean allowLtoIndexing, - boolean includeLinkStaticInLtoIndexing, - ImmutableSet<String> features) { + boolean includeLinkStaticInLtoIndexing) { Set<Artifact> compiled = new LinkedHashSet<>(); for (LibraryToLink lib : uniqueLibraries) { compiled.addAll(lib.getLtoBitcodeFiles().keySet()); @@ -563,7 +562,7 @@ public class CppLinkActionBuilder { } } - List<String> argv = getLtoBackendCommandLineOptions(features); + List<String> argv = getLtoBackendCommandLineOptions(); ImmutableList.Builder<LtoBackendArtifacts> ltoOutputs = ImmutableList.builder(); for (LibraryToLink lib : uniqueLibraries) { if (!lib.containsObjectFiles()) { @@ -614,7 +613,7 @@ public class CppLinkActionBuilder { } private ImmutableMap<Artifact, LtoBackendArtifacts> createSharedNonLtoArtifacts( - ImmutableSet<String> features, boolean isLtoIndexing) { + boolean isLtoIndexing) { // Only create the shared LTO artifacts for a statically linked library that has bitcode files. if (ltoBitcodeFiles == null || isLtoIndexing || linkType.staticness() != Staticness.STATIC) { return ImmutableMap.<Artifact, LtoBackendArtifacts>of(); @@ -622,7 +621,7 @@ public class CppLinkActionBuilder { PathFragment ltoOutputRootPrefix = PathFragment.create(SHARED_NONLTO_BACKEND_ROOT_PREFIX); - List<String> argv = getLtoBackendCommandLineOptions(features); + List<String> argv = getLtoBackendCommandLineOptions(); ImmutableMap.Builder<Artifact, LtoBackendArtifacts> sharedNonLtoBackends = ImmutableMap.builder(); @@ -676,8 +675,7 @@ public class CppLinkActionBuilder { } } - private ImmutableList<String> getToolchainFlags( - ImmutableSet<String> features, List<String> linkopts) { + private ImmutableList<String> getToolchainFlags(List<String> linkopts) { if (Staticness.STATIC.equals(linkType.staticness())) { return ImmutableList.of(); } @@ -706,15 +704,12 @@ public class CppLinkActionBuilder { // Extra toolchain link options based on the output's link staticness. if (fullyStatic) { result.addAll( - CppHelper.getFullyStaticLinkOptions( - cppConfiguration, toolchain, features, sharedLinkopts)); + CppHelper.getFullyStaticLinkOptions(cppConfiguration, toolchain, sharedLinkopts)); } else if (mostlyStatic) { result.addAll( - CppHelper.getMostlyStaticLinkOptions( - cppConfiguration, toolchain, features, sharedLinkopts)); + CppHelper.getMostlyStaticLinkOptions(cppConfiguration, toolchain, sharedLinkopts)); } else { - result.addAll( - CppHelper.getDynamicLinkOptions(cppConfiguration, toolchain, features, sharedLinkopts)); + result.addAll(CppHelper.getDynamicLinkOptions(cppConfiguration, toolchain, sharedLinkopts)); } // Extra test-specific link options. @@ -782,10 +777,6 @@ public class CppLinkActionBuilder { includeLinkStaticInLtoIndexing || (linkStaticness == LinkStaticness.DYNAMIC && !ltoBitcodeFiles.isEmpty()); - // ruleContext can only be null during testing. This is kind of ugly. - final ImmutableSet<String> features = - (ruleContext == null) ? ImmutableSet.of() : ruleContext.getFeatures(); - NestedSet<LibraryToLink> originalUniqueLibraries = libraries.build(); PathFragment ltoOutputRootPrefix = null; @@ -804,8 +795,7 @@ public class CppLinkActionBuilder { ltoOutputRootPrefix, originalUniqueLibraries, allowLtoIndexing, - includeLinkStaticInLtoIndexing, - features); + includeLinkStaticInLtoIndexing); if (!allowLtoIndexing) { return null; @@ -865,7 +855,7 @@ public class CppLinkActionBuilder { libraryIdentifier, combinedObjectArtifacts, ltoBitcodeFiles, - createSharedNonLtoArtifacts(features, isLtoIndexing)); + createSharedNonLtoArtifacts(isLtoIndexing)); final LibraryToLink interfaceOutputLibrary = (interfaceOutput == null) ? null @@ -1071,7 +1061,7 @@ public class CppLinkActionBuilder { new Variables.Builder(buildVariables) .addStringSequenceVariable( LinkBuildVariables.LEGACY_LINK_FLAGS.getVariableName(), - getToolchainFlags(features, linkoptsForVariables)) + getToolchainFlags(linkoptsForVariables)) .build(); linkCommandLineBuilder.setBuildVariables(patchedVariables); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java index f059510d91..dbaa75e85c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppLinkstampCompileHelper.java @@ -177,8 +177,6 @@ public class CppLinkstampCompileHelper { fdoBuildStamp, /* dotdFileExecPath= */ null, /* variablesExtensions= */ ImmutableList.of(), - /* additionalBuildVariables= */ ImmutableMap.of(), - // TODO(b/76449614): Remove use of optional_*_flag from CROSSTOOL and get rid of this param - ruleContext.getFeatures()); + /* additionalBuildVariables= */ ImmutableMap.of()); } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java index 2b5e54c9da..0814552932 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppToolchainInfo.java @@ -33,7 +33,6 @@ import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain.ArtifactNamePattern; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain.OptionalFlag; 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; @@ -95,7 +94,6 @@ public final class CppToolchainInfo { private final ImmutableList<String> crosstoolCompilerFlags; private final ImmutableList<String> crosstoolCxxFlags; - private final ImmutableList<OptionalFlag> crosstoolOptionalCompilerFlags; private final ImmutableListMultimap<CompilationMode, String> cFlagsByCompilationMode; private final ImmutableListMultimap<CompilationMode, String> cxxFlagsByCompilationMode; @@ -183,7 +181,6 @@ public final class CppToolchainInfo { toolchain.getHostSystemName(), new FlagList( ImmutableList.copyOf(toolchain.getDynamicLibraryLinkerFlagList()), - ImmutableList.of(), ImmutableList.of()), ImmutableList.copyOf(toolchain.getLinkerFlagList()), linkOptionsFromLinkingModeBuilder.build(), @@ -206,14 +203,12 @@ public final class CppToolchainInfo { computeAdditionalMakeVariables(toolchain), ImmutableList.copyOf(toolchain.getCompilerFlagList()), ImmutableList.copyOf(toolchain.getCxxFlagList()), - ImmutableList.copyOf(toolchain.getOptionalCompilerFlagList()), cFlagsBuilder.build(), cxxFlagsBuilder.build(), lipoCFlagsBuilder.build(), lipoCxxFlagsBuilder.build(), new FlagList( ImmutableList.copyOf(toolchain.getUnfilteredCxxFlagList()), - ImmutableList.of(), ImmutableList.of()), toolchain.getSupportsFission(), toolchain.getSupportsStartEndLib(), @@ -260,7 +255,6 @@ public final class CppToolchainInfo { ImmutableMap<String, String> additionalMakeVariables, ImmutableList<String> crosstoolCompilerFlags, ImmutableList<String> crosstoolCxxFlags, - ImmutableList<OptionalFlag> crosstoolOptionalCompilerFlags, ImmutableListMultimap<CompilationMode, String> cFlagsByCompilationMode, ImmutableListMultimap<CompilationMode, String> cxxFlagsByCompilationMode, ImmutableListMultimap<LipoMode, String> lipoCFlags, @@ -305,7 +299,6 @@ public final class CppToolchainInfo { this.additionalMakeVariables = additionalMakeVariables; this.crosstoolCompilerFlags = crosstoolCompilerFlags; this.crosstoolCxxFlags = crosstoolCxxFlags; - this.crosstoolOptionalCompilerFlags = crosstoolOptionalCompilerFlags; this.cFlagsByCompilationMode = cFlagsByCompilationMode; this.cxxFlagsByCompilationMode = cxxFlagsByCompilationMode; this.lipoCFlags = lipoCFlags; @@ -628,10 +621,10 @@ public final class CppToolchainInfo { * Returns link options for the specified flag list, combined with universal options for all * shared libraries (regardless of link staticness). */ - ImmutableList<String> getSharedLibraryLinkOptions(FlagList flags, Iterable<String> features) { + ImmutableList<String> getSharedLibraryLinkOptions(FlagList flags) { return ImmutableList.<String>builder() - .addAll(flags.evaluate(features)) - .addAll(dynamicLibraryLinkFlags.evaluate(features)) + .addAll(flags.evaluate()) + .addAll(dynamicLibraryLinkFlags.evaluate()) .build(); } @@ -738,22 +731,14 @@ public final class CppToolchainInfo { return lipoCxxFlags; } - /** Returns optional compiler flags from this toolchain. */ - @Deprecated - // TODO(b/76449614): Remove all traces of optional flag crosstool fields when g3 is migrated. - public ImmutableList<OptionalFlag> getOptionalCompilerFlags() { - return crosstoolOptionalCompilerFlags; - } - /** Returns unfiltered compiler options for C++ from this toolchain. */ - public ImmutableList<String> getUnfilteredCompilerOptions( - Iterable<String> features, @Nullable PathFragment sysroot) { + public ImmutableList<String> getUnfilteredCompilerOptions(@Nullable PathFragment sysroot) { if (sysroot == null) { - return unfilteredCompilerFlags.evaluate(features); + return unfilteredCompilerFlags.evaluate(); } return ImmutableList.<String>builder() .add("--sysroot=" + sysroot) - .addAll(unfilteredCompilerFlags.evaluate(features)) + .addAll(unfilteredCompilerFlags.evaluate()) .build(); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigProtoCodecRegisterer.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigProtoCodecRegisterer.java index 1e5c2fcbb5..5d7a0b04b2 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigProtoCodecRegisterer.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CrosstoolConfigProtoCodecRegisterer.java @@ -25,7 +25,6 @@ class CrosstoolConfigProtoCodecRegisterer implements CodecRegisterer<MessageLite return ImmutableList.of( new MessageLiteCodec(CrosstoolConfig.CrosstoolRelease::newBuilder), new MessageLiteCodec(CrosstoolConfig.CToolchain::newBuilder), - new MessageLiteCodec(CrosstoolConfig.CToolchain.OptionalFlag::newBuilder), new MessageLiteCodec(CrosstoolConfig.CToolchain.FlagGroup::newBuilder), new MessageLiteCodec(CrosstoolConfig.CToolchain.VariableWithValue::newBuilder), new MessageLiteCodec(CrosstoolConfig.CToolchain.EnvEntry::newBuilder), diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/FlagList.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/FlagList.java index 976914135c..7fbdc308fe 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/FlagList.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/FlagList.java @@ -16,78 +16,29 @@ package com.google.devtools.build.lib.rules.cpp; import com.google.common.annotations.VisibleForTesting; import com.google.common.collect.ImmutableList; -import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable; import com.google.devtools.build.lib.skyframe.serialization.autocodec.AutoCodec; -import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.CToolchain; import java.io.Serializable; -import java.util.List; /** Represents a list of c++ tool flags. */ @AutoCodec @Immutable public class FlagList implements Serializable { - /** Represents an optional flag that can be toggled using the package features mechanism. */ - @AutoCodec - @Immutable - static class OptionalFlag implements Serializable { - private final String name; - private final ImmutableList<String> flags; - - @AutoCodec.Instantiator - OptionalFlag(String name, ImmutableList<String> flags) { - this.name = name; - this.flags = flags; - } - - private ImmutableList<String> getFlags() { - return flags; - } - - private String getName() { - return name; - } - } - private final ImmutableList<String> prefixFlags; - private final ImmutableList<OptionalFlag> optionalFlags; private final ImmutableList<String> suffixFlags; @AutoCodec.Instantiator FlagList( ImmutableList<String> prefixFlags, - ImmutableList<OptionalFlag> optionalFlags, ImmutableList<String> suffixFlags) { this.prefixFlags = prefixFlags; - this.optionalFlags = optionalFlags; this.suffixFlags = suffixFlags; } - static ImmutableList<OptionalFlag> convertOptionalOptions( - List<CToolchain.OptionalFlag> optionalFlagList) { - ImmutableList.Builder<OptionalFlag> result = ImmutableList.builder(); - - for (CToolchain.OptionalFlag crosstoolOptionalFlag : optionalFlagList) { - String name = crosstoolOptionalFlag.getDefaultSettingName(); - result.add(new OptionalFlag(name, ImmutableList.copyOf(crosstoolOptionalFlag.getFlagList()))); - } - - return result.build(); - } - @VisibleForTesting - ImmutableList<String> evaluate(Iterable<String> features) { - ImmutableSet<String> featureSet = ImmutableSet.copyOf(features); + ImmutableList<String> evaluate() { ImmutableList.Builder<String> result = ImmutableList.builder(); result.addAll(prefixFlags); - for (OptionalFlag optionalFlag : optionalFlags) { - // The flag is added if the default is true and the flag is not specified, - // or if the default is false and the flag is specified. - if (featureSet.contains(optionalFlag.getName())) { - result.addAll(optionalFlag.getFlags()); - } - } - result.addAll(suffixFlags); return result.build(); } diff --git a/src/main/protobuf/crosstool_config.proto b/src/main/protobuf/crosstool_config.proto index 9905b20290..7323d75526 100644 --- a/src/main/protobuf/crosstool_config.proto +++ b/src/main/protobuf/crosstool_config.proto @@ -40,13 +40,6 @@ package com.google.devtools.build.lib.view.config.crosstool; // exectuable. In particular, it is used to check if the compilation products of // a toolchain can run on the host machine. message CToolchain { - // Optional flag. Some option lists have a corresponding optional flag list. - // These are appended at the end of the option list in case the corresponding - // default setting is set. - message OptionalFlag { - required string default_setting_name = 1; - repeated string flag = 2; - } // A group of correlated flags. Supports parametrization via variable // expansion. @@ -380,7 +373,6 @@ message CToolchain { // Compiler flags for C/C++/Asm compilation. repeated string compiler_flag = 13; - repeated OptionalFlag optional_compiler_flag = 35; // Additional compiler flags for C++ compilation. repeated string cxx_flag = 14; // Additional unfiltered compiler flags for C/C++/Asm compilation. @@ -538,11 +530,6 @@ message DefaultCpuToolchain { // cpu, a default toolchain for each supported cpu type, and a set of // toolchains. message CrosstoolRelease { - message DefaultSetting { - required string name = 1; - required bool default_value = 2; - } - // The major and minor version of the crosstool release. required string major_version = 1; required string minor_version = 2; @@ -554,9 +541,6 @@ message CrosstoolRelease { // The default toolchain to use for each given cpu. repeated DefaultCpuToolchain default_toolchain = 4; - // The default settings used in this release. - repeated DefaultSetting default_setting = 6; - // All the toolchains in this release. repeated CToolchain toolchain = 5; } diff --git a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java index 324ff10a86..1762ad5afe 100644 --- a/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java +++ b/src/test/java/com/google/devtools/build/lib/packages/util/MockCcSupport.java @@ -547,38 +547,6 @@ public abstract class MockCcSupport { return TextFormat.printToString(crosstoolBuilder.build()); } - public static String addOptionalDefaultCoptsToCrosstool(String original) - throws TextFormat.ParseException { - CrosstoolConfig.CrosstoolRelease.Builder builder = - CrosstoolConfig.CrosstoolRelease.newBuilder(); - TextFormat.merge(original, builder); - for (CrosstoolConfig.CToolchain.Builder toolchain : builder.getToolchainBuilderList()) { - CrosstoolConfig.CToolchain.OptionalFlag.Builder defaultTrue = - CrosstoolConfig.CToolchain.OptionalFlag.newBuilder(); - defaultTrue.setDefaultSettingName("crosstool_default_true"); - defaultTrue.addFlag("-DDEFAULT_TRUE"); - toolchain.addOptionalCompilerFlag(defaultTrue.build()); - CrosstoolConfig.CToolchain.OptionalFlag.Builder defaultFalse = - CrosstoolConfig.CToolchain.OptionalFlag.newBuilder(); - defaultFalse.setDefaultSettingName("crosstool_default_false"); - defaultFalse.addFlag("-DDEFAULT_FALSE"); - toolchain.addOptionalCompilerFlag(defaultFalse.build()); - } - - CrosstoolConfig.CrosstoolRelease.DefaultSetting.Builder defaultTrue = - CrosstoolConfig.CrosstoolRelease.DefaultSetting.newBuilder(); - defaultTrue.setName("crosstool_default_true"); - defaultTrue.setDefaultValue(true); - builder.addDefaultSetting(defaultTrue.build()); - CrosstoolConfig.CrosstoolRelease.DefaultSetting.Builder defaultFalse = - CrosstoolConfig.CrosstoolRelease.DefaultSetting.newBuilder(); - defaultFalse.setName("crosstool_default_false"); - defaultFalse.setDefaultValue(false); - builder.addDefaultSetting(defaultFalse.build()); - - return TextFormat.printToString(builder.build()); - } - public static String addLibcLabelToCrosstool(String original, String label) throws TextFormat.ParseException { CrosstoolConfig.CrosstoolRelease.Builder builder = diff --git a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java index 02365f10bb..8141951ec8 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/cpp/CcToolchainTest.java @@ -31,7 +31,6 @@ import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig; import com.google.devtools.common.options.OptionsParsingException; import java.io.IOException; import java.util.ArrayList; -import java.util.Collections; import java.util.List; import org.junit.Test; import org.junit.runner.RunWith; @@ -510,59 +509,12 @@ public class CcToolchainTest extends BuildViewTestCase { assertDoesNotContainSublist( CppHelper.getCompilerOptions( - getConfiguration(lib).getFragment(CppConfiguration.class), - toolchain, - Collections.emptyList()), + getConfiguration(lib).getFragment(CppConfiguration.class), toolchain), "--param", "df-double-quote-threshold-factor=0"); } @Test - public void testFeatures() throws Exception { - writeDummyCcToolchain(); - - String originalCrosstool = analysisMock.ccSupport().readCrosstoolFile(); - getAnalysisMock() - .ccSupport() - .setupCrosstoolWithRelease( - mockToolsConfig, MockCcSupport.addOptionalDefaultCoptsToCrosstool(originalCrosstool)); - - scratch.file("lib/BUILD", "cc_library(", " name = 'lib',", " srcs = ['a.cc'],", ")"); - - useConfiguration(); - ConfiguredTarget lib = getConfiguredTarget("//lib"); - CcToolchainProvider toolchain = - CppHelper.getToolchainUsingDefaultCcToolchainAttribute(getRuleContext(lib)); - - String defaultSettingFalse = "crosstool_default_false"; - List<String> copts = - CppHelper.getCompilerOptions( - getConfiguration(lib).getFragment(CppConfiguration.class), - toolchain, - Collections.emptyList()); - assertThat(copts).doesNotContain("-DDEFAULT_FALSE"); - copts = - CppHelper.getCompilerOptions( - getConfiguration(lib).getFragment(CppConfiguration.class), - toolchain, - ImmutableList.of(defaultSettingFalse)); - assertThat(copts).contains("-DDEFAULT_FALSE"); - - useConfiguration("--copt", "-DCOPT"); - lib = getConfiguredTarget("//lib"); - toolchain = CppHelper.getToolchainUsingDefaultCcToolchainAttribute(getRuleContext(lib)); - - copts = - CppHelper.getCompilerOptions( - getConfiguration(lib).getFragment(CppConfiguration.class), - toolchain, - ImmutableList.of(defaultSettingFalse)); - assertThat(copts).contains("-DDEFAULT_FALSE"); - assertThat(copts).contains("-DCOPT"); - assertThat(copts.indexOf("-DDEFAULT_FALSE")).isLessThan(copts.indexOf("-DCOPT")); - } - - @Test public void testMergesDefaultCoptsWithUserProvidedOnes() throws Exception { writeDummyCcToolchain(); scratch.file("lib/BUILD", "cc_library(", " name = 'lib',", " srcs = ['a.cc'],", ")"); @@ -574,9 +526,7 @@ public class CcToolchainTest extends BuildViewTestCase { List<String> expected = new ArrayList<>(); expected.addAll( CppHelper.getCompilerOptions( - getConfiguration(lib).getFragment(CppConfiguration.class), - toolchain, - Collections.emptyList())); + getConfiguration(lib).getFragment(CppConfiguration.class), toolchain)); expected.add("-Dfoo"); useConfiguration("--copt", "-Dfoo"); @@ -585,9 +535,7 @@ public class CcToolchainTest extends BuildViewTestCase { assertThat( ImmutableList.copyOf( CppHelper.getCompilerOptions( - getConfiguration(lib).getFragment(CppConfiguration.class), - toolchain, - Collections.emptyList()))) + getConfiguration(lib).getFragment(CppConfiguration.class), toolchain))) .isEqualTo(ImmutableList.copyOf(expected)); } 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 e45e32fe56..9f6fd5d56a 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 @@ -18,7 +18,6 @@ import static com.google.common.truth.Truth.assertThat; import static org.junit.Assert.fail; import com.google.common.base.Preconditions; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableMap; import com.google.devtools.build.lib.analysis.ConfiguredTarget; import com.google.devtools.build.lib.analysis.TemplateVariableInfo; @@ -37,8 +36,6 @@ import com.google.devtools.build.lib.testutil.TestRuleClassProvider; import com.google.devtools.build.lib.vfs.PathFragment; import com.google.devtools.build.lib.view.config.crosstool.CrosstoolConfig.LipoMode; import java.io.IOException; -import java.util.Collection; -import java.util.Collections; import org.junit.Before; import org.junit.Test; import org.junit.runner.RunWith; @@ -49,7 +46,6 @@ import org.junit.runners.JUnit4; */ @RunWith(JUnit4.class) public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { - private static final Collection<String> NO_FEATURES = Collections.emptySet(); private CppConfiguration create(CppConfigurationLoader loader, String... args) throws Exception { useConfiguration(args); @@ -202,28 +198,26 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { .containsExactly(getToolPath("system-include-dir")); assertThat(ccProvider.getSysroot()).isNull(); - assertThat(CppHelper.getCompilerOptions(toolchain, ccProvider, NO_FEATURES)) + assertThat(CppHelper.getCompilerOptions(toolchain, ccProvider)) .containsExactly("c", "fastbuild") .inOrder(); assertThat(toolchain.getCOptions()).isEmpty(); - assertThat(CppHelper.getCxxOptions(toolchain, ccProvider, NO_FEATURES)) + assertThat(CppHelper.getCxxOptions(toolchain, ccProvider)) .containsExactly("cxx", "cxx-fastbuild") .inOrder(); - assertThat(ccProvider.getUnfilteredCompilerOptions(NO_FEATURES)) - .containsExactly("unfiltered") - .inOrder(); + assertThat(ccProvider.getUnfilteredCompilerOptions()).containsExactly("unfiltered").inOrder(); assertThat(ccProvider.getLinkOptions()).isEmpty(); - assertThat(CppHelper.getFullyStaticLinkOptions(toolchain, ccProvider, NO_FEATURES, false)) + assertThat(CppHelper.getFullyStaticLinkOptions(toolchain, ccProvider, false)) .containsExactly("linker", "linker-fastbuild", "fully static") .inOrder(); - assertThat(CppHelper.getDynamicLinkOptions(toolchain, ccProvider, NO_FEATURES, false)) + assertThat(CppHelper.getDynamicLinkOptions(toolchain, ccProvider, false)) .containsExactly("linker", "linker-fastbuild", "dynamic") .inOrder(); - assertThat(CppHelper.getFullyStaticLinkOptions(toolchain, ccProvider, NO_FEATURES, true)) + assertThat(CppHelper.getFullyStaticLinkOptions(toolchain, ccProvider, true)) .containsExactly("linker", "linker-fastbuild", "mostly static", "solinker") .inOrder(); - assertThat(CppHelper.getDynamicLinkOptions(toolchain, ccProvider, NO_FEATURES, true)) + assertThat(CppHelper.getDynamicLinkOptions(toolchain, ccProvider, true)) .containsExactly("linker", "linker-fastbuild", "dynamic", "solinker") .inOrder(); @@ -385,10 +379,6 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { + " needsPic: true\n" + " compiler_flag: \"compiler-flag-B-1\"\n" + " compiler_flag: \"compiler-flag-B-2\"\n" - + " optional_compiler_flag {\n" - + " default_setting_name: \"crosstool_fig\"\n" - + " flag: \"-Wfig\"\n" - + " }\n" + " cxx_flag: \"cxx-flag-B-1\"\n" + " cxx_flag: \"cxx-flag-B-2\"\n" + " unfiltered_cxx_flag: \"unfiltered-flag-B-1\"\n" @@ -461,10 +451,6 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { + " debian_extra_requires: \"c\"" + " debian_extra_requires: \"d\"" + "}\n" - + "default_setting {\n" - + " name: \"crosstool_fig\"\n" - + " default_value: false\n" - + "}\n" + "toolchain {\n" + " toolchain_identifier: \"toolchain-identifier-C\"\n" + " host_system_name: \"host-system-name-C\"\n" @@ -525,18 +511,18 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { assertThat(ccProviderA.supportsEmbeddedRuntimes()).isTrue(); assertThat(ccProviderA.toolchainNeedsPic()).isTrue(); - assertThat(CppHelper.getCompilerOptions(toolchainA, ccProviderA, NO_FEATURES)) + assertThat(CppHelper.getCompilerOptions(toolchainA, ccProviderA)) .containsExactly( "compiler-flag-A-1", "compiler-flag-A-2", "fastbuild-flag-A-1", "fastbuild-flag-A-2") .inOrder(); - assertThat(CppHelper.getCxxOptions(toolchainA, ccProviderA, NO_FEATURES)) + assertThat(CppHelper.getCxxOptions(toolchainA, ccProviderA)) .containsExactly( "cxx-flag-A-1", "cxx-flag-A-2", "cxx-fastbuild-flag-A-1", "cxx-fastbuild-flag-A-2") .inOrder(); - assertThat(ccProviderA.getUnfilteredCompilerOptions(NO_FEATURES)) + assertThat(ccProviderA.getUnfilteredCompilerOptions()) .containsExactly("unfiltered-flag-A-1", "unfiltered-flag-A-2") .inOrder(); - assertThat(CppHelper.getDynamicLinkOptions(toolchainA, ccProviderA, NO_FEATURES, true)) + assertThat(CppHelper.getDynamicLinkOptions(toolchainA, ccProviderA, true)) .containsExactly( "linker-flag-A-1", "linker-flag-A-2", @@ -644,12 +630,11 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { assertThat(ccProviderC.toolchainNeedsPic()).isFalse(); assertThat(ccProviderC.supportsFission()).isFalse(); - assertThat(CppHelper.getCompilerOptions(toolchainC, ccProviderC, NO_FEATURES)).isEmpty(); + assertThat(CppHelper.getCompilerOptions(toolchainC, ccProviderC)).isEmpty(); assertThat(toolchainC.getCOptions()).isEmpty(); - assertThat(CppHelper.getCxxOptions(toolchainC, ccProviderC, NO_FEATURES)).isEmpty(); - assertThat(ccProviderC.getUnfilteredCompilerOptions(NO_FEATURES)).isEmpty(); - assertThat(CppHelper.getDynamicLinkOptions(toolchainC, ccProviderC, NO_FEATURES, true)) - .isEmpty(); + assertThat(CppHelper.getCxxOptions(toolchainC, ccProviderC)).isEmpty(); + assertThat(ccProviderC.getUnfilteredCompilerOptions()).isEmpty(); + assertThat(CppHelper.getDynamicLinkOptions(toolchainC, ccProviderC, true)).isEmpty(); assertThat( ccProviderC.configureLinkerOptions( CompilationMode.FASTBUILD, @@ -704,10 +689,8 @@ public class CrosstoolConfigurationLoaderTest extends AnalysisTestCase { "linker-dbg-flag-B-2", "linker-lipo_" + lipoSuffix) .inOrder(); - assertThat( - CppHelper.getCompilerOptions( - toolchainB, ccProviderB, ImmutableList.of("crosstool_fig"))) - .containsAllOf("compiler-flag-B-1", "compiler-flag-B-2", "lipo_" + lipoSuffix, "-Wfig") + assertThat(CppHelper.getCompilerOptions(toolchainB, ccProviderB)) + .containsAllOf("compiler-flag-B-1", "compiler-flag-B-2", "lipo_" + lipoSuffix) .inOrder(); } |