diff options
Diffstat (limited to 'src')
9 files changed, 44 insertions, 140 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 81e0975a4f..249b43c374 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,8 +117,7 @@ public class AppleStaticLibrary implements RuleConfiguredTargetFactory { protosToAvoid, ImmutableList.<ProtoSourcesProvider>of(), objcProtoProviders, - ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders), - childConfigurationsAndToolchains.get(childConfig)) + ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders)) .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 306e4d460e..285fbd0fe5 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 @@ -302,7 +302,6 @@ 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; @@ -326,7 +325,6 @@ public abstract class CompilationSupport { CompilationAttributes compilationAttributes, boolean useDeps, Map<String, NestedSet<Artifact>> outputGroupCollector, - CcToolchainProvider toolchain, boolean isTestRule, boolean usePch) { this.ruleContext = ruleContext; @@ -339,18 +337,6 @@ 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} */ @@ -362,7 +348,6 @@ 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; @@ -439,17 +424,6 @@ 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. @@ -485,7 +459,6 @@ public abstract class CompilationSupport { compilationAttributes, useDeps, outputGroupCollector, - toolchain, isTestRule, usePch); } else { @@ -496,7 +469,6 @@ public abstract class CompilationSupport { compilationAttributes, useDeps, outputGroupCollector, - toolchain, isTestRule, usePch); } @@ -519,7 +491,7 @@ public abstract class CompilationSupport { objcProvider, ExtraCompileArgs.NONE, ImmutableList.<PathFragment>of(), - toolchain, + maybeGetCcToolchain(), maybeGetFdoSupport()); } @@ -600,7 +572,7 @@ public abstract class CompilationSupport { return registerFullyLinkAction( objcProvider, outputArchive, - toolchain, + maybeGetCcToolchain(), maybeGetFdoSupport()); } @@ -658,7 +630,7 @@ public abstract class CompilationSupport { objcProvider, inputArtifacts, outputArchive, - toolchain, + maybeGetCcToolchain(), maybeGetFdoSupport()); } @@ -787,7 +759,7 @@ public abstract class CompilationSupport { common.getObjcProvider(), extraCompileArgs, priorityHeaders, - toolchain, + maybeGetCcToolchain(), maybeGetFdoSupport()); } return this; @@ -1474,6 +1446,18 @@ 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 c609fa8139..cf5cf0c8fa 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 @@ -134,7 +134,6 @@ public class CrosstoolCompilationSupport extends CompilationSupport { CompilationAttributes.Builder.fromRuleContext(ruleContext).build(), /*useDeps=*/ true, outputGroupCollector, - null, /*isTestRule=*/ false, /*usePch=*/ true); } @@ -147,7 +146,6 @@ 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( @@ -157,7 +155,6 @@ public class CrosstoolCompilationSupport extends CompilationSupport { CompilationAttributes compilationAttributes, boolean useDeps, Map<String, NestedSet<Artifact>> outputGroupCollector, - CcToolchainProvider toolchain, boolean isTestRule, boolean usePch) { super( @@ -167,7 +164,6 @@ public class CrosstoolCompilationSupport extends CompilationSupport { compilationAttributes, useDeps, outputGroupCollector, - toolchain, isTestRule, usePch); } @@ -195,28 +191,18 @@ 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, - priorityHeaders) + objcProvider, compilationArtifacts, extension.build(), ccToolchain, fdoSupport) .setLinkType(LinkTargetType.OBJC_ARCHIVE) .addLinkActionInput(objList); } else { helper = createCcLibraryHelper( - objcProvider, - compilationArtifacts, - extension.build(), - ccToolchain, - fdoSupport, - priorityHeaders); + objcProvider, compilationArtifacts, extension.build(), ccToolchain, fdoSupport); } Info info = helper.build(); @@ -405,8 +391,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { CompilationArtifacts compilationArtifacts, VariablesExtension extension, CcToolchainProvider ccToolchain, - FdoSupportProvider fdoSupport, - Iterable<PathFragment> priorityHeaders) { + FdoSupportProvider fdoSupport) { PrecompiledFiles precompiledFiles = new PrecompiledFiles(ruleContext); Collection<Artifact> arcSources = ImmutableSortedSet.copyOf(compilationArtifacts.getSrcs()); Collection<Artifact> nonArcSources = @@ -425,8 +410,7 @@ public class CrosstoolCompilationSupport extends CompilationSupport { createIncludeProcessing(privateHdrs, objcProvider, pchHdr), ruleContext.getFragment(ObjcConfiguration.class), isHeaderThinningEnabled(), - intermediateArtifacts, - buildConfiguration); + intermediateArtifacts); CcLibraryHelper result = new CcLibraryHelper( ruleContext, @@ -456,7 +440,6 @@ public class CrosstoolCompilationSupport extends CompilationSupport { .getFragment(ObjcConfiguration.class) .getCoptsForCompilationMode()) .build()) - .addIncludeDirs(priorityHeaders) .addIncludeDirs(objcProvider.get(INCLUDE)) .addSystemIncludeDirs(objcProvider.get(INCLUDE_SYSTEM)) .setCppModuleMap(intermediateArtifacts.moduleMap()) 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 570487a49f..8f490626cb 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 @@ -148,7 +148,6 @@ public class LegacyCompilationSupport extends CompilationSupport { CompilationAttributes compilationAttributes, boolean useDeps, Map<String, NestedSet<Artifact>> outputGroupCollector, - CcToolchainProvider toolchain, boolean isTestRule, boolean usePch) { super( @@ -158,7 +157,6 @@ 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 eee37b0089..6cf378355a 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,8 +243,7 @@ public class MultiArchBinarySupport { protosToAvoid, ImmutableList.<ProtoSourcesProvider>of(), objcProtoProviders, - ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders), - childConfigurationsAndToolchains.get(childConfig)) + ProtobufSupport.getTransitivePortableProtoFilters(objcProtoProviders)) .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 30b9710604..0e1f2f63d5 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,7 +22,6 @@ 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; @@ -47,7 +46,6 @@ 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 @@ -71,21 +69,18 @@ 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, - BuildConfiguration buildConfiguration) { + IntermediateArtifacts intermediateArtifacts) { this.objcProvider = objcProvider; this.includeProcessing = includeProcessing; this.config = config; this.isHeaderThinningEnabled = isHeaderThinningEnabled; this.intermediateArtifacts = intermediateArtifacts; - this.buildConfiguration = buildConfiguration; } @Override @@ -137,14 +132,6 @@ 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 9038e68d78..f779b952e1 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,7 +71,6 @@ 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, @@ -113,8 +112,7 @@ final class ProtobufSupport { NestedSetBuilder.<Artifact>stableOrder().build(), protoProviders, objcProtoProviders, - portableProtoFilters, - null); + portableProtoFilters); } /** @@ -131,8 +129,6 @@ 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, @@ -140,8 +136,7 @@ final class ProtobufSupport { NestedSet<Artifact> dylibHandledProtos, Iterable<ProtoSourcesProvider> protoProviders, Iterable<ObjcProtoProvider> objcProtoProviders, - NestedSet<Artifact> portableProtoFilters, - CcToolchainProvider toolchain) { + NestedSet<Artifact> portableProtoFilters) { this.ruleContext = ruleContext; this.buildConfiguration = buildConfiguration; this.attributes = new ProtoAttributes(ruleContext); @@ -151,7 +146,6 @@ final class ProtobufSupport { this.intermediateArtifacts = ObjcRuleClasses.intermediateArtifacts(ruleContext, buildConfiguration); this.inputsToOutputsMap = getInputsToOutputsMap(attributes, protoProviders, objcProtoProviders); - this.toolchain = toolchain; } /** @@ -210,18 +204,16 @@ final class ProtobufSupport { ObjcCommon common = getCommon(intermediateArtifacts, compilationArtifacts); - 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); + new LegacyCompilationSupport( + ruleContext, + buildConfiguration, + intermediateArtifacts, + new CompilationAttributes.Builder().build(), + /*useDeps=*/ false, + new TreeMap<String, NestedSet<Artifact>>(), + /*isTestRule=*/ false, + /*usePch=*/ false) + .registerCompileAndArchiveActions(common, userHeaderSearchPaths); actionId++; } diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java index c58c64972c..c78b771a88 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/AppleBinaryTest.java @@ -568,9 +568,7 @@ public class AppleBinaryTest extends ObjcRuleTestCase { assertThat(getFirstArtifactEndingWith(binObjectFiles, "DataB.pbobjc.o")).isNull(); Action dataAObjectAction = getGeneratingAction(getFirstArtifactEndingWith(binObjectFiles, "DataA.pbobjc.o")); - assertThat( - getFirstArtifactEndingWith( - getExpandedActionInputs(dataAObjectAction), "DataB.pbobjc.h")) + assertThat(getFirstArtifactEndingWith(dataAObjectAction.getInputs(), "DataB.pbobjc.h")) .isNotNull(); } diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java index 8116e34c9b..c099a11be3 100644 --- a/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java +++ b/src/test/java/com/google/devtools/build/lib/rules/objc/ObjcRuleTestCase.java @@ -91,7 +91,6 @@ import com.google.devtools.build.lib.rules.apple.ApplePlatform.PlatformType; import com.google.devtools.build.lib.rules.apple.AppleToolchain; import com.google.devtools.build.lib.rules.apple.DottedVersion; import com.google.devtools.build.lib.rules.apple.XcodeVersionProperties; -import com.google.devtools.build.lib.rules.cpp.CppCompileAction; import com.google.devtools.build.lib.rules.cpp.CppLinkAction; import com.google.devtools.build.lib.rules.objc.CompilationSupport.ExtraLinkArgs; import com.google.devtools.build.lib.rules.objc.ObjcCommandLineOptions.ObjcCrosstoolMode; @@ -1355,9 +1354,6 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase { assertOnlyRequiredInputsArePresentForBundledCompilation(topTarget); assertCoptsAndDefinesForBundlingTarget(topTarget); assertBundledGroupsGetCreatedAndLinked(topTarget); - if (getObjcCrosstoolMode() == ObjcCrosstoolMode.ALL) { - assertBundledCompilationUsesCrosstool(topTarget); - } } protected ImmutableList<Artifact> getAllObjectFilesLinkedInBin(Artifact bin) { @@ -1456,22 +1452,6 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase { .doesNotContain("protos/data_b.proto"); } - /** - * Ensures that all middleman artifacts in the action input are expanded so that the real inputs - * are also included. - */ - protected Iterable<Artifact> getExpandedActionInputs(Action action) { - List<Artifact> containedArtifacts = new ArrayList<>(); - for (Artifact input : action.getInputs()) { - if (input.isMiddlemanArtifact()) { - Action middlemanAction = getGeneratingAction(input); - Iterables.addAll(containedArtifacts, getExpandedActionInputs(middlemanAction)); - } - containedArtifacts.add(input); - } - return containedArtifacts; - } - private void assertOnlyRequiredInputsArePresentForBundledCompilation(ConfiguredTarget topTarget) { Artifact protoHeaderA = getBinArtifact("_generated_protos/x/protos/DataA.pbobjc.h", topTarget); Artifact protoHeaderB = getBinArtifact("_generated_protos/x/protos/DataB.pbobjc.h", topTarget); @@ -1497,15 +1477,15 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase { assertThat(protoObjectActionC).isNotNull(); assertThat(protoObjectActionD).isNotNull(); - assertThat(getExpandedActionInputs(protoObjectActionA)) + assertThat(protoObjectActionA.getInputs()) .containsNoneOf(protoHeaderB, protoHeaderC, protoHeaderD); - assertThat(getExpandedActionInputs(protoObjectActionB)) + assertThat(protoObjectActionB.getInputs()) .containsNoneOf(protoHeaderA, protoHeaderC, protoHeaderD); - assertThat(getExpandedActionInputs(protoObjectActionC)) + assertThat(protoObjectActionC.getInputs()) .containsNoneOf(protoHeaderA, protoHeaderB, protoHeaderD); - assertThat(getExpandedActionInputs(protoObjectActionD)) + assertThat(protoObjectActionD.getInputs()) .containsAllOf(protoHeaderA, protoHeaderC, protoHeaderD); - assertThat(getExpandedActionInputs(protoObjectActionD)) + assertThat(protoObjectActionD.getInputs()) .doesNotContain(protoHeaderB); } @@ -1549,22 +1529,6 @@ public abstract class ObjcRuleTestCase extends BuildViewTestCase { .containsAllOf(protosGroup0Lib, protosGroup1Lib, protosGroup2Lib, protosGroup3Lib); } - private void assertBundledCompilationUsesCrosstool(ConfiguredTarget topTarget) { - Artifact protoObjectA = - getBinArtifact("_objs/x/x/_generated_protos/x/protos/DataA.pbobjc.o", topTarget); - Artifact protoObjectB = - getBinArtifact("_objs/x/x/_generated_protos/x/protos/DataB.pbobjc.o", topTarget); - Artifact protoObjectC = - getBinArtifact("_objs/x/x/_generated_protos/x/protos/DataC.pbobjc.o", topTarget); - Artifact protoObjectD = - getBinArtifact("_objs/x/x/_generated_protos/x/protos/DataD.pbobjc.o", topTarget); - - assertThat(getGeneratingAction(protoObjectA)).isInstanceOf(CppCompileAction.class); - assertThat(getGeneratingAction(protoObjectB)).isInstanceOf(CppCompileAction.class); - assertThat(getGeneratingAction(protoObjectC)).isInstanceOf(CppCompileAction.class); - assertThat(getGeneratingAction(protoObjectD)).isInstanceOf(CppCompileAction.class); - } - protected void checkProtoBundlingDoesNotHappen(RuleType ruleType) throws Exception { scratch.file( "protos/BUILD", |