diff options
author | Googler <noreply@google.com> | 2017-07-06 22:00:21 -0400 |
---|---|---|
committer | John Cater <jcater@google.com> | 2017-07-07 07:08:36 -0400 |
commit | 6cfffdf37e11018c7e6e2cabc90440d6d29c819b (patch) | |
tree | 79620427c42b614fa4094876533d98b7f8fea1d8 /src/main/java/com/google/devtools/build/lib/rules | |
parent | cf483e472790fad1503c5728f7c91dd84f7d9348 (diff) |
Automated rollback of commit 9000e61fc1737444392ffe251727e8331fab3cf2.
*** Reason for rollback ***
Fixed handling of pch, so this should work again
*** Original change description ***
Automated rollback of commit 29ec1b89989db411d2038e2df8657b6435f80403.
*** Reason for rollback ***
Breaks the classroom_ios TAP project [1] in the presence of --experimental_objc_crosstool=all, which was added to the global .blazerc last week.
[1] []
*** Original change description ***
Change ProtobufSupport to use CrosstoolCompilationSupport if experimental_objc_crosstool=all
PiperOrigin-RevId: 161159846
Diffstat (limited to 'src/main/java/com/google/devtools/build/lib/rules')
7 files changed, 96 insertions, 38 deletions
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java index 0ffe8b17de..a231754922 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/AppleStaticLibrary.java @@ -117,7 +117,8 @@ public class AppleStaticLibrary implements RuleConfiguredTargetFactory { protosToAvoid, ImmutableList.<ProtoSourcesProvider>of(), objcProtoProviders, - ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders)) + ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders), + childConfigurationsAndToolchains.get(childConfig)) .registerGenerationActions() .registerCompilationActions(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java index dcaa523911..89803bdfdd 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationSupport.java @@ -301,6 +301,7 @@ public abstract class CompilationSupport { protected final IntermediateArtifacts intermediateArtifacts; protected final boolean useDeps; protected final Map<String, NestedSet<Artifact>> outputGroupCollector; + protected final CcToolchainProvider toolchain; protected final boolean isTestRule; protected final boolean usePch; @@ -324,6 +325,7 @@ public abstract class CompilationSupport { CompilationAttributes compilationAttributes, boolean useDeps, Map<String, NestedSet<Artifact>> outputGroupCollector, + CcToolchainProvider toolchain, boolean isTestRule, boolean usePch) { this.ruleContext = ruleContext; @@ -336,6 +338,18 @@ public abstract class CompilationSupport { this.isTestRule = isTestRule; this.outputGroupCollector = outputGroupCollector; this.usePch = usePch; + // TODO(b/62143697): Remove this check once all rules are using the crosstool support. + if (ruleContext + .attributes() + .has(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, BuildType.LABEL)) { + if (toolchain == null) { + toolchain = CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext); + } + this.toolchain = toolchain; + } else { + // Since the rule context doesn't have a toolchain at all, ignore any provided override. + this.toolchain = null; + } } /** Builder for {@link CompilationSupport} */ @@ -347,6 +361,7 @@ public abstract class CompilationSupport { private boolean useDeps = true; private Map<String, NestedSet<Artifact>> outputGroupCollector; private boolean isObjcLibrary = false; + private CcToolchainProvider toolchain; private boolean isTestRule = false; private boolean usePch = true; @@ -423,6 +438,17 @@ public abstract class CompilationSupport { } /** + * Sets {@link CcToolchainProvider} for the calling target. + * + * <p>This is needed if it can't correctly be inferred directly from the rule context. Setting + * to null causes the default to be used as if this was never called. + */ + public Builder setToolchainProvider(CcToolchainProvider toolchain) { + this.toolchain = toolchain; + return this; + } + + /** * Returns a {@link CompilationSupport} instance. This is either a {@link * CrosstoolCompilationSupport} or {@link LegacyCompilationSupport} depending on the value of * --experimental_objc_crosstool. @@ -458,6 +484,7 @@ public abstract class CompilationSupport { compilationAttributes, useDeps, outputGroupCollector, + toolchain, isTestRule, usePch); } else { @@ -468,6 +495,7 @@ public abstract class CompilationSupport { compilationAttributes, useDeps, outputGroupCollector, + toolchain, isTestRule, usePch); } @@ -490,7 +518,7 @@ public abstract class CompilationSupport { objcProvider, ExtraCompileArgs.NONE, ImmutableList.<PathFragment>of(), - maybeGetCcToolchain(), + toolchain, maybeGetFdoSupport()); } @@ -571,7 +599,7 @@ public abstract class CompilationSupport { return registerFullyLinkAction( objcProvider, outputArchive, - maybeGetCcToolchain(), + toolchain, maybeGetFdoSupport()); } @@ -629,7 +657,7 @@ public abstract class CompilationSupport { objcProvider, inputArtifacts, outputArchive, - maybeGetCcToolchain(), + toolchain, maybeGetFdoSupport()); } @@ -758,7 +786,7 @@ public abstract class CompilationSupport { common.getObjcProvider(), extraCompileArgs, priorityHeaders, - maybeGetCcToolchain(), + toolchain, maybeGetFdoSupport()); } return this; @@ -1446,18 +1474,6 @@ public abstract class CompilationSupport { } @Nullable - private CcToolchainProvider maybeGetCcToolchain() { - // TODO(rduan): Remove this check once all rules are using the crosstool support. - if (ruleContext - .attributes() - .has(CcToolchain.CC_TOOLCHAIN_DEFAULT_ATTRIBUTE_NAME, BuildType.LABEL)) { - return CppHelper.getToolchainUsingDefaultCcToolchainAttribute(ruleContext); - } else { - return null; - } - } - - @Nullable private FdoSupportProvider maybeGetFdoSupport() { // TODO(rduan): Remove this check once all rules are using the crosstool support. if (ruleContext diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java index 2718464fa8..f83b57410a 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CrosstoolCompilationSupport.java @@ -132,6 +132,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { CompilationAttributes.Builder.fromRuleContext(ruleContext).build(), /*useDeps=*/ true, outputGroupCollector, + null, /*isTestRule=*/ false, /*usePch=*/ true); } @@ -144,6 +145,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { * @param intermediateArtifacts IntermediateArtifacts for deriving artifact paths * @param compilationAttributes attributes of the calling target * @param useDeps true if deps should be used + * @param toolchain if not null overrides the default toolchain from the ruleContext. * @param usePch true if pch should be used */ public CrosstoolCompilationSupport( @@ -153,6 +155,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { CompilationAttributes compilationAttributes, boolean useDeps, Map<String, NestedSet<Artifact>> outputGroupCollector, + CcToolchainProvider toolchain, boolean isTestRule, boolean usePch) { super( @@ -162,6 +165,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { compilationAttributes, useDeps, outputGroupCollector, + toolchain, isTestRule, usePch); } @@ -189,18 +193,28 @@ public class CrosstoolCompilationSupport extends CompilationSupport { // TODO(b/30783125): Signal the need for this action in the CROSSTOOL. registerObjFilelistAction(getObjFiles(compilationArtifacts, intermediateArtifacts), objList); - + extension.addVariableCategory(VariableCategory.ARCHIVE_VARIABLES); - + helper = createCcLibraryHelper( - objcProvider, compilationArtifacts, extension.build(), ccToolchain, fdoSupport) + objcProvider, + compilationArtifacts, + extension.build(), + ccToolchain, + fdoSupport, + priorityHeaders) .setLinkType(LinkTargetType.OBJC_ARCHIVE) .addLinkActionInput(objList); } else { helper = createCcLibraryHelper( - objcProvider, compilationArtifacts, extension.build(), ccToolchain, fdoSupport); + objcProvider, + compilationArtifacts, + extension.build(), + ccToolchain, + fdoSupport, + priorityHeaders); } Info info = helper.build(); @@ -382,7 +396,8 @@ public class CrosstoolCompilationSupport extends CompilationSupport { CompilationArtifacts compilationArtifacts, VariablesExtension extension, CcToolchainProvider ccToolchain, - FdoSupportProvider fdoSupport) { + FdoSupportProvider fdoSupport, + Iterable<PathFragment> priorityHeaders) { PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext); Collection<Artifact> arcSources = ImmutableSortedSet.copyOf(compilationArtifacts.getSrcs()); Collection<Artifact> nonArcSources = @@ -401,7 +416,8 @@ public class CrosstoolCompilationSupport extends CompilationSupport { createIncludeProcessing(privateHdrs, objcProvider, pchHdr), ruleContext.getFragment(ObjcConfiguration.class), isHeaderThinningEnabled(), - intermediateArtifacts); + intermediateArtifacts, + buildConfiguration); CcLibraryHelper result = new CcLibraryHelper( ruleContext, @@ -424,6 +440,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { // generate C++ protos. .setCheckDepsGenerateCpp(false) .addCopts(getCompileRuleCopts()) + .addIncludeDirs(priorityHeaders) .addIncludeDirs(objcProvider.get(INCLUDE)) .addCopts(ruleContext.getFragment(ObjcConfiguration.class).getCoptsForCompilationMode()) .addSystemIncludeDirs(objcProvider.get(INCLUDE_SYSTEM)) diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java index dc6045f7d8..25910d43c5 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/LegacyCompilationSupport.java @@ -146,6 +146,7 @@ public class LegacyCompilationSupport extends CompilationSupport { CompilationAttributes compilationAttributes, boolean useDeps, Map<String, NestedSet<Artifact>> outputGroupCollector, + CcToolchainProvider toolchain, boolean isTestRule, boolean usePch) { super( @@ -155,6 +156,7 @@ public class LegacyCompilationSupport extends CompilationSupport { compilationAttributes, useDeps, outputGroupCollector, + toolchain, isTestRule, usePch); } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java index 8130c4e525..1b3d53f89e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/MultiArchBinarySupport.java @@ -243,7 +243,8 @@ public class MultiArchBinarySupport { protosToAvoid, ImmutableList.<ProtoSourcesProvider>of(), objcProtoProviders, - ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders)) + ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders), + childConfigurationsAndToolchains.get(childConfig)) .registerGenerationActions() .registerCompilationActions(); protosObjcProvider = protoSupport.getObjcProvider(); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java index f936d43881..51d4c7c9a6 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCppSemantics.java @@ -22,6 +22,7 @@ import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import com.google.devtools.build.lib.actions.Artifact; import com.google.devtools.build.lib.analysis.RuleContext; +import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.rules.cpp.CppCompilationContext.Builder; import com.google.devtools.build.lib.rules.cpp.CppCompileActionBuilder; @@ -46,6 +47,7 @@ public class ObjcCppSemantics implements CppSemantics { private final ObjcConfiguration config; private final boolean isHeaderThinningEnabled; private final IntermediateArtifacts intermediateArtifacts; + private final BuildConfiguration buildConfiguration; /** * Set of {@link com.google.devtools.build.lib.util.FileType} of source artifacts that are @@ -69,18 +71,21 @@ public class ObjcCppSemantics implements CppSemantics { * @param isHeaderThinningEnabled true if headers_list artifacts should be generated and added as * input to compiling actions * @param intermediateArtifacts used to create headers_list artifacts + * @param buildConfiguration the build configuration for this build */ public ObjcCppSemantics( ObjcProvider objcProvider, IncludeProcessing includeProcessing, ObjcConfiguration config, boolean isHeaderThinningEnabled, - IntermediateArtifacts intermediateArtifacts) { + IntermediateArtifacts intermediateArtifacts, + BuildConfiguration buildConfiguration) { this.objcProvider = objcProvider; this.includeProcessing = includeProcessing; this.config = config; this.isHeaderThinningEnabled = isHeaderThinningEnabled; this.intermediateArtifacts = intermediateArtifacts; + this.buildConfiguration = buildConfiguration; } @Override @@ -134,6 +139,14 @@ public class ObjcCppSemantics implements CppSemantics { ObjcCommon.userHeaderSearchPaths(objcProvider, ruleContext.getConfiguration())) { contextBuilder.addQuoteIncludeDir(iquotePath); } + + // ProtoSupport creates multiple compilation contexts for a single rule, potentially multiple + // archives per build configuration. This covers that worst case. + contextBuilder.setPurpose( + "ObjcCppSemantics_build_arch_" + + buildConfiguration.getMnemonic() + + "_with_suffix_" + + intermediateArtifacts.archiveFileNameSuffix()); } @Override diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java index 4a1c4bb7c4..053cd56181 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ProtobufSupport.java @@ -33,12 +33,12 @@ import com.google.devtools.build.lib.analysis.config.BuildConfiguration; import com.google.devtools.build.lib.collect.nestedset.NestedSet; import com.google.devtools.build.lib.collect.nestedset.NestedSetBuilder; import com.google.devtools.build.lib.packages.RuleClass.ConfiguredTargetFactory.RuleErrorException; +import com.google.devtools.build.lib.rules.cpp.CcToolchainProvider; import com.google.devtools.build.lib.rules.proto.ProtoSourcesProvider; import com.google.devtools.build.lib.vfs.FileSystemUtils; import com.google.devtools.build.lib.vfs.PathFragment; import java.util.HashMap; import java.util.Set; -import java.util.TreeMap; /** * Support for generating Objective C proto static libraries that registers actions which generate @@ -71,6 +71,7 @@ final class ProtobufSupport { private final Set<Artifact> dylibHandledProtos; private final Iterable<ObjcProtoProvider> objcProtoProviders; private final NestedSet<Artifact> portableProtoFilters; + private final CcToolchainProvider toolchain; // Each entry of this map represents a generation action and a compilation action. The input set // are dependencies of the output set. The output set is always a subset of, or the same set as, @@ -112,7 +113,8 @@ final class ProtobufSupport { NestedSetBuilder.<Artifact>stableOrder().build(), protoProviders, objcProtoProviders, - portableProtoFilters); + portableProtoFilters, + null); } /** @@ -129,6 +131,8 @@ final class ProtobufSupport { * symbols * @param protoProviders the list of ProtoSourcesProviders that this proto support should process * @param objcProtoProviders the list of ObjcProtoProviders that this proto support should process + * @param toolchain if not null, the toolchain to override the default toolchain for the rule + * context. */ public ProtobufSupport( RuleContext ruleContext, @@ -136,7 +140,8 @@ final class ProtobufSupport { NestedSet<Artifact> dylibHandledProtos, Iterable<ProtoSourcesProvider> protoProviders, Iterable<ObjcProtoProvider> objcProtoProviders, - NestedSet<Artifact> portableProtoFilters) { + NestedSet<Artifact> portableProtoFilters, + CcToolchainProvider toolchain) { this.ruleContext = ruleContext; this.buildConfiguration = buildConfiguration; this.attributes = new ProtoAttributes(ruleContext); @@ -146,6 +151,7 @@ final class ProtobufSupport { this.intermediateArtifacts = ObjcRuleClasses.intermediateArtifacts(ruleContext, buildConfiguration); this.inputsToOutputsMap = getInputsToOutputsMap(attributes, protoProviders, objcProtoProviders); + this.toolchain = toolchain; } /** @@ -204,16 +210,18 @@ final class ProtobufSupport { ObjcCommon common = getCommon(intermediateArtifacts, compilationArtifacts); - new LegacyCompilationSupport( - ruleContext, - buildConfiguration, - intermediateArtifacts, - new CompilationAttributes.Builder().build(), - /*useDeps=*/ false, - new TreeMap<String, NestedSet<Artifact>>(), - /*isTestRule=*/ false, - /*usePch=*/ false) - .registerCompileAndArchiveActions(common, userHeaderSearchPaths); + CompilationSupport compilationSupport = + new CompilationSupport.Builder() + .setRuleContext(ruleContext) + .setConfig(buildConfiguration) + .setIntermediateArtifacts(intermediateArtifacts) + .setCompilationAttributes(new CompilationAttributes.Builder().build()) + .setToolchainProvider(toolchain) + .doNotUseDeps() + .doNotUsePch() + .build(); + + compilationSupport.registerCompileAndArchiveActions(common, userHeaderSearchPaths); actionId++; } |