Don't have registerCompileAndArchiveActions depend on ObjcProvider
RELNOTES: None
PiperOrigin-RevId: 295897807
diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
index 2cca679..940f88a 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java
@@ -965,10 +965,12 @@
private final Artifact.DerivedArtifact picHeaderModule;
/** All header files that are compiled into this module. */
- private final ImmutableList<Artifact> modularHeaders;
+ // Note: this change will be reverted in the migration cl.
+ public final ImmutableList<Artifact> modularHeaders;
/** All header files that are contained in this module. */
- private final ImmutableList<Artifact> textualHeaders;
+ // Note: this change will be reverted in the migration cl.
+ public final ImmutableList<Artifact> textualHeaders;
HeaderInfo(
Artifact.DerivedArtifact headerModule,
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 59dab83..80a98e0 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
@@ -515,11 +515,25 @@
* This method returns null when a required SkyValue is missing and a Skyframe restart is
* required.
*/
+ // Note: this function will be deleted in the migration cl.
@Nullable
private static IncludeScanningHeaderData.Builder createIncludeScanningHeaderData(
- SkyFunction.Environment env, Iterable<Artifact> inputs) throws InterruptedException {
+ SkyFunction.Environment env,
+ List<CcCompilationContext.HeaderInfo> headerInfos,
+ Iterable<Artifact> inputs)
+ throws InterruptedException {
Map<PathFragment, Artifact> pathToLegalOutputArtifact = new HashMap<>();
ArrayList<Artifact> treeArtifacts = new ArrayList<>();
+ // Not using range-based for loops here and below as the additional overhead of the
+ // ImmutableList iterators has shown up in profiles.
+ for (CcCompilationContext.HeaderInfo headerInfo : headerInfos) {
+ for (Artifact a : headerInfo.modularHeaders) {
+ IncludeScanningHeaderDataHelper.handleArtifact(a, pathToLegalOutputArtifact, treeArtifacts);
+ }
+ for (Artifact a : headerInfo.textualHeaders) {
+ IncludeScanningHeaderDataHelper.handleArtifact(a, pathToLegalOutputArtifact, treeArtifacts);
+ }
+ }
for (Artifact a : inputs) {
IncludeScanningHeaderDataHelper.handleArtifact(a, pathToLegalOutputArtifact, treeArtifacts);
}
@@ -544,7 +558,7 @@
List<CcCompilationContext.HeaderInfo> headerInfo)
throws InterruptedException {
if (alternateIncludeScanningDataInputs != null) {
- return createIncludeScanningHeaderData(env, alternateIncludeScanningDataInputs);
+ return createIncludeScanningHeaderData(env, headerInfo, alternateIncludeScanningDataInputs);
} else {
return ccCompilationContext.createIncludeScanningHeaderData(
env, usePic, useHeaderModules, headerInfo);
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 9c2968b..1735084 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
@@ -164,7 +164,8 @@
.build();
compilationSupport
- .registerCompileAndArchiveActions(common.getCompilationArtifacts().get(), objcProvider)
+ .registerCompileAndArchiveActions(
+ common.getCompilationArtifacts().get(), ObjcCompilationContext.EMPTY)
.registerFullyLinkAction(
objcProvider, intermediateArtifacts.strippedSingleArchitectureLibrary())
.validateAttributes();
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 defc7a7..054bf65 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
@@ -61,7 +61,6 @@
import com.google.devtools.build.lib.analysis.FilesToRunProvider;
import com.google.devtools.build.lib.analysis.PrerequisiteArtifacts;
import com.google.devtools.build.lib.analysis.RuleContext;
-import com.google.devtools.build.lib.analysis.TransitiveInfoCollection;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine;
import com.google.devtools.build.lib.analysis.actions.CustomCommandLine.VectorArg;
import com.google.devtools.build.lib.analysis.actions.ParameterFileWriteAction;
@@ -87,7 +86,6 @@
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper;
import com.google.devtools.build.lib.rules.cpp.CcCompilationHelper.CompilationInfo;
import com.google.devtools.build.lib.rules.cpp.CcCompilationOutputs;
-import com.google.devtools.build.lib.rules.cpp.CcInfo;
import com.google.devtools.build.lib.rules.cpp.CcLinkingHelper;
import com.google.devtools.build.lib.rules.cpp.CcToolchain;
import com.google.devtools.build.lib.rules.cpp.CcToolchainFeatures.CollidingProvidesException;
@@ -275,7 +273,7 @@
}
private CompilationInfo compile(
- ObjcProvider objcProvider,
+ ObjcCompilationContext objcCompilationContext,
VariablesExtension extension,
ExtraCompileArgs extraCompileArgs,
CcToolchainProvider ccToolchain,
@@ -286,12 +284,11 @@
Collection<Artifact> publicHdrs,
Collection<Artifact> dependentGeneratedHdrs,
Artifact pchHdr,
- // TODO(b/70777494): Find out how deps get used and remove if not needed.
- Iterable<? extends TransitiveInfoCollection> deps,
ObjcCppSemantics semantics,
String purpose,
boolean generateModuleMap)
throws RuleErrorException, InterruptedException {
+ ObjcProvider depObjcProvidersSummary = objcCompilationContext.getDepObjcProvidersSummary();
CcCompilationHelper result =
new CcCompilationHelper(
ruleContext,
@@ -307,14 +304,21 @@
TargetUtils.getExecutionInfo(
ruleContext.getRule(), ruleContext.isAllowTagsPropagation()))
.addSources(sources)
- .addPrivateHeaders(privateHdrs)
- .addDefines(objcProvider.get(DEFINE))
.addPublicHeaders(publicHdrs)
+ .addPublicTextualHeaders(objcCompilationContext.getPublicTextualHeaders())
+ .addPrivateHeaders(privateHdrs)
.addPrivateHeadersUnchecked(dependentGeneratedHdrs)
- .addCcCompilationContexts(
- AnalysisUtils.getProviders(deps, CcInfo.PROVIDER).stream()
- .map(CcInfo::getCcCompilationContext)
- .collect(ImmutableList.toImmutableList()))
+ .addDefines(depObjcProvidersSummary.get(DEFINE))
+ .addDefines(objcCompilationContext.getDefines())
+ .addIncludeDirs(priorityHeaders)
+ .addIncludeDirs(objcCompilationContext.getIncludes())
+ .addIncludeDirs(depObjcProvidersSummary.get(INCLUDE))
+ .addSystemIncludeDirs(objcCompilationContext.getSystemIncludes())
+ .addSystemIncludeDirs(depObjcProvidersSummary.get(INCLUDE_SYSTEM))
+ .addQuoteIncludeDirs(objcCompilationContext.getQuoteIncludes())
+ .addQuoteIncludeDirs(depObjcProvidersSummary.get(IQUOTE))
+ .addFrameworkIncludeDirs(frameworkHeaderSearchPathFragments(depObjcProvidersSummary))
+ .addCcCompilationContexts(objcCompilationContext.getDepCcCompilationContexts())
.setCopts(
ImmutableList.<String>builder()
.addAll(getCompileRuleCopts())
@@ -324,15 +328,10 @@
.getCoptsForCompilationMode())
.addAll(extraCompileArgs)
.build())
- .addFrameworkIncludeDirs(frameworkHeaderSearchPathFragments(objcProvider))
- .addIncludeDirs(priorityHeaders)
- .addIncludeDirs(objcProvider.get(INCLUDE))
- .addSystemIncludeDirs(objcProvider.get(INCLUDE_SYSTEM))
.setCppModuleMap(intermediateArtifacts.moduleMap())
.setPropagateModuleMapToCompileAction(false)
.addVariableExtension(extension)
.setPurpose(purpose)
- .addQuoteIncludeDirs(objcProvider.get(IQUOTE))
.setCodeCoverageEnabled(CcCompilationHelper.isCodeCoverageEnabled(ruleContext))
.setHeadersCheckingMode(semantics.determineHeadersCheckingMode(ruleContext));
@@ -348,7 +347,7 @@
}
private Pair<CcCompilationOutputs, ImmutableMap<String, NestedSet<Artifact>>> ccCompileAndLink(
- ObjcProvider objcProvider,
+ ObjcCompilationContext objcCompilationContext,
CompilationArtifacts compilationArtifacts,
ObjcVariablesExtension.Builder extensionBuilder,
ExtraCompileArgs extraCompileArgs,
@@ -375,22 +374,23 @@
// CcCompilationContexts, but ObjcProvider does not propagate that. This issue will go away
// when we finish migrating the compile info in ObjcProvider to CcCompilationContext.
//
- // To limit the extra work we're adding, we only add what is required, i.e. the
- // generated headers.
+ // To limit the extra work we're adding, we only add what is required, i.e. the generated
+ // headers. Headers from own rule's attributes and from dependent CcCompilationContext are
+ // already accounted for, so we only need the ones from depObjcProviders.
Collection<Artifact> dependentGeneratedHdrs =
(includeProcessingType == IncludeProcessingType.INCLUDE_SCANNING)
- ? objcProvider.getGeneratedHeaderList()
+ ? objcCompilationContext.getDepObjcProvidersSummary().getGeneratedHeaderList()
: ImmutableList.of();
Artifact pchHdr = getPchFile().orNull();
- Iterable<? extends TransitiveInfoCollection> deps =
- ruleContext.getPrerequisites("deps", Mode.TARGET);
- ObjcCppSemantics semantics = createObjcCppSemantics(objcProvider, privateHdrs, pchHdr);
+ ObjcCppSemantics semantics =
+ createObjcCppSemantics(
+ objcCompilationContext.getDepObjcProvidersSummary(), privateHdrs, pchHdr);
String purpose = String.format("%s_objc_arc", semantics.getPurpose());
extensionBuilder.setArcEnabled(true);
CompilationInfo objcArcCompilationInfo =
compile(
- objcProvider,
+ objcCompilationContext,
extensionBuilder.build(),
extraCompileArgs,
ccToolchain,
@@ -401,7 +401,6 @@
publicHdrs,
dependentGeneratedHdrs,
pchHdr,
- deps,
semantics,
purpose,
/* generateModuleMap= */ true);
@@ -410,7 +409,7 @@
extensionBuilder.setArcEnabled(false);
CompilationInfo nonObjcArcCompilationInfo =
compile(
- objcProvider,
+ objcCompilationContext,
extensionBuilder.build(),
extraCompileArgs,
ccToolchain,
@@ -421,7 +420,6 @@
publicHdrs,
dependentGeneratedHdrs,
pchHdr,
- deps,
semantics,
purpose,
// Only generate the module map once (see above) and re-use it here.
@@ -940,16 +938,16 @@
* Registers all actions necessary to compile this rule's sources and archive them.
*
* @param compilationArtifacts collection of artifacts required for the compilation
- * @param objcProvider provides all compiling and linking information to register these actions
+ * @param objcCompilationContext provides the compiling information to register these actions
* @return this compilation support
* @throws RuleErrorException for invalid crosstool files
*/
CompilationSupport registerCompileAndArchiveActions(
- CompilationArtifacts compilationArtifacts, ObjcProvider objcProvider)
+ CompilationArtifacts compilationArtifacts, ObjcCompilationContext objcCompilationContext)
throws RuleErrorException, InterruptedException {
return registerCompileAndArchiveActions(
compilationArtifacts,
- objcProvider,
+ objcCompilationContext,
ExtraCompileArgs.NONE,
ImmutableList.<PathFragment>of());
}
@@ -985,7 +983,7 @@
* Registers all actions necessary to compile this rule's sources and archive them.
*
* @param compilationArtifacts collection of artifacts required for the compilation
- * @param objcProvider provides all compiling and linking information to register these actions
+ * @param objcCompilationContext provides the compiling information to register these actions
* @param extraCompileArgs args to be added to compile actions
* @param priorityHeaders priority headers to be included before the dependency headers
* @return this compilation support
@@ -993,7 +991,7 @@
*/
private CompilationSupport registerCompileAndArchiveActions(
CompilationArtifacts compilationArtifacts,
- ObjcProvider objcProvider,
+ ObjcCompilationContext objcCompilationContext,
ExtraCompileArgs extraCompileArgs,
List<PathFragment> priorityHeaders)
throws RuleErrorException, InterruptedException {
@@ -1015,7 +1013,7 @@
compilationInfo =
ccCompileAndLink(
- objcProvider,
+ objcCompilationContext,
compilationArtifacts,
extension,
extraCompileArgs,
@@ -1032,7 +1030,7 @@
} else {
compilationInfo =
ccCompileAndLink(
- objcProvider,
+ objcCompilationContext,
compilationArtifacts,
extension,
extraCompileArgs,
@@ -1064,7 +1062,7 @@
if (common.getCompilationArtifacts().isPresent()) {
registerCompileAndArchiveActions(
common.getCompilationArtifacts().get(),
- common.getObjcProvider(),
+ common.getObjcCompilationContext(),
extraCompileArgs,
priorityHeaders);
}
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 9c3df7d..3bb3156 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
@@ -180,7 +180,7 @@
.build();
compilationSupport
- .registerCompileAndArchiveActions(compilationArtifacts, objcProvider)
+ .registerCompileAndArchiveActions(compilationArtifacts, ObjcCompilationContext.EMPTY)
.registerLinkActions(
objcProvider,
j2ObjcMappingFileProvider,
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
index 1cd5e02..8b98f7d 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCommon.java
@@ -100,7 +100,6 @@
private ImmutableSet.Builder<Artifact> textualHeaders = ImmutableSet.builder();
private Iterable<ObjcProvider> depObjcProviders = ImmutableList.of();
private Iterable<ObjcProvider> runtimeDepObjcProviders = ImmutableList.of();
- private Iterable<String> defines = ImmutableList.of();
private Iterable<PathFragment> includes = ImmutableList.of();
private IntermediateArtifacts intermediateArtifacts;
private boolean alwayslink;
@@ -229,11 +228,6 @@
return this;
}
- public Builder addDefines(Iterable<String> defines) {
- this.defines = Iterables.concat(this.defines, defines);
- return this;
- }
-
Builder setIntermediateArtifacts(IntermediateArtifacts intermediateArtifacts) {
this.intermediateArtifacts = intermediateArtifacts;
return this;
@@ -273,30 +267,28 @@
ObjcCommon build() {
+ ObjcCompilationContext.Builder objcCompilationContextBuilder =
+ ObjcCompilationContext.builder(semantics);
+
ObjcProvider.Builder objcProvider =
new ObjcProvider.Builder(semantics)
.addAll(IMPORTED_LIBRARY, extraImportLibraries)
- .addAll(INCLUDE, includes)
- .add(IQUOTE, buildConfiguration.getGenfilesFragment())
- .addAll(DEFINE, defines)
- .addTransitiveAndPropagate(depObjcProviders);
+ .addTransitiveAndPropagateNonCompileInfo(depObjcProviders);
+
+ objcCompilationContextBuilder
+ .addIncludes(includes)
+ .addDepObjcProviders(depObjcProviders)
+ .addDepObjcProviders(runtimeDepObjcProviders)
+ .addDepCcCompilationContexts(depCcHeaderProviders);
+
+ for (CcCompilationContext headerProvider : depCcHeaderProviders) {
+ textualHeaders.addAll(headerProvider.getTextualHdrs());
+ }
for (ObjcProvider provider : runtimeDepObjcProviders) {
- objcProvider.addTransitiveAndPropagate(ObjcProvider.FRAMEWORK_SEARCH_PATHS, provider);
- objcProvider.addTransitiveAndPropagate(ObjcProvider.HEADER, provider);
objcProvider.addTransitiveAndPropagate(ObjcProvider.MERGE_ZIP, provider);
}
- for (CcCompilationContext headerProvider : depCcHeaderProviders) {
- objcProvider.addAll(HEADER, filterFileset(headerProvider.getDeclaredIncludeSrcs()));
- objcProvider.addAll(INCLUDE, headerProvider.getIncludeDirs());
- // TODO(bazel-team): This pulls in stl via
- // CppHelper.mergeToolchainDependentCcCompilationContext but
- // probably shouldn't.
- objcProvider.addAll(INCLUDE_SYSTEM, headerProvider.getSystemIncludeDirs());
- objcProvider.addAll(DEFINE, headerProvider.getDefines());
- textualHeaders.addAll(headerProvider.getTextualHdrs());
- }
for (CcLinkingContext linkProvider : depCcLinkProviders) {
ImmutableList<String> linkOpts = linkProvider.getFlattenedUserLinkFlags();
ImmutableSet.Builder<SdkFramework> frameworkLinkOpts = new ImmutableSet.Builder<>();
@@ -329,15 +321,16 @@
Iterables.transform(
attributes.sdkIncludes().toList(), (p) -> usrIncludeDir.getRelative(p));
objcProvider
- .addAll(HEADER, filterFileset(attributes.hdrs()))
- .addAll(HEADER, filterFileset(attributes.textualHdrs()))
- .addAll(DEFINE, attributes.defines())
- .addTransitiveAndPropagate(
- INCLUDE, attributes.headerSearchPaths(buildConfiguration.getGenfilesFragment()))
- .addAll(INCLUDE, sdkIncludes)
.addAll(SDK_FRAMEWORK, attributes.sdkFrameworks())
.addAll(WEAK_SDK_FRAMEWORK, attributes.weakSdkFrameworks())
.addAll(SDK_DYLIB, attributes.sdkDylibs());
+ objcCompilationContextBuilder
+ .addPublicHeaders(filterFileset(attributes.hdrs().toList()))
+ .addPublicTextualHeaders(filterFileset(attributes.textualHdrs().toList()))
+ .addDefines(attributes.defines())
+ .addIncludes(
+ attributes.headerSearchPaths(buildConfiguration.getGenfilesFragment()).toList())
+ .addIncludes(sdkIncludes);
}
for (CompilationArtifacts artifacts : compilationArtifacts.asSet()) {
@@ -346,9 +339,11 @@
// TODO(bazel-team): Add private headers to the provider when we have module maps to enforce
// them.
objcProvider
- .addAll(HEADER, filterFileset(artifacts.getAdditionalHdrs()))
.addAll(LIBRARY, artifacts.getArchive().asSet())
.addAll(SOURCE, allSources);
+ objcCompilationContextBuilder.addPublicHeaders(
+ filterFileset(artifacts.getAdditionalHdrs()));
+ objcCompilationContextBuilder.addPrivateHeaders(artifacts.getPrivateHdrs());
if (artifacts.getArchive().isPresent()
&& J2ObjcLibrary.J2OBJC_SUPPORTED_RULES.contains(context.getRule().getRuleClass())) {
@@ -388,7 +383,38 @@
objcProvider.addAll(LINKED_BINARY, linkedBinary.asSet());
- return new ObjcCommon(objcProvider.build(), compilationArtifacts, textualHeaders.build());
+ ObjcCompilationContext objcCompilationContext = objcCompilationContextBuilder.build();
+ addObjcCompilationContext(objcProvider, objcCompilationContext);
+
+ return new ObjcCommon(
+ objcProvider.build(),
+ objcCompilationContext,
+ compilationArtifacts,
+ textualHeaders.build());
+ }
+
+ private static void addObjcCompilationContext(
+ ObjcProvider.Builder objcProvider, ObjcCompilationContext objcCompilationContext) {
+ // No need to add frameworkIncludes because same info is used for linking so should
+ // already have been added.
+ objcProvider
+ .addAll(DEFINE, objcCompilationContext.getDefines())
+ .addAll(HEADER, objcCompilationContext.getPublicHeaders())
+ .addAll(HEADER, objcCompilationContext.getPublicTextualHeaders())
+ .addAll(INCLUDE, objcCompilationContext.getIncludes())
+ .addAll(INCLUDE_SYSTEM, objcCompilationContext.getSystemIncludes())
+ .addAll(IQUOTE, objcCompilationContext.getQuoteIncludes())
+ // We can't use the summary provider here because it would mess up the strict
+ // dependency and non-propagate items.
+ .addTransitiveAndPropagateCompileInfo(objcCompilationContext.getDepObjcProviders());
+ for (CcCompilationContext ccCompilationContext :
+ objcCompilationContext.getDepCcCompilationContexts()) {
+ objcProvider
+ .addAll(DEFINE, ccCompilationContext.getDefines())
+ .addAll(HEADER, ccCompilationContext.getDeclaredIncludeSrcs())
+ .addAll(INCLUDE, ccCompilationContext.getIncludeDirs())
+ .addAll(INCLUDE_SYSTEM, ccCompilationContext.getSystemIncludeDirs());
+ }
}
private static boolean isCcLibrary(ConfiguredTargetAndData info) {
@@ -408,15 +434,18 @@
}
private final ObjcProvider objcProvider;
+ private final ObjcCompilationContext objcCompilationContext;
private final Optional<CompilationArtifacts> compilationArtifacts;
private final ImmutableSet<Artifact> textualHdrs;
private ObjcCommon(
ObjcProvider objcProvider,
+ ObjcCompilationContext objcCompilationContext,
Optional<CompilationArtifacts> compilationArtifacts,
ImmutableSet<Artifact> textualHdrs) {
this.objcProvider = Preconditions.checkNotNull(objcProvider);
+ this.objcCompilationContext = Preconditions.checkNotNull(objcCompilationContext);
this.compilationArtifacts = Preconditions.checkNotNull(compilationArtifacts);
this.textualHdrs = textualHdrs;
}
@@ -425,6 +454,10 @@
return objcProvider;
}
+ public ObjcCompilationContext getObjcCompilationContext() {
+ return objcCompilationContext;
+ }
+
public Optional<CompilationArtifacts> getCompilationArtifacts() {
return compilationArtifacts;
}
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompilationContext.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompilationContext.java
new file mode 100644
index 0000000..774cca1
--- /dev/null
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcCompilationContext.java
@@ -0,0 +1,224 @@
+// Copyright 2020 The Bazel Authors. All rights reserved.
+//
+// Licensed under the Apache License, Version 2.0 (the "License");
+// you may not use this file except in compliance with the License.
+// You may obtain a copy of the License at
+//
+// http://www.apache.org/licenses/LICENSE-2.0
+//
+// Unless required by applicable law or agreed to in writing, software
+// distributed under the License is distributed on an "AS IS" BASIS,
+// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
+// See the License for the specific language governing permissions and
+// limitations under the License.
+
+package com.google.devtools.build.lib.rules.objc;
+
+import static com.google.devtools.build.lib.rules.objc.ObjcProvider.HEADER;
+
+import com.google.common.collect.ImmutableList;
+import com.google.common.collect.Iterables;
+import com.google.devtools.build.lib.actions.Artifact;
+import com.google.devtools.build.lib.concurrent.ThreadSafety.Immutable;
+import com.google.devtools.build.lib.rules.cpp.CcCompilationContext;
+import com.google.devtools.build.lib.syntax.StarlarkSemantics;
+import com.google.devtools.build.lib.vfs.PathFragment;
+import java.util.ArrayList;
+import java.util.List;
+
+/**
+ * A collection of compilation information gathered for a particular rule. This is used to generate
+ * the compilation command line and to the supply information that goes into the compilation info
+ * provider.
+ */
+@Immutable
+public final class ObjcCompilationContext {
+ public static final ObjcCompilationContext EMPTY =
+ builder(StarlarkSemantics.DEFAULT_SEMANTICS).build();
+
+ private final ImmutableList<String> defines;
+
+ /**
+ * The list of public headers. We expect this to contain both the headers from the src attribute,
+ * as well as any "additional" headers required for compilation.
+ */
+ private final ImmutableList<Artifact> publicHeaders;
+
+ private final ImmutableList<Artifact> publicTextualHeaders;
+ private final ImmutableList<Artifact> privateHeaders;
+ private final ImmutableList<PathFragment> includes;
+ private final ImmutableList<PathFragment> systemIncludes;
+ private final ImmutableList<PathFragment> quoteIncludes;
+ private final ImmutableList<ObjcProvider> depObjcProviders;
+ private final ImmutableList<CcCompilationContext> depCcCompilationContexts;
+ private final ObjcProvider depObjcProvidersSummary;
+
+ ObjcCompilationContext(
+ Iterable<String> defines,
+ Iterable<Artifact> publicHeaders,
+ Iterable<Artifact> publicTextualHeaders,
+ Iterable<Artifact> privateHeaders,
+ Iterable<PathFragment> includes,
+ Iterable<PathFragment> systemIncludes,
+ Iterable<PathFragment> quoteIncludes,
+ Iterable<ObjcProvider> depObjcProviders,
+ Iterable<CcCompilationContext> depCcCompilationContexts,
+ ObjcProvider depObjcProvidersSummary) {
+ this.defines = ImmutableList.copyOf(defines);
+ this.publicHeaders = ImmutableList.copyOf(publicHeaders);
+ this.publicTextualHeaders = ImmutableList.copyOf(publicTextualHeaders);
+ this.privateHeaders = ImmutableList.copyOf(privateHeaders);
+ this.includes = ImmutableList.copyOf(includes);
+ this.systemIncludes = ImmutableList.copyOf(systemIncludes);
+ this.quoteIncludes = ImmutableList.copyOf(quoteIncludes);
+ this.depObjcProviders = ImmutableList.copyOf(depObjcProviders);
+ this.depCcCompilationContexts = ImmutableList.copyOf(depCcCompilationContexts);
+ this.depObjcProvidersSummary = depObjcProvidersSummary;
+ }
+
+ public ImmutableList<String> getDefines() {
+ return defines;
+ }
+
+ public ImmutableList<Artifact> getPublicHeaders() {
+ return publicHeaders;
+ }
+
+ public ImmutableList<Artifact> getPublicTextualHeaders() {
+ return publicTextualHeaders;
+ }
+
+ public ImmutableList<Artifact> getPrivateHeaders() {
+ return privateHeaders;
+ }
+
+ public ImmutableList<PathFragment> getIncludes() {
+ return includes;
+ }
+
+ public ImmutableList<PathFragment> getSystemIncludes() {
+ return systemIncludes;
+ }
+
+ public ImmutableList<PathFragment> getQuoteIncludes() {
+ return quoteIncludes;
+ }
+
+ public ImmutableList<ObjcProvider> getDepObjcProviders() {
+ return depObjcProviders;
+ }
+
+ public ImmutableList<CcCompilationContext> getDepCcCompilationContexts() {
+ return depCcCompilationContexts;
+ }
+
+ public ObjcProvider getDepObjcProvidersSummary() {
+ return depObjcProvidersSummary;
+ }
+
+ // Note: this function will be deleted in the migration cl.
+ public ImmutableList<Artifact> getAllTransitiveHeaders() {
+ ImmutableList.Builder<Artifact> allHeaders = ImmutableList.builder();
+ allHeaders.addAll(publicHeaders);
+ allHeaders.addAll(publicTextualHeaders);
+ allHeaders.addAll(depObjcProvidersSummary.get(HEADER).toList());
+ for (CcCompilationContext ccCompilationContext : depCcCompilationContexts) {
+ allHeaders.addAll(ccCompilationContext.getDeclaredIncludeSrcs().toList());
+ }
+ return allHeaders.build();
+ }
+
+ public static Builder builder(StarlarkSemantics semantics) {
+ return new Builder(semantics);
+ }
+
+ static class Builder {
+ private final StarlarkSemantics semantics;
+ private final List<String> defines = new ArrayList<>();
+ private final List<Artifact> publicHeaders = new ArrayList<>();
+ private final List<Artifact> publicTextualHeaders = new ArrayList<>();
+ private final List<Artifact> privateHeaders = new ArrayList<>();
+ private final List<PathFragment> includes = new ArrayList<>();
+ private final List<PathFragment> systemIncludes = new ArrayList<>();
+ private final List<PathFragment> quoteIncludes = new ArrayList<>();
+ private final List<ObjcProvider> depObjcProviders = new ArrayList<>();
+ private final List<CcCompilationContext> depCcCompilationContexts = new ArrayList<>();
+
+ Builder(StarlarkSemantics semantics) {
+ this.semantics = semantics;
+ }
+
+ public Builder addDefines(Iterable<String> defines) {
+ Iterables.addAll(this.defines, defines);
+ return this;
+ }
+
+ public Builder addPublicHeaders(Iterable<Artifact> headers) {
+ Iterables.addAll(this.publicHeaders, headers);
+ return this;
+ }
+
+ public Builder addPublicTextualHeaders(Iterable<Artifact> headers) {
+ Iterables.addAll(this.publicTextualHeaders, headers);
+ return this;
+ }
+
+ public Builder addPrivateHeaders(Iterable<Artifact> headers) {
+ Iterables.addAll(this.privateHeaders, headers);
+ return this;
+ }
+
+ public Builder addIncludes(Iterable<PathFragment> includes) {
+ Iterables.addAll(this.includes, includes);
+ return this;
+ }
+
+ public Builder addSystemIncludes(Iterable<PathFragment> includes) {
+ Iterables.addAll(this.systemIncludes, includes);
+ return this;
+ }
+
+ public Builder addQuoteIncludes(Iterable<PathFragment> includes) {
+ Iterables.addAll(this.quoteIncludes, includes);
+ return this;
+ }
+
+ public Builder addDepObjcProviders(Iterable<ObjcProvider> objcProviders) {
+ // After migration, we will have nice embedded CcCompilationContexts that we can
+ // addDepCcCompilationContexts().
+ Iterables.addAll(this.depObjcProviders, objcProviders);
+ return this;
+ }
+
+ public Builder addDepCcCompilationContexts(
+ Iterable<CcCompilationContext> ccCompilationContexts) {
+ Iterables.addAll(this.depCcCompilationContexts, ccCompilationContexts);
+ return this;
+ }
+
+ public Builder addDepCcCompilationContext(CcCompilationContext ccCompilationContext) {
+ this.depCcCompilationContexts.add(ccCompilationContext);
+ return this;
+ }
+
+ ObjcCompilationContext build() {
+ ObjcProvider.Builder depObjcProvidersSummaryBuilder = new ObjcProvider.Builder(semantics);
+ for (ObjcProvider dep : depObjcProviders) {
+ depObjcProvidersSummaryBuilder.addTransitiveAndPropagateCompileInfo(dep);
+ }
+ ObjcProvider depObjcProvidersSummary = depObjcProvidersSummaryBuilder.build();
+
+ return new ObjcCompilationContext(
+ defines,
+ publicHeaders,
+ publicTextualHeaders,
+ privateHeaders,
+ includes,
+ systemIncludes,
+ quoteIncludes,
+ depObjcProviders,
+ depCcCompilationContexts,
+ depObjcProvidersSummary);
+ }
+ }
+}
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 408026d..6513106 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
@@ -98,15 +98,24 @@
if (includeProcessingType == NO_PROCESSING) {
// TODO(b/62060839): Identify the mechanism used to add generated headers in c++, and recycle
// it here.
+ //
+ // This is used to patch the action graph of missing dependencies. We only need to do this
+ // for headers in depObjcProviderCompileInfo. We can rid of this in the migraiton cl.
actionBuilder.addTransitiveMandatoryInputs(objcProvider.getGeneratedHeaders());
}
}
@Override
public NestedSet<Artifact> getAdditionalPrunableIncludes() {
+ // This is a hook used when header processing is off, to supply CppCompilationAction with
+ // additional inputs that are not properly added to CcCompilationContext. We will delete this
+ // in the migration cl.
return objcProvider.get(HEADER);
}
+ // This function now returns "additional" (not "alternate" include scanning data inputs, over
+ // those already accounted for in dependent CcCompilationContext. We can delete this in the
+ // migration cl so no point renaming it.
@Override
public Iterable<Artifact> getAlternateIncludeScanningDataInputs() {
// Include scanning data only cares about generated artifacts. Since the generated headers from
diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
index 07468c7..c373885 100644
--- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
+++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcProvider.java
@@ -341,6 +341,11 @@
// Lazily initialized because it's only needed for including scanning.
@Nullable private volatile ImmutableList<Artifact> generatedHeaderList;
+ /** All keys in ObjProvider corresponding to information needed for compile actions. */
+ static final ImmutableSet<Key<?>> KEYS_FOR_COMPILE_INFO =
+ ImmutableSet.<Key<?>>of(
+ DEFINE, FRAMEWORK_SEARCH_PATHS, HEADER, INCLUDE, INCLUDE_SYSTEM, IQUOTE);
+
/** All keys in ObjcProvider that will be passed in the corresponding Skylark provider. */
static final ImmutableList<Key<?>> KEYS_FOR_SKYLARK =
ImmutableList.<Key<?>>of(
@@ -1084,6 +1089,68 @@
return this;
}
+ // The following CompileInfo/NonCompileInfo family of methods will be deleted in the migration
+ // CL.
+
+ /**
+ * Add compile info from providers, and propagate it to any (transitive) dependers on this
+ * ObjcProvider.
+ */
+ public Builder addTransitiveAndPropagateCompileInfo(Iterable<ObjcProvider> providers) {
+ for (ObjcProvider provider : providers) {
+ addTransitiveAndPropagateCompileInfo(provider);
+ }
+ return this;
+ }
+
+ /**
+ * Add compile info from provider, and propagate it to any (transitive) dependers on this
+ * ObjcProvider.
+ */
+ public Builder addTransitiveAndPropagateCompileInfo(ObjcProvider provider) {
+ for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.items.entrySet()) {
+ if (KEYS_FOR_COMPILE_INFO.contains(typeEntry.getKey())) {
+ uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.items);
+ }
+ }
+ for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.strictDependencyItems.entrySet()) {
+ if (KEYS_FOR_COMPILE_INFO.contains(typeEntry.getKey())) {
+ uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.nonPropagatedItems);
+ }
+ }
+ return this;
+ }
+
+ /**
+ * Add non-compile info from providers, and propagate it to any (transitive) dependers on this
+ * ObjcProvider.
+ */
+ public Builder addTransitiveAndPropagateNonCompileInfo(Iterable<ObjcProvider> providers) {
+ for (ObjcProvider provider : providers) {
+ addTransitiveAndPropagateNonCompileInfo(provider);
+ }
+ return this;
+ }
+
+ /**
+ * Add non-compile info from provider, and propagate it to any (transitive) dependers on this
+ * ObjcProvider.
+ */
+ public Builder addTransitiveAndPropagateNonCompileInfo(ObjcProvider provider) {
+ for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.items.entrySet()) {
+ if (!KEYS_FOR_COMPILE_INFO.contains(typeEntry.getKey())) {
+ uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.items);
+ }
+ }
+ for (Map.Entry<Key<?>, NestedSet<?>> typeEntry : provider.strictDependencyItems.entrySet()) {
+ if (!KEYS_FOR_COMPILE_INFO.contains(typeEntry.getKey())) {
+ uncheckedAddTransitive(typeEntry.getKey(), typeEntry.getValue(), this.nonPropagatedItems);
+ }
+ }
+ return this;
+ }
+
+ /** Return an EvalException for having a bad key in the direct dependency provider. */
private static <E> EvalException badDirectDependencyKeyError(Key<E> key) {
return new EvalException(
null,
diff --git a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
index 1bcb22d..aa0ac0d 100644
--- a/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
+++ b/src/test/java/com/google/devtools/build/lib/rules/objc/BazelJ2ObjcLibraryTest.java
@@ -1194,12 +1194,12 @@
getActionGraph().getGeneratingAction(objectFilesFromGenJar);
Artifact sourceFilesFromGenJar =
getFirstArtifactEndingWith(actionTemplate.getInputs(), "source_files");
- Artifact headerFilesFromGenJar =
- getFirstArtifactEndingWith(actionTemplate.getInputs(), "header_files");
assertThat(sourceFilesFromGenJar.getRootRelativePathString())
.isEqualTo("java/com/google/app/test/_j2objc/src_jar_files/test/source_files");
- assertThat(headerFilesFromGenJar.getRootRelativePathString())
- .isEqualTo("java/com/google/app/test/_j2objc/src_jar_files/test/header_files");
+ // We can't easily access the header artifact through the middleman artifact, so use its
+ // expected path directly.
+ String headerFilesFromGenJar =
+ "java/com/google/app/test/_j2objc/src_jar_files/test/header_files";
// The files contained inside the tree artifacts are not known until execution time.
// Therefore we need to fake some files inside them to test the action template in this
@@ -1240,7 +1240,7 @@
.add("-I")
.add(binFragment + "/java/com/google/app/test/_j2objc/test")
.add("-I")
- .add(headerFilesFromGenJar.getExecPathString())
+ .add(headerFilesFromGenJar)
.add("-fno-strict-overflow")
.add("-fno-objc-arc")
.add("-c")
