diff options
author | Ulf Adams <ulfjack@google.com> | 2016-08-29 13:00:22 +0000 |
---|---|---|
committer | Klaus Aehlig <aehlig@google.com> | 2016-08-29 15:36:48 +0000 |
commit | 8e8d4446d17ce6422c2ee15c15be3387b7fb6b1a (patch) | |
tree | 801934416cf9afb06dbb98121de337c90735b233 /src/main/java/com/google/devtools/build/lib/rules/cpp | |
parent | 8e138f9ffa098273823b4fe59e910de463801775 (diff) |
Remove the dependency from the C++ rules to Apple stuff.
This makes the C++ rules standalone, and the CcCommonTest ensures that (at
least) analysis of C++ targets works even if no Apple / Xcode rules are
present.
We can also compile them separately, in a future change.
--
MOS_MIGRATED_REVID=131583691
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules/cpp')
6 files changed, 49 insertions, 23 deletions
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 f24432db98..f00c3d9d96 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 @@ -218,7 +218,8 @@ public class CcToolchain implements RuleConfiguredTargetFactory { supportsHeaderParsing, getBuildVariables(ruleContext), getBuiltinIncludes(ruleContext), - coverageEnvironment.build()); + coverageEnvironment.build(), + getEnvironment(ruleContext)); RuleConfiguredTargetBuilder builder = new RuleConfiguredTargetBuilder(ruleContext) .add(CcToolchainProvider.class, provider) @@ -323,10 +324,22 @@ public class CcToolchain implements RuleConfiguredTargetFactory { } /** - * Returns a map that should be templated into the crosstool as build variables. Is meant to + * Returns a map that should be templated into the crosstool as build variables. Is meant to * be overridden by subclasses of CcToolchain. + * + * @param ruleContext the rule context */ protected Map<String, String> getBuildVariables(RuleContext ruleContext) { return ImmutableMap.<String, String>of(); } + + /** + * Returns a map of environment variables to be added to the compile actions created for this + * toolchain. Ideally, this will get replaced by features, which also allow setting env variables. + * + * @param ruleContext the rule context + */ + protected ImmutableMap<String, String> getEnvironment(RuleContext ruleContext) { + return ImmutableMap.<String, String>of(); + } } 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 082d469810..755da5311c 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 @@ -59,7 +59,8 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { false, ImmutableMap.<String, String>of(), ImmutableList.<Artifact>of(), - NestedSetBuilder.<Pair<String, String>>emptySet(Order.COMPILE_ORDER)); + NestedSetBuilder.<Pair<String, String>>emptySet(Order.COMPILE_ORDER), + ImmutableMap.<String, String>of()); @Nullable private final CppConfiguration cppConfiguration; private final NestedSet<Artifact> crosstool; @@ -78,9 +79,10 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { private final CppCompilationContext cppCompilationContext; private final boolean supportsParamFiles; private final boolean supportsHeaderParsing; - private final Map<String, String> buildVariables; + private final ImmutableMap<String, String> buildVariables; private final ImmutableList<Artifact> builtinIncludeFiles; private final NestedSet<Pair<String, String>> coverageEnvironment; + private final ImmutableMap<String, String> environment; public CcToolchainProvider( @Nullable CppConfiguration cppConfiguration, @@ -102,7 +104,8 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { boolean supportsHeaderParsing, Map<String, String> buildVariables, ImmutableList<Artifact> builtinIncludeFiles, - NestedSet<Pair<String, String>> coverageEnvironment) { + NestedSet<Pair<String, String>> coverageEnvironment, + ImmutableMap<String, String> environment) { this.cppConfiguration = cppConfiguration; this.crosstool = Preconditions.checkNotNull(crosstool); this.crosstoolMiddleman = Preconditions.checkNotNull(crosstoolMiddleman); @@ -120,9 +123,10 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { this.cppCompilationContext = Preconditions.checkNotNull(cppCompilationContext); this.supportsParamFiles = supportsParamFiles; this.supportsHeaderParsing = supportsHeaderParsing; - this.buildVariables = buildVariables; + this.buildVariables = ImmutableMap.copyOf(buildVariables); this.builtinIncludeFiles = builtinIncludeFiles; this.coverageEnvironment = coverageEnvironment; + this.environment = environment; } /** @@ -257,7 +261,7 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { /** * Returns build variables to be templated into the crosstool. */ - public Map<String, String> getBuildVariables() { + public ImmutableMap<String, String> getBuildVariables() { return buildVariables; } @@ -275,4 +279,8 @@ public final class CcToolchainProvider implements TransitiveInfoProvider { public NestedSet<Pair<String, String>> getCoverageEnvironment() { return coverageEnvironment; } + + public ImmutableMap<String, String> getEnvironment() { + return environment; + } } 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 456df2d61c..0debf56da0 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 @@ -51,8 +51,6 @@ import com.google.devtools.build.lib.events.Event; import com.google.devtools.build.lib.events.EventHandler; import com.google.devtools.build.lib.profiler.Profiler; import com.google.devtools.build.lib.profiler.ProfilerTask; -import com.google.devtools.build.lib.rules.apple.AppleConfiguration; -import com.google.devtools.build.lib.rules.apple.Platform; import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.FeatureConfiguration; import com.google.devtools.build.lib.rules.cpp.CppCompileActionContext.Reply; import com.google.devtools.build.lib.rules.cpp.CppConfiguration.Tool; @@ -157,7 +155,9 @@ public class CppCompileAction extends AbstractAction public static final String ASSEMBLE = "assemble"; public static final String PREPROCESS_ASSEMBLE = "preprocess-assemble"; - + // TODO(ulfjack): this is only used to get the local shell environment and to check if coverage is + // enabled. Move those two things to local fields and drop this. Accessing anything other than + // these fields can impact correctness! private final BuildConfiguration configuration; protected final Artifact outputFile; private final Label sourceLabel; @@ -170,6 +170,7 @@ public class CppCompileAction extends AbstractAction private final ImmutableList<Artifact> builtinIncludeFiles; @VisibleForTesting public final CppCompileCommandLine cppCompileCommandLine; private final ImmutableSet<String> executionRequirements; + private final ImmutableMap<String, String> environment; @VisibleForTesting final CppConfiguration cppConfiguration; private final FeatureConfiguration featureConfiguration; @@ -255,6 +256,7 @@ public class CppCompileAction extends AbstractAction Iterable<IncludeScannable> lipoScannables, UUID actionClassId, ImmutableSet<String> executionRequirements, + ImmutableMap<String, String> environment, String actionName, RuleContext ruleContext) { super( @@ -289,6 +291,7 @@ public class CppCompileAction extends AbstractAction this.lipoScannables = lipoScannables; this.actionClassId = actionClassId; this.executionRequirements = executionRequirements; + this.environment = environment; // We do not need to include the middleman artifact since it is a generated // artifact and will definitely exist prior to this action execution. @@ -632,18 +635,7 @@ public class CppCompileAction extends AbstractAction environment.put("PWD", "/proc/self/cwd"); } - // TODO(bazel-team): Handle at the level of crosstool (feature) templates instead of in this - // compile action. This will also prevent the need for apple host system and target platform - // evaluation here. - AppleConfiguration appleConfiguration = configuration.getFragment(AppleConfiguration.class); - if (CppConfiguration.MAC_SYSTEM_NAME.equals(getHostSystemName())) { - environment.putAll(appleConfiguration.getAppleHostSystemEnv()); - } - if (Platform.isApplePlatform(cppConfiguration.getTargetCpu())) { - environment.putAll(appleConfiguration.appleTargetPlatformEnv( - Platform.forTargetCpu(cppConfiguration.getTargetCpu()))); - } - + environment.putAll(this.environment); environment.putAll(cppCompileCommandLine.getEnvironment()); // TODO(bazel-team): Check (crosstool) host system name instead of using OS.getCurrent. 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 d5bd8ed5be..0fe142b181 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 @@ -38,7 +38,9 @@ import com.google.devtools.build.lib.util.Preconditions; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.ArrayList; import java.util.Collection; +import java.util.LinkedHashMap; import java.util.List; +import java.util.Map; import java.util.UUID; import java.util.regex.Pattern; @@ -76,6 +78,7 @@ public class CppCompileActionBuilder { private ImmutableMap<Artifact, IncludeScannable> lipoScannableMap; private RuleContext ruleContext = null; private Boolean shouldScanIncludes; + private Map<String, String> environment = new LinkedHashMap<>(); // New fields need to be added to the copy constructor. /** @@ -143,6 +146,7 @@ public class CppCompileActionBuilder { this.lipoScannableMap = other.lipoScannableMap; this.ruleContext = other.ruleContext; this.shouldScanIncludes = other.shouldScanIncludes; + this.environment = new LinkedHashMap<>(other.environment); } public PathFragment getTempOutputFile() { @@ -263,7 +267,7 @@ public class CppCompileActionBuilder { executionRequirements = featureConfiguration.getToolForAction(getActionName()).getExecutionRequirements(); } - + // Copying the collections is needed to make the builder reusable. if (fake) { return new FakeCppCompileAction( @@ -314,6 +318,7 @@ public class CppCompileActionBuilder { getLipoScannables(realMandatoryInputs), actionClassId, executionRequirements, + ImmutableMap.copyOf(environment), getActionName(), ruleContext); } @@ -336,6 +341,11 @@ public class CppCompileActionBuilder { return this; } + public CppCompileActionBuilder addEnvironment(Map<String, String> environment) { + this.environment.putAll(environment); + return this; + } + public CppCompileActionBuilder setSpecialInputsHandler( SpecialInputsHandler specialInputsHandler) { this.specialInputsHandler = specialInputsHandler; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java index f4c8e09a04..432dc3ca15 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CppModel.java @@ -1015,6 +1015,7 @@ public final class CppModel { ruleContext, source, label); builder.setContext(forInterface ? interfaceContext : context).addCopts(copts); + builder.addEnvironment(CppHelper.getToolchain(ruleContext).getEnvironment()); return builder; } 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 e31ff2ee4b..4b68bcb229 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 @@ -20,6 +20,7 @@ import com.google.common.base.Function; import com.google.common.base.Joiner; import com.google.common.base.Predicate; import com.google.common.collect.ImmutableList; +import com.google.common.collect.ImmutableMap; import com.google.common.collect.ImmutableSet; import com.google.common.collect.Iterables; import com.google.devtools.build.lib.actions.ActionExecutionContext; @@ -110,6 +111,7 @@ public class FakeCppCompileAction extends CppCompileAction { ImmutableList.<IncludeScannable>of(), GUID, ImmutableSet.<String>of(), + ImmutableMap.<String, String>of(), CppCompileAction.CPP_COMPILE, ruleContext); this.tempOutputFile = Preconditions.checkNotNull(tempOutputFile); |