From 21a28cf0c59a18ed7e6f217af967c3fa738f31c9 Mon Sep 17 00:00:00 2001 From: Oscar Bonilla <6f6231@gmail.com> Date: Tue, 21 Aug 2018 14:59:08 -0700 Subject: [PATCH 1/6] WIP: objc include_prefix/strip_include_prefix --- .../lib/rules/cpp/CcCompilationHelper.java | 15 ++++- .../lib/rules/objc/CompilationAttributes.java | 48 ++++++++++++++++ .../lib/rules/objc/CompilationSupport.java | 8 +++ .../build/lib/rules/objc/ObjcRuleClasses.java | 34 +++++++++++ src/test/shell/bazel/apple/bazel_objc_test.sh | 57 +++++++++++++++++++ 5 files changed, 161 insertions(+), 1 deletion(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 22ddbbfddf80e1..e20d826dd25b2e 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -412,7 +412,7 @@ private CcCompilationHelper addPrivateHeader(Artifact privateHeader, Label label compilationUnitSources.put( privateHeader, CppSource.create(privateHeader, label, CppSource.Type.HEADER)); } - + this.privateHeaders.add(privateHeader); return this; } @@ -741,6 +741,13 @@ public CompilationInfo compile() throws RuleErrorException { } ccCompilationContext = initializeCcCompilationContext(); + System.out.println("OB DEBUG: Rule: " + + ruleContext.attributes().getName() + + " created ccCompilationContext = " + + ccCompilationContext.toString() + + " including " + + ccCompilationContext.getIncludeDirs().toString() + ); boolean compileHeaderModules = featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES); Preconditions.checkState( @@ -950,6 +957,12 @@ private CcCompilationContext initializeCcCompilationContext() { PublicHeaders publicHeaders = computePublicHeaders(); if (publicHeaders.getVirtualIncludePath() != null) { ccCompilationContextBuilder.addIncludeDir(publicHeaders.getVirtualIncludePath()); + System.out.println("OB DEBUG: rule " + + ruleContext.attributes().getName() + + " added " + + publicHeaders.getVirtualIncludePath().toString() + + " to CcCompilationContext.Builder" + ); } if (useDeps) { diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationAttributes.java b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationAttributes.java index 8bac4e661acedf..7f4820171df863 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationAttributes.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/CompilationAttributes.java @@ -55,6 +55,8 @@ static class Builder { private final NestedSetBuilder weakSdkFrameworks = NestedSetBuilder.stableOrder(); private final NestedSetBuilder sdkDylibs = NestedSetBuilder.stableOrder(); private Optional packageFragment = Optional.absent(); + private String includePrefix = null; + private String stripIncludePrefix = null; private boolean enableModules; /** @@ -173,6 +175,22 @@ public Builder enableModules() { return this; } + /** + * Sets the include_prefix attribute. + */ + public Builder setIncludePrefix(String includePrefix) { + this.includePrefix = includePrefix; + return this; + } + + /** + * Sets the strip_include_prefix attribute. + */ + public Builder setStripIncludePrefix(String stripIncludePrefix) { + this.stripIncludePrefix = stripIncludePrefix; + return this; + } + /** * Builds a {@code CompilationAttributes} object. */ @@ -189,6 +207,8 @@ public CompilationAttributes build() { this.copts.build(), this.linkopts.build(), this.moduleMapsForDirectDeps.build(), + this.includePrefix, + this.stripIncludePrefix, this.enableModules); } @@ -224,6 +244,18 @@ private static void addIncludesFromRuleContext(Builder builder, RuleContext rule PathFragment::create)); builder.addSdkIncludes(sdkIncludes.build()); } + + if (ruleContext.getRule().isAttrDefined("include_prefix", Type.STRING) + && ruleContext.attributes().isAttributeValueExplicitlySpecified("include_prefix")) { + builder.setIncludePrefix( + ruleContext.attributes().get("include_prefix", Type.STRING)); + } + + if (ruleContext.getRule().isAttrDefined("strip_include_prefix", Type.STRING) + && ruleContext.attributes().isAttributeValueExplicitlySpecified("strip_include_prefix")) { + builder.setStripIncludePrefix( + ruleContext.attributes().get("strip_include_prefix", Type.STRING)); + } } private static void addSdkAttributesFromRuleContext(Builder builder, RuleContext ruleContext) { @@ -302,6 +334,8 @@ private static void addModuleOptionsFromRuleContext(Builder builder, RuleContext private final ImmutableList copts; private final ImmutableList linkopts; private final NestedSet moduleMapsForDirectDeps; + private final String includePrefix; + private final String stripIncludePrefix; private final boolean enableModules; private CompilationAttributes( @@ -316,6 +350,8 @@ private CompilationAttributes( ImmutableList copts, ImmutableList linkopts, NestedSet moduleMapsForDirectDeps, + String includePrefix, + String stripIncludePrefix, boolean enableModules) { this.hdrs = hdrs; this.textualHdrs = textualHdrs; @@ -328,6 +364,8 @@ private CompilationAttributes( this.copts = copts; this.linkopts = linkopts; this.moduleMapsForDirectDeps = moduleMapsForDirectDeps; + this.includePrefix = includePrefix; + this.stripIncludePrefix = stripIncludePrefix; this.enableModules = enableModules; } @@ -424,6 +462,16 @@ public NestedSet moduleMapsForDirectDeps() { return this.moduleMapsForDirectDeps; } + /** + * Returns the include_prefix attribute + */ + public String getIncludePrefix() { return this.includePrefix; } + + /** + * Returns the strip_include_prefix attribute + */ + public String getStripIncludePrefix() { return this.stripIncludePrefix; } + /** * Returns whether this target uses language features that require clang modules, such as * {@literal @}import. 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 5ef84aba9b088c..d697cb1390403f 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 @@ -307,6 +307,8 @@ private CompilationInfo compile( Collection privateHdrs, Collection publicHdrs, Artifact pchHdr, + String includePrefix, + String stripIncludePrefix, // TODO(b/70777494): Find out how deps get used and remove if not needed. Iterable deps, ObjcCppSemantics semantics, @@ -341,6 +343,8 @@ private CompilationInfo compile( .getCoptsForCompilationMode()) .addAll(extraCompileArgs) .build()) + .setIncludePrefix(includePrefix) + .setStripIncludePrefix(stripIncludePrefix) .addIncludeDirs(priorityHeaders) .addIncludeDirs(objcProvider.get(INCLUDE)) .addSystemIncludeDirs(objcProvider.get(INCLUDE_SYSTEM)) @@ -405,6 +409,8 @@ private CompilationInfo compile( privateHdrs, publicHdrs, pchHdr, + attributes.getIncludePrefix(), + attributes.getStripIncludePrefix(), deps, semantics, purpose); @@ -424,6 +430,8 @@ private CompilationInfo compile( privateHdrs, publicHdrs, pchHdr, + attributes.getIncludePrefix(), + attributes.getStripIncludePrefix(), deps, semantics, purpose); diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java index c17fa0692650b3..8f8a2fb5c596ce 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java @@ -684,6 +684,40 @@ Enables clang module support (via -fmodules). provided module map to the compiler. */ .add(attr("module_map", LABEL).allowedFileTypes(FileType.of(".modulemap"))) + + /* + The prefix to strip from the paths of the headers of this rule. + +

When set, the headers in the hdrs attribute of this rule are accessible + at their path with this prefix cut off. + +

If it's a relative path, it's taken as a package-relative one. If it's an absolute one, + it's understood as a repository-relative path. + +

The prefix in the include_prefix attribute is added after this prefix is + stripped. + */ + .add(attr("strip_include_prefix", STRING)) + /* + The prefix to add to the paths of the headers of this rule. + +

When set, the headers in the hdrs attribute of this rule are accessible + at is the value of this attribute prepended to their repository-relative path. + +

The prefix in the strip_include_prefix attribute is removed before this + prefix is added. + */ + .add(attr("include_prefix", STRING)) + + /* + When flatten virtual headers is set, we add all headers to the headermap. + + This allows a user to include a Header, "path/to/header" as "header.h" + + Together with include_prefix, this allows bazel to support Xcode style includes. + */ + .add(attr("flatten_virtual_headers", BOOLEAN)) + /* Provides the label for header_scanner tool that is used to scan inclusions for ObjC sources and provide a list of required headers via a .header_list file. diff --git a/src/test/shell/bazel/apple/bazel_objc_test.sh b/src/test/shell/bazel/apple/bazel_objc_test.sh index 09dd0536ff800d..19aa456c0d9bc1 100755 --- a/src/test/shell/bazel/apple/bazel_objc_test.sh +++ b/src/test/shell/bazel/apple/bazel_objc_test.sh @@ -117,4 +117,61 @@ EOF fail "Timestamp of contents of archive file should be zero" } +function test_objc_library_include_prefix_external_repository() { + r="$TEST_TMPDIR/r" + mkdir -p "$TEST_TMPDIR/r/foo/v1" + touch "$TEST_TMPDIR/r/WORKSPACE" + echo "#define FOO 42" > "$TEST_TMPDIR/r/foo/v1/foo.h" + cat > "$TEST_TMPDIR/r/foo/BUILD" < WORKSPACE < BUILD < ok.m < +#include "foolib/foo.h" +int main() { + printf("FOO is %d\n", FOO); +} +EOF + + cat > bad.m < +#include "foo/v1/foo.h" +int main() { + printf("FOO is %d\n", FOO); +} +EOF + + bazel build --sandbox_debug --verbose_failures -s :bad && fail "Should not have found include at repository-relative path" + bazel build --sandbox_debug --verbose_failures -s :ok || fail "Should have found include at synthetic path" +} + + run_suite "objc/ios test suite" From 83ad4b76e1a25eb3403b0bbf329624db9587a732 Mon Sep 17 00:00:00 2001 From: Oscar Bonilla <6f6231@gmail.com> Date: Tue, 21 Aug 2018 15:25:10 -0700 Subject: [PATCH 2/6] Remove diffs committed by mistake --- .../devtools/build/lib/rules/objc/ObjcRuleClasses.java | 9 --------- 1 file changed, 9 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java index 8f8a2fb5c596ce..c5850982c60ad7 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcRuleClasses.java @@ -709,15 +709,6 @@ Enables clang module support (via -fmodules). */ .add(attr("include_prefix", STRING)) - /* - When flatten virtual headers is set, we add all headers to the headermap. - - This allows a user to include a Header, "path/to/header" as "header.h" - - Together with include_prefix, this allows bazel to support Xcode style includes. - */ - .add(attr("flatten_virtual_headers", BOOLEAN)) - /* Provides the label for header_scanner tool that is used to scan inclusions for ObjC sources and provide a list of required headers via a .header_list file. From 7e56fad1f6936b2b53e0c8e36a9548e266699584 Mon Sep 17 00:00:00 2001 From: Oscar Bonilla <6f6231@gmail.com> Date: Wed, 22 Aug 2018 08:35:55 -0700 Subject: [PATCH 3/6] Snap: this works --- .../build/lib/rules/cpp/CcCompilationContext.java | 9 +++++++++ .../build/lib/rules/cpp/CcCompilationHelper.java | 1 + .../build/lib/rules/objc/CompilationSupport.java | 8 +++++++- .../build/lib/rules/objc/ObjcLibrary.java | 15 +++------------ 4 files changed, 20 insertions(+), 13 deletions(-) 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 0e0553949aaba9..0594a9145aeaa0 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 @@ -402,6 +402,10 @@ public Builder mergeDependentCcCompilationContext( compilationPrerequisites.addAll( otherCcCompilationContext.getTransitiveCompilationPrerequisites()); includeDirs.addAll(otherCcCompilationContext.getIncludeDirs()); + System.out.println("OB DEBUG: \t" + + "adding " + + otherCcCompilationContext.getIncludeDirs().toString()); + System.out.println("OB DEBUG: includeDirs is now:\n\t" + includeDirs.toString()); quoteIncludeDirs.addAll(otherCcCompilationContext.getQuoteIncludeDirs()); systemIncludeDirs.addAll(otherCcCompilationContext.getSystemIncludeDirs()); declaredIncludeDirs.addTransitive(otherCcCompilationContext.getDeclaredIncludeDirs()); @@ -435,6 +439,11 @@ public Builder mergeDependentCcCompilationContext( */ public Builder mergeDependentCcCompilationContexts(Iterable targets) { for (CcCompilationContext target : targets) { + System.out.println("OB DEBUG: merging " + + target.toString() + + " into " + + this.toString() + ); mergeDependentCcCompilationContext(target); } return this; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index e20d826dd25b2e..1a2150db6aaa1f 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -1054,6 +1054,7 @@ private CcCompilationContext initializeCcCompilationContext() { ccCompilationContextBuilder.setPurpose(purpose); semantics.setupCcCompilationContext(ruleContext, ccCompilationContextBuilder); + System.out.println("OB DEBUG: " + ccCompilationContextBuilder.toString() + " returning"); return ccCompilationContextBuilder.build(); } 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 d697cb1390403f..bf35c33f160eec 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 @@ -472,8 +472,11 @@ private CompilationInfo compile( compilationOutputsBuilder.merge(objcArcCompilationInfo.getCcCompilationOutputs()); compilationOutputsBuilder.merge(nonObjcArcCompilationInfo.getCcCompilationOutputs()); + // OB TODO: This mergedCcCompilationContext is what needs to be passed to ObjcLibrary.java + CcCompilationContext mergedCcCompilationContext = ccCompilationContextBuilder.build(); LinkingInfo linkingInfo = - resultLink.link(compilationOutputsBuilder.build(), ccCompilationContextBuilder.build()); + resultLink.link(compilationOutputsBuilder.build(), mergedCcCompilationContext); + ccCompilationContext = mergedCcCompilationContext; Map> mergedOutputGroups = CcCommon.mergeOutputGroups( @@ -698,6 +701,7 @@ static Iterable commonFrameworkNames( private final Map> outputGroupCollector; private final ImmutableList.Builder objectFilesCollector; private final CcToolchainProvider toolchain; + private CcCompilationContext ccCompilationContext; private final boolean isTestRule; private final boolean usePch; @@ -751,6 +755,8 @@ private CompilationSupport( } } + public CcCompilationContext getCcCompilationContext() { return this.ccCompilationContext; } + /** Builder for {@link CompilationSupport} */ public static class Builder { private RuleContext ruleContext; diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java index 42741b573b4a1f..67568692b0cc97 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java @@ -30,6 +30,7 @@ import com.google.devtools.build.lib.rules.cpp.CcLinkingInfo; import com.google.devtools.build.lib.rules.objc.ObjcCommon.ResourceAttributes; import com.google.devtools.build.lib.syntax.Type; +import com.google.devtools.build.lib.vfs.PathFragment; import java.util.Map; import java.util.TreeMap; @@ -91,19 +92,9 @@ public ConfiguredTarget create(RuleContext ruleContext) J2ObjcEntryClassProvider j2ObjcEntryClassProvider = new J2ObjcEntryClassProvider.Builder() .addTransitive(ruleContext.getPrerequisites("deps", Mode.TARGET, J2ObjcEntryClassProvider.class)).build(); - CcCompilationContext ccCompilationContext = - new CcCompilationContext.Builder(ruleContext) - .addDeclaredIncludeSrcs( - CompilationAttributes.Builder.fromRuleContext(ruleContext) - .build() - .hdrs() - .toCollection()) - .addTextualHdrs(common.getTextualHdrs()) - .addDeclaredIncludeSrcs(common.getTextualHdrs()) - .build(); - + CcCompilationContext mergedCcCompilationContext = compilationSupport.getCcCompilationContext(); CcCompilationInfo.Builder ccCompilationInfoBuilder = CcCompilationInfo.Builder.create(); - ccCompilationInfoBuilder.setCcCompilationContext(ccCompilationContext); + ccCompilationInfoBuilder.setCcCompilationContext(mergedCcCompilationContext); CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create(); ccLinkingInfoBuilder.setCcLinkParamsStore( From 64c4ac81e3a75fe5f8a6fdcb70fac57ddcbb7357 Mon Sep 17 00:00:00 2001 From: Oscar Bonilla <6f6231@gmail.com> Date: Mon, 27 Aug 2018 11:31:35 -0700 Subject: [PATCH 4/6] Remove debugging --- .../build/lib/rules/cpp/CcCompilationContext.java | 9 --------- .../build/lib/rules/cpp/CcCompilationHelper.java | 7 ------- 2 files changed, 16 deletions(-) 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 0594a9145aeaa0..0e0553949aaba9 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 @@ -402,10 +402,6 @@ public Builder mergeDependentCcCompilationContext( compilationPrerequisites.addAll( otherCcCompilationContext.getTransitiveCompilationPrerequisites()); includeDirs.addAll(otherCcCompilationContext.getIncludeDirs()); - System.out.println("OB DEBUG: \t" - + "adding " - + otherCcCompilationContext.getIncludeDirs().toString()); - System.out.println("OB DEBUG: includeDirs is now:\n\t" + includeDirs.toString()); quoteIncludeDirs.addAll(otherCcCompilationContext.getQuoteIncludeDirs()); systemIncludeDirs.addAll(otherCcCompilationContext.getSystemIncludeDirs()); declaredIncludeDirs.addTransitive(otherCcCompilationContext.getDeclaredIncludeDirs()); @@ -439,11 +435,6 @@ public Builder mergeDependentCcCompilationContext( */ public Builder mergeDependentCcCompilationContexts(Iterable targets) { for (CcCompilationContext target : targets) { - System.out.println("OB DEBUG: merging " - + target.toString() - + " into " - + this.toString() - ); mergeDependentCcCompilationContext(target); } return this; diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 1a2150db6aaa1f..35b5961ef81938 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -741,13 +741,6 @@ public CompilationInfo compile() throws RuleErrorException { } ccCompilationContext = initializeCcCompilationContext(); - System.out.println("OB DEBUG: Rule: " - + ruleContext.attributes().getName() - + " created ccCompilationContext = " - + ccCompilationContext.toString() - + " including " - + ccCompilationContext.getIncludeDirs().toString() - ); boolean compileHeaderModules = featureConfiguration.isEnabled(CppRuleClasses.HEADER_MODULES); Preconditions.checkState( From 745b3b153799082ed31ffec51b19c4331bff7a2f Mon Sep 17 00:00:00 2001 From: Oscar Bonilla <6f6231@gmail.com> Date: Tue, 28 Aug 2018 09:00:36 -0700 Subject: [PATCH 5/6] Clean up debugging --- .../devtools/build/lib/rules/cpp/CcCompilationHelper.java | 7 ------- .../devtools/build/lib/rules/objc/CompilationSupport.java | 1 - 2 files changed, 8 deletions(-) diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index 35b5961ef81938..fe87253951a109 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -950,12 +950,6 @@ private CcCompilationContext initializeCcCompilationContext() { PublicHeaders publicHeaders = computePublicHeaders(); if (publicHeaders.getVirtualIncludePath() != null) { ccCompilationContextBuilder.addIncludeDir(publicHeaders.getVirtualIncludePath()); - System.out.println("OB DEBUG: rule " - + ruleContext.attributes().getName() - + " added " - + publicHeaders.getVirtualIncludePath().toString() - + " to CcCompilationContext.Builder" - ); } if (useDeps) { @@ -1047,7 +1041,6 @@ private CcCompilationContext initializeCcCompilationContext() { ccCompilationContextBuilder.setPurpose(purpose); semantics.setupCcCompilationContext(ruleContext, ccCompilationContextBuilder); - System.out.println("OB DEBUG: " + ccCompilationContextBuilder.toString() + " returning"); return ccCompilationContextBuilder.build(); } 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 bf35c33f160eec..914ed2d8173f49 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 @@ -472,7 +472,6 @@ private CompilationInfo compile( compilationOutputsBuilder.merge(objcArcCompilationInfo.getCcCompilationOutputs()); compilationOutputsBuilder.merge(nonObjcArcCompilationInfo.getCcCompilationOutputs()); - // OB TODO: This mergedCcCompilationContext is what needs to be passed to ObjcLibrary.java CcCompilationContext mergedCcCompilationContext = ccCompilationContextBuilder.build(); LinkingInfo linkingInfo = resultLink.link(compilationOutputsBuilder.build(), mergedCcCompilationContext); From 8d0d2e06c49a4ba3c9b06e10ddfcfb88b4c48b49 Mon Sep 17 00:00:00 2001 From: Oscar Bonilla <6f6231@gmail.com> Date: Tue, 28 Aug 2018 09:33:24 -0700 Subject: [PATCH 6/6] Add a way to avoid merging private headers --- .../lib/rules/cpp/CcCompilationContext.java | 8 +++++--- .../lib/rules/cpp/CcCompilationHelper.java | 5 +++-- .../build/lib/rules/objc/ObjcLibrary.java | 18 ++++++++++++++++-- 3 files changed, 24 insertions(+), 7 deletions(-) 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 0e0553949aaba9..ab4c34d606f32d 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 @@ -397,7 +397,7 @@ public String getPurpose() { * of all of its attributes. */ public Builder mergeDependentCcCompilationContext( - CcCompilationContext otherCcCompilationContext) { + CcCompilationContext otherCcCompilationContext, boolean mergeDelcaredIncludeSrcs) { Preconditions.checkNotNull(otherCcCompilationContext); compilationPrerequisites.addAll( otherCcCompilationContext.getTransitiveCompilationPrerequisites()); @@ -405,7 +405,9 @@ public Builder mergeDependentCcCompilationContext( quoteIncludeDirs.addAll(otherCcCompilationContext.getQuoteIncludeDirs()); systemIncludeDirs.addAll(otherCcCompilationContext.getSystemIncludeDirs()); declaredIncludeDirs.addTransitive(otherCcCompilationContext.getDeclaredIncludeDirs()); - declaredIncludeSrcs.addTransitive(otherCcCompilationContext.getDeclaredIncludeSrcs()); + if (mergeDelcaredIncludeSrcs) { + declaredIncludeSrcs.addTransitive(otherCcCompilationContext.getDeclaredIncludeSrcs()); + } transitiveHeaderInfo.addTransitive(otherCcCompilationContext.transitiveHeaderInfos); transitiveModules.addTransitive(otherCcCompilationContext.transitiveModules); if (otherCcCompilationContext.headerInfo.headerModule != null) { @@ -435,7 +437,7 @@ public Builder mergeDependentCcCompilationContext( */ public Builder mergeDependentCcCompilationContexts(Iterable targets) { for (CcCompilationContext target : targets) { - mergeDependentCcCompilationContext(target); + mergeDependentCcCompilationContext(target, /* mergeDeclaredIncludeSrcs= */ true); } return this; } diff --git a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java index fe87253951a109..e3eb104e6b178c 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java +++ b/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationHelper.java @@ -2046,12 +2046,13 @@ private static void mergeToolchainDependentCcCompilationContext( "Unable to merge the STL '" + stl.getLabel() + "' and toolchain contexts"); return; } - ccCompilationContextBuilder.mergeDependentCcCompilationContext(ccCompilationContext); + ccCompilationContextBuilder.mergeDependentCcCompilationContext(ccCompilationContext, + /* mergeDeclaredIncludeSrcs= */ true); } } if (toolchain != null) { ccCompilationContextBuilder.mergeDependentCcCompilationContext( - toolchain.getCcCompilationContext()); + toolchain.getCcCompilationContext(), /* mergeDeclaredIncludeSrcs= */ true); } } } diff --git a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java index 67568692b0cc97..1ecf77fa54201d 100644 --- a/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java +++ b/src/main/java/com/google/devtools/build/lib/rules/objc/ObjcLibrary.java @@ -93,8 +93,22 @@ public ConfiguredTarget create(RuleContext ruleContext) .addTransitive(ruleContext.getPrerequisites("deps", Mode.TARGET, J2ObjcEntryClassProvider.class)).build(); CcCompilationContext mergedCcCompilationContext = compilationSupport.getCcCompilationContext(); - CcCompilationInfo.Builder ccCompilationInfoBuilder = CcCompilationInfo.Builder.create(); - ccCompilationInfoBuilder.setCcCompilationContext(mergedCcCompilationContext); + + CcCompilationContext ccCompilationContext = + new CcCompilationContext.Builder(ruleContext) + .addDeclaredIncludeSrcs( + CompilationAttributes.Builder.fromRuleContext(ruleContext) + .build() + .hdrs() + .toCollection()) + .addTextualHdrs(common.getTextualHdrs()) + .addDeclaredIncludeSrcs(common.getTextualHdrs()) + .mergeDependentCcCompilationContext(mergedCcCompilationContext, + /* mergeDeclaredIncludeSrcs= */ false) + .build(); + + CcCompilationInfo.Builder ccCompilationInfoBuilder = CcCompilationInfo.Builder.create(); + ccCompilationInfoBuilder.setCcCompilationContext(ccCompilationContext); CcLinkingInfo.Builder ccLinkingInfoBuilder = CcLinkingInfo.Builder.create(); ccLinkingInfoBuilder.setCcLinkParamsStore(