diff options
author | Brian Silverman <bsilver16384@gmail.com> | 2015-05-18 15:03:28 +0000 |
---|---|---|
committer | Damien Martin-Guillerez <dmarting@google.com> | 2015-05-18 20:02:03 +0000 |
commit | b1983bec912ada50b68c26cb72e34c7eaaf9250f (patch) | |
tree | 064e457ba48557d36f60ca38c25ae0353d1eb341 /src/main/java | |
parent | afebc0d53191ac81019c49d1f201cb2b9051fd50 (diff) |
Switch to a CROSSTOOL feature for include flags.
This allows supporting compilers which don't understand -iquote. It
looks like support for -iquote was released in GCC 4.0
(https://gcc.gnu.org/gcc-4.0/changes.html). GCC 3.3.2 definitely does
not have support.
Here's what the error looks like when you try to use an older GCC
without this change:
cc1plus: error: unrecognized option `-iquote.'
cc1plus: error: unrecognized option `
-iquotebazel-out/powerpc-603e-fastbuild/genfiles'
--
Change-Id: Ibf3b74d058a741e23da51963b39ad7505c507d57
Reviewed-on: https://bazel-review.googlesource.com/#/c/1263/
MOS_MIGRATED_REVID=93888319
Diffstat (limited to 'src/main/java')
5 files changed, 73 insertions, 30 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java index d847353cbb..a26585e614 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCommon.java @@ -92,7 +92,8 @@ public final class CcCommon { private static final ImmutableSet<String> DEFAULT_FEATURES = ImmutableSet.of( CppRuleClasses.MODULE_MAPS, CppRuleClasses.MODULE_MAP_HOME_CWD, - CppRuleClasses.HEADER_MODULE_INCLUDES_DEPENDENCIES); + CppRuleClasses.HEADER_MODULE_INCLUDES_DEPENDENCIES, + CppRuleClasses.INCLUDE_PATHS); /** C++ configuration */ private final CppConfiguration cppConfiguration; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java index 25d01b989b..1e5837ecc7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcToolchainFeatures.java @@ -604,7 +604,8 @@ public class CcToolchainFeatures implements Serializable { * @param toolchain the toolchain configuration as specified by the user. * @throws InvalidConfigurationException if the configuration has logical errors. */ - CcToolchainFeatures(CToolchain toolchain) throws InvalidConfigurationException { + @VisibleForTesting + public CcToolchainFeatures(CToolchain toolchain) throws InvalidConfigurationException { // Build up the feature graph. // First, we build up the map of name -> features in one pass, so that earlier features can // reference later features in their configuration. @@ -698,7 +699,7 @@ public class CcToolchainFeatures implements Serializable { /** * Convenience method taking a variadic string argument list for testing. */ - FeatureConfiguration getFeatureConfiguration(String... requestedFeatures) { + public FeatureConfiguration getFeatureConfiguration(String... requestedFeatures) { return getFeatureConfiguration(Arrays.asList(requestedFeatures)); } 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 20f0addf94..2ceecab5ef 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 @@ -1217,23 +1217,8 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable public List<String> getCompilerOptions() { List<String> options = new ArrayList<>(); - - for (PathFragment quoteIncludePath : context.getQuoteIncludeDirs()) { - // "-iquote" is a gcc-specific option. For C compilers that don't support "-iquote", - // we should instead use "-I". - options.add("-iquote"); - options.add(quoteIncludePath.getSafePathString()); - } - for (PathFragment includePath : context.getIncludeDirs()) { - options.add("-I" + includePath.getSafePathString()); - } - for (PathFragment systemIncludePath : context.getSystemIncludeDirs()) { - options.add("-isystem"); - options.add(systemIncludePath.getSafePathString()); - } - CppConfiguration toolchain = cppConfiguration; - + // pluginOpts has to be added before defaultCopts because -fplugin must precede -plugin-arg. options.addAll(pluginOpts); addFilteredOptions(options, toolchain.getCompilerOptions(features)); @@ -1268,6 +1253,31 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable options.add("-D" + CppConfiguration.FDO_STAMP_MACRO + "=\"" + fdoBuildStamp + "\""); } + CcToolchainFeatures.Variables.Builder buildVariables = + new CcToolchainFeatures.Variables.Builder(); + if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS)) { + buildVariables.addVariable("module_name", cppModuleMap.getName()); + buildVariables.addVariable("module_map_file", + cppModuleMap.getArtifact().getExecPathString()); + } + if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) { + buildVariables.addSequenceVariable("module_files", getHeaderModulePaths()); + } + if (featureConfiguration.isEnabled(CppRuleClasses.INCLUDE_PATHS)) { + buildVariables.addSequenceVariable("include_paths", + getSafePathStrings(context.getIncludeDirs())); + buildVariables.addSequenceVariable("quote_include_paths", + getSafePathStrings(context.getQuoteIncludeDirs())); + buildVariables.addSequenceVariable("system_include_paths", + getSafePathStrings(context.getSystemIncludeDirs())); + } + // TODO(bazel-team): This needs to be before adding getUnfilteredCompilerOptions() and after + // adding the warning flags until all toolchains are migrated; currently toolchains use the + // unfiltered compiler options to inject include paths, which is superseded by the feature + // configuration; on the other hand toolchains switch off warnings for the layering check + // that will be re-added by the feature flags. + options.addAll(featureConfiguration.getCommandLine(getActionName(), buildVariables.build())); + options.addAll(toolchain.getUnfilteredCompilerOptions(features)); // GCC gives randomized names to symbols which are defined in @@ -1318,21 +1328,21 @@ public class CppCompileAction extends AbstractAction implements IncludeScannable options.add("-fPIC"); } - CcToolchainFeatures.Variables.Builder buildVariables = - new CcToolchainFeatures.Variables.Builder(); - if (featureConfiguration.isEnabled(CppRuleClasses.MODULE_MAPS)) { - buildVariables.addVariable("module_name", cppModuleMap.getName()); - buildVariables.addVariable("module_map_file", - cppModuleMap.getArtifact().getExecPathString()); - } - if (featureConfiguration.isEnabled(CppRuleClasses.USE_HEADER_MODULES)) { - buildVariables.addSequenceVariable("module_files", getHeaderModulePaths()); - } - options.addAll(featureConfiguration.getCommandLine(getActionName(), buildVariables.build())); return options; } /** + * Get the safe path strings for a list of paths to use in the build variables. + */ + private Collection<String> getSafePathStrings(Collection<PathFragment> paths) { + ImmutableSet.Builder<String> result = ImmutableSet.builder(); + for (PathFragment path : paths) { + result.add(path.getSafePathString()); + } + return result.build(); + } + + /** * Select .pcm inputs to pass on the command line depending on whether we are in pic or non-pic * mode. */ 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 3f74ecfbd8..e0860644f8 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 @@ -800,6 +800,32 @@ public class CppConfiguration extends BuildConfiguration.Fragment { + "}", toolchainBuilder); } + if (!features.contains("include_paths")) { + TextFormat.merge("" + + "feature {" + + " name: 'include_paths'" + + " flag_set {" + + " action: 'preprocess-assemble'" + + " action: 'c-compile'" + + " action: 'c++-compile'" + + " action: 'c++-header-parsing'" + + " action: 'c++-header-preprocessing'" + + " action: 'c++-module-compile'" + + " flag_group {" + + " flag: '-iquote'" + + " flag: '%{quote_include_paths}'" + + " }" + + " flag_group {" + + " flag: '-I%{include_paths}'" + + " }" + + " flag_group {" + + " flag: '-isystem'" + + " flag: '%{system_include_paths}'" + + " }" + + " }" + + "}", + toolchainBuilder); + } } catch (ParseException e) { // Can only happen if we change the proto definition without changing our configuration above. throw new RuntimeException(e); diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java index 9fa675d3dc..c6b3a216d6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppRuleClasses.java @@ -140,4 +140,9 @@ public class CppRuleClasses { * default legacy feature set. */ public static final String NO_LEGACY_FEATURES = "no_legacy_features"; + + /** + * A string constant for the include_paths feature. + */ + public static final String INCLUDE_PATHS = "include_paths"; } |