WIP: objc include_prefix/strip_include_prefix by ob · Pull Request #5954 · bazelbuild/bazel

ob · 2018-08-21T22:09:46Z

As per this design document, I am adding include_prefix and strip_include_prefix attributes to the objc_library rule.

However, I'm noticing a few interesting things regarding the objc_library behavior when compared to the cc_library behavior.

I pulled the test cases from the integration tests, namely these files, and when I run them, I see the following output:

for bazel_objc_test.sh

OB DEBUG: Rule: stl created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@637ea676 including []
OB DEBUG: Rule: malloc created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@3a108c15 including []
OB DEBUG: Rule: blaze_exit_code created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@649358ad including []
OB DEBUG: Rule: port created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@54876885 including []
OB DEBUG: Rule: strings created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@6278b2ca including []
OB DEBUG: Rule: logging created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@7ab8efc2 including []
OB DEBUG: Rule: errors created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@3fc74d43 including []
OB DEBUG: Rule: filesystem created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@1425ed22 including []
OB DEBUG: Rule: util created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@ce60254 including []
OB DEBUG: Rule: data_parser created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@66a090e4 including []
OB DEBUG: Rule: launcher_base created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@34dffc00 including []
OB DEBUG: Rule: python_launcher created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@16e4a40c including []
OB DEBUG: Rule: java_launcher created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@520b04a8 including []
OB DEBUG: Rule: bash_launcher created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@2399c60b including []
OB DEBUG: Rule: launcher created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@799ac0e including []
OB DEBUG: rule foo added bazel-out/ios_x86_64-fastbuild/bin/external/foo/foo/_virtual_includes/foo to CcCompilationContext.Builder
OB DEBUG: Rule: foo created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@4b3f30f4 including [bazel-out/ios_x86_64-fastbuild/bin/external/foo/foo/_virtual_includes/foo]
OB DEBUG: rule foo added bazel-out/ios_x86_64-fastbuild/bin/external/foo/foo/_virtual_includes/foo to CcCompilationContext.Builder
OB DEBUG: Rule: foo created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@12f30b47 including [bazel-out/ios_x86_64-fastbuild/bin/external/foo/foo/_virtual_includes/foo]
OB DEBUG: Rule: ok created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@4d10f48 including []
OB DEBUG: Rule: ok created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@68422153 including []

And for bazel_cc_test.sh:

OB DEBUG: Rule: stl created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@6d825b70 including []
OB DEBUG: rule foo added bazel-out/darwin-fastbuild/bin/external/foo/foo/_virtual_includes/foo to CcCompilationContext.Builder
OB DEBUG: Rule: foo created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@2f14c87b including [bazel-out/darwin-fastbuild/bin/external/foo/foo/_virtual_includes/foo]
OB DEBUG: Rule: ok created ccCompilationContext = com.google.devtools.build.lib.rules.cpp.CcCompilationContext@6c804fae including [bazel-out/darwin-fastbuild/bin/external/foo/foo/_virtual_includes/foo]

This makes me wonder a few things:

Why is the objc_library case loading so many more rules than the cc_library rule? Some of them seem unnecessary, like for example all the python, bash, java, etc launchers.
Why is the objc_library rule creating two separate CcCompilationContext for each rule? If you look at the BUILD files, the configuration is identical. It should be one CcCompilationContext per rule no?
Why are the includes (in CommandLineCcCompilationContext.includeDirs) not being propagated? I have been looking for some difference between the two, but I still haven't figured it out.

lberki · 2018-08-22T07:46:16Z

(1) is simply because objc_library apparently has a more implicit dependencies than cc_library. You can check by running bazel query --host_deps --implicit_deps deps(<your objc_library rule>) what targets there are.

(2), I don't know off the bat, but I if you say it's because of ARCness, I believe you :)

(3), I also don't know -- CompilationSupport#mergeDependentCcCompilationContexts() stipulates that include directories should be merged from the transitive dependencies and https://github.com/bazelbuild/bazel/blob/master/src/main/java/com/google/devtools/build/lib/rules/cpp/CcCompilationContext.java#L404 says it should be like that. Are you sure that's the case? Also, what's do you mean when you say "some difference between the two?" objc_library and cc_library?

lberki · 2018-08-22T07:46:23Z

/cc @mhlopko

ob · 2018-08-22T15:35:07Z

(1) I see. And do you know where those implicit dependencies come from? There are a whole bunch in there that don't really seem like they should be there. I'd be interested in another PR to remove some of them and make the list closer to cc_library.

(2) What I found was that we are creating CcCompilationContext's via a call to CcCompilationHelper.compile() twice. Once for ARC sources here and again for non-ARC sources here. This is regardless of whether there exists a non_arc_srcs attribute. I'm wondering if this is necessary or if the second call to compile() could be conditioned on the existence of non_arc_srcs.

(3) I found that even though the two CcCompilationContext's mentioned in the previous bullet point are merged here, they are dropped on the floor and a new one is created here. If I remove that and wire in the merged CcCompilationContext from the CompilationSupport:ccCompileAndLink() function, then everything works as expected. I'm left wondering what is the rationale for creating this extra CcCompilationSupport in ObjcLibrary.java...

ob · 2018-08-22T18:58:01Z

Also, what's do you mean when you say "some difference between the two?" objc_library and cc_library?

I mean that cc_library properly propagates the virtual directory created when include_prefix and/or strip_include_prefix is specified and objc_library does not.

Doing some archeology, I found this commit, which seems to be the one that introduced creating a new CcCompilationContext to fix a bug in propagating the headers...

I've played with some variations of this but I'm not sure how to fix it... if I just pass the includeDirs like so:

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 42741b573b..53149ba42a 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.CcLinkParamsStore;
 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,7 +92,8 @@ public class ObjcLibrary implements RuleConfiguredTargetFactory {
     J2ObjcEntryClassProvider j2ObjcEntryClassProvider = new J2ObjcEntryClassProvider.Builder()
       .addTransitive(ruleContext.getPrerequisites("deps", Mode.TARGET,
           J2ObjcEntryClassProvider.class)).build();
-    CcCompilationContext ccCompilationContext =
+    CcCompilationContext mergedCcCompilationContext = compilationSupport.getCcCompilationContext();
+    CcCompilationContext.Builder ccCompilationContextBuilder =
         new CcCompilationContext.Builder(ruleContext)
             .addDeclaredIncludeSrcs(
                 CompilationAttributes.Builder.fromRuleContext(ruleContext)
@@ -99,9 +101,13 @@ public class ObjcLibrary implements RuleConfiguredTargetFactory {
                     .hdrs()
                     .toCollection())
             .addTextualHdrs(common.getTextualHdrs())
-            .addDeclaredIncludeSrcs(common.getTextualHdrs())
-            .build();
+            .addDeclaredIncludeSrcs(common.getTextualHdrs());
+    for (PathFragment path : mergedCcCompilationContext.getIncludeDirs()) {
+      ccCompilationContextBuilder.addIncludeDir(path);
+    }
 
+    CcCompilationContext ccCompilationContext = ccCompilationContextBuilder.build();
     CcCompilationInfo.Builder ccCompilationInfoBuilder = CcCompilationInfo.Builder.create();
     ccCompilationInfoBuilder.setCcCompilationContext(ccCompilationContext);

Then the action that creates the virtual directory doesn't run, even though the -I for the virtual directory is there. It also breaks the testDoesNotPropagateProtoIncludes test, which explicitly tests that we don't propagate the includes for objc prototypes.

If I use the merged ccCompilationContext directly, like so:

--- 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.CcLinkParamsStore;
 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 class ObjcLibrary implements RuleConfiguredTargetFactory {
     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(

Then my test works (-I of virtual directory is passed and virtual directory creation action is run), but it breaks the following tests:

1) testProvidesObjcLibraryAndHeaders(com.google.devtools.build.lib.rules.objc.ObjcLibraryTest)
unexpected (1): objc/private.h
---
expected      : [objc/a.h, objc/b.h, objc2/c.h, objc2/d.h]
but was       : [objc/a.h, objc/b.h, objc/private.h, objc2/c.h, objc2/d.h]
        at com.google.devtools.build.lib.rules.objc.ObjcLibraryTest.testProvidesObjcLibraryAndHeaders(ObjcLibraryTest.java:1042)

There were 2 failures:
1) testProvidesObjcHeadersWithDotMFiles(com.google.devtools.build.lib.rules.objc.ObjcLibraryTest)
unexpected (1): objc/private.h
---
expected      : [objc/a.h, objc/b.h, objc/f.m, objc2/d.h, objc2/e.m]
but was       : [objc/a.h, objc/b.h, objc/f.m, objc/private.h, objc2/d.h, objc2/e.m]
        at com.google.devtools.build.lib.rules.objc.ObjcLibraryTest.testProvidesObjcHeadersWithDotMFiles(ObjcLibraryTest.java:382)
2) testDoesNotPropagateProtoIncludes(com.google.devtools.build.lib.rules.objc.ObjcLibraryTest)
expected not to contain: objc_proto_lib
but was                : tools/osx/crosstool/iossim/wrapped_clang -arch x86_64 -DCOMPILER_GCC3 -DCOMPILER_GCC4 -Dunix -DOS_IOS -DU_HAVE_NL_LANGINFO_CODESET=0 -DU_HAVE_STD_STRING -D__STDC_FORMAT_MACROS -fcolor-diagnostics -O0 -DDEBUG -Wshorten-64-to-32 -Wbool-conversion -Wconstant-conversion -Wduplicate-method-match -Wempty-body -Wenum-conversion -Wint-conversion -Wunreachable-code -Wmismatched-return-types -Wundeclared-selector -Wuninitialized -Wunused-function -Wunused-variable -DXCODE_FEATURE_FOR_TESTING=xcode_7.3 -F__BAZEL_XCODE_SDKROOT__/Developer/Library/Frameworks -F__BAZEL_XCODE_DEVELOPER_DIR__/Platforms/iPhoneSimulator.platform/Developer/Library/Frameworks -DOS_IOS -fno-autolink -isysroot __BAZEL_XCODE_SDKROOT__ -fobjc-arc -mios-simulator-version-min=8.4 -MD -MF bazel-out/ios_x86_64-fastbuild/bin/b/_objs/lib/arc/b.d -iquote . -iquote bazel-out/ios_x86_64-fastbuild/genfiles -iquote bazel-out/ios_x86_64-fastbuild/bin -Ibazel-out/ios_x86_64-fastbuild/bin/x/_generated_protos/objc_proto_lib -Iexternal/bazel_tools/objcproto/include -Ibazel-out/ios_x86_64-fastbuild/genfiles/external/bazel_tools/objcproto/include -fexceptions -fasm-blocks -fobjc-abi-version=2 -fobjc-legacy-dispatch -O0 -DDEBUG=1 -c b/b.m -o bazel-out/ios_x86_64-fastbuild/bin/b/_objs/lib/arc/b.o --FASTBUILD_ONLY_FLAG
        at com.google.devtools.build.lib.rules.objc.ObjcLibraryTest.testDoesNotPropagateProtoIncludes(ObjcLibraryTest.java:1793)

If anyone has any ideas what the right answer here is, I'm all ears...

…refix

ob · 2018-08-28T16:51:26Z

So I fixe one of the failures by adding a flag to the mergeDependentCcCompilationContext function that prevents merging headers. Not pretty but it fixes the testProvidesObjcLibraryAndHeaders test. However, now I'm finding something strange. in my test case bazel_objc_test.sh, if I run bazel, I get:

$ sh bazel_objc_test.sh 
+ which bazel
/Users/obonilla/o/bazel/bazel-bin/src/bazel
+ test_objc_library_include_prefix_external_repository
+ r=/Users/obonilla/bazel-test-tmp/objc_include_prefix/objc/r
+ mkdir -p /Users/obonilla/bazel-test-tmp/objc_include_prefix/objc/r/foo/v1
+ touch /Users/obonilla/bazel-test-tmp/objc_include_prefix/objc/r/WORKSPACE
+ echo '#define FOO 42'
+ cat
+ cat
+ cat
+ cat
+ cat
+ bazel clean
Extracting Bazel installation...
Starting local Bazel server and connecting to it...
INFO: Starting clean.
+ bazel build --sandbox_debug --verbose_failures -s :ok
INFO: Analysed target //:ok (18 packages loaded).
INFO: Found 1 target...
SUBCOMMAND: # //:ok [action 'Compiling ok.m']
(cd /private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/execroot/__main__ && \
  exec env - \
    APPLE_SDK_PLATFORM=iPhoneSimulator \
    APPLE_SDK_VERSION_OVERRIDE=11.4 \
    PATH='/Users/obonilla/o/bazel/bazel-bin/src:/Users/obonilla/.fastlane/bin:/Users/obonilla/.cargo/bin:/usr/local/cuda/bin:/Users/obonilla/.cargo/bin:/Users/obonilla/.nix-profile/bin:/Users/obonilla/.nix-profile/sbin:/Users/obonilla/.nix-profile/lib/kde4/libexec:/nix/var/nix/profiles/default/bin:/nix/var/nix/profiles/default/sbin:/nix/var/nix/profiles/default/lib/kde4/libexec:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/linkedin/bin:/Applications/VMware Fusion.app/Contents/Public:/usr/local/MacGPG2/bin:/opt/X11/bin:/Applications/Wireshark.app/Contents/MacOS:/export/content/linkedin/bin:/Users/obonilla/bin:/Users/obonilla/o/pcm' \
    XCODE_VERSION_OVERRIDE=9.4.1 \
  external/local_config_cc/wrapped_clang -arch x86_64 '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign -fno-omit-frame-pointer -O0 -DDEBUG -Wshorten-64-to-32 -Wbool-conversion -Wconstant-conversion -Wduplicate-method-match -Wempty-body -Wenum-conversion -Wint-conversion -Wunreachable-code -Wmismatched-return-types -Wundeclared-selector -Wuninitialized -Wunused-function -Wunused-variable -iquote . -iquote bazel-out/ios_x86_64-fastbuild/genfiles -iquote bazel-out/ios_x86_64-fastbuild/bin -iquote external/foo -iquote bazel-out/ios_x86_64-fastbuild/genfiles/external/foo -iquote bazel-out/ios_x86_64-fastbuild/bin/external/foo -Ibazel-out/ios_x86_64-fastbuild/bin/external/foo/foo/_virtual_includes/foo -MD -MF bazel-out/ios_x86_64-fastbuild/bin/_objs/ok/arc/ok.d -F__BAZEL_XCODE_SDKROOT__/System/Library/Frameworks -F__BAZEL_XCODE_DEVELOPER_DIR__/Platforms/iPhoneSimulator.platform/Developer/Library/Frameworks -DOS_IOS -fno-autolink -isysroot __BAZEL_XCODE_SDKROOT__ -fobjc-arc '-mios-simulator-version-min=11.4' -fexceptions -fasm-blocks '-fobjc-abi-version=2' -fobjc-legacy-dispatch -O0 '-DDEBUG=1' -c ok.m -o bazel-out/ios_x86_64-fastbuild/bin/_objs/ok/arc/ok.o)
ERROR: /Users/obonilla/bazel-test-tmp/objc_include_prefix/BUILD:1:1: C++ compilation of rule '//:ok' failed (Exit 1): sandbox-exec failed: error executing command 
  (cd /private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/execroot/__main__ && \
  exec env - \
    APPLE_SDK_PLATFORM=iPhoneSimulator \
    APPLE_SDK_VERSION_OVERRIDE=11.4 \
    DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer \
    PATH='/Users/obonilla/o/bazel/bazel-bin/src:/Users/obonilla/.fastlane/bin:/Users/obonilla/.cargo/bin:/usr/local/cuda/bin:/Users/obonilla/.cargo/bin:/Users/obonilla/.nix-profile/bin:/Users/obonilla/.nix-profile/sbin:/Users/obonilla/.nix-profile/lib/kde4/libexec:/nix/var/nix/profiles/default/bin:/nix/var/nix/profiles/default/sbin:/nix/var/nix/profiles/default/lib/kde4/libexec:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/linkedin/bin:/Applications/VMware Fusion.app/Contents/Public:/usr/local/MacGPG2/bin:/opt/X11/bin:/Applications/Wireshark.app/Contents/MacOS:/export/content/linkedin/bin:/Users/obonilla/bin:/Users/obonilla/o/pcm' \
    SDKROOT=/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator11.4.sdk \
    TMPDIR=/var/folders/b2/5s3n8vxx2wx22cm0x66dby1c000ncy/T/ \
    XCODE_VERSION_OVERRIDE=9.4.1 \
  /usr/bin/sandbox-exec -f /private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/sandbox/darwin-sandbox/1/sandbox.sb /private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/execroot/__main__/_bin/process-wrapper '--timeout=0' '--kill_delay=15' external/local_config_cc/wrapped_clang -arch x86_64 '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign -fno-omit-frame-pointer -O0 -DDEBUG -Wshorten-64-to-32 -Wbool-conversion -Wconstant-conversion -Wduplicate-method-match -Wempty-body -Wenum-conversion -Wint-conversion -Wunreachable-code -Wmismatched-return-types -Wundeclared-selector -Wuninitialized -Wunused-function -Wunused-variable -iquote . -iquote bazel-out/ios_x86_64-fastbuild/genfiles -iquote bazel-out/ios_x86_64-fastbuild/bin -iquote external/foo -iquote bazel-out/ios_x86_64-fastbuild/genfiles/external/foo -iquote bazel-out/ios_x86_64-fastbuild/bin/external/foo -Ibazel-out/ios_x86_64-fastbuild/bin/external/foo/foo/_virtual_includes/foo -MD -MF bazel-out/ios_x86_64-fastbuild/bin/_objs/ok/arc/ok.d -F__BAZEL_XCODE_SDKROOT__/System/Library/Frameworks -F__BAZEL_XCODE_DEVELOPER_DIR__/Platforms/iPhoneSimulator.platform/Developer/Library/Frameworks -DOS_IOS -fno-autolink -isysroot __BAZEL_XCODE_SDKROOT__ -fobjc-arc '-mios-simulator-version-min=11.4' -fexceptions -fasm-blocks '-fobjc-abi-version=2' -fobjc-legacy-dispatch -O0 '-DDEBUG=1' -c ok.m -o bazel-out/ios_x86_64-fastbuild/bin/_objs/ok/arc/ok.o)
ok.m:2:10: fatal error: 'foolib/foo.h' file not found
#include "foolib/foo.h"
         ^~~~~~~~~~~~~~
1 error generated.
Target //:ok failed to build
INFO: Elapsed time: 3.412s, Critical Path: 0.11s
INFO: 0 processes.
FAILED: Build did NOT complete successfully

However, if I copy and paste the command that failed:

$ (cd /private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/execroot/__main__ && \
>   exec env - \
>     APPLE_SDK_PLATFORM=iPhoneSimulator \
>     APPLE_SDK_VERSION_OVERRIDE=11.4 \
>     DEVELOPER_DIR=/Applications/Xcode.app/Contents/Developer \
>     PATH='/Users/obonilla/.fastlane/bin:/Users/obonilla/.cargo/bin:/usr/local/cuda/bin:/Users/obonilla/.cargo/bin:/Users/obonilla/.nix-profile/bin:/Users/obonilla/.nix-profile/sbin:/Users/obonilla/.nix-profile/lib/kde4/libexec:/nix/var/nix/profiles/default/bin:/nix/var/nix/profiles/default/sbin:/nix/var/nix/profiles/default/lib/kde4/libexec:/usr/local/bin:/usr/bin:/bin:/usr/sbin:/sbin:/usr/local/linkedin/bin:/Applications/VMware Fusion.app/Contents/Public:/usr/local/MacGPG2/bin:/opt/X11/bin:/Applications/Wireshark.app/Contents/MacOS:/export/content/linkedin/bin:/Users/obonilla/bin:/Users/obonilla/o/pcm' \
>     SDKROOT=/Applications/Xcode.app/Contents/Developer/Platforms/iPhoneSimulator.platform/Developer/SDKs/iPhoneSimulator11.4.sdk \
>     TMPDIR=/var/folders/b2/5s3n8vxx2wx22cm0x66dby1c000ncy/T/ \
>     XCODE_VERSION_OVERRIDE=9.4.1 \
>   /usr/bin/sandbox-exec -f /private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/sandbox/darwin-sandbox/1/sandbox.sb /private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/execroot/__main__/_bin/process-wrapper '--timeout=0' '--kill_delay=15' external/local_config_cc/wrapped_clang -arch x86_64 '-D_FORTIFY_SOURCE=1' -fstack-protector -fcolor-diagnostics -Wall -Wthread-safety -Wself-assign -fno-omit-frame-pointer -O0 -DDEBUG -Wshorten-64-to-32 -Wbool-conversion -Wconstant-conversion -Wduplicate-method-match -Wempty-body -Wenum-conversion -Wint-conversion -Wunreachable-code -Wmismatched-return-types -Wundeclared-selector -Wuninitialized -Wunused-function -Wunused-variable -iquote . -iquote bazel-out/ios_x86_64-fastbuild/genfiles -iquote bazel-out/ios_x86_64-fastbuild/bin -iquote external/foo -iquote bazel-out/ios_x86_64-fastbuild/genfiles/external/foo -iquote bazel-out/ios_x86_64-fastbuild/bin/external/foo -Ibazel-out/ios_x86_64-fastbuild/bin/external/foo/foo/_virtual_includes/foo -MD -MF bazel-out/ios_x86_64-fastbuild/bin/_objs/ok/arc/ok.d -F__BAZEL_XCODE_SDKROOT__/System/Library/Frameworks -F__BAZEL_XCODE_DEVELOPER_DIR__/Platforms/iPhoneSimulator.platform/Developer/Library/Frameworks -DOS_IOS -fno-autolink -isysroot __BAZEL_XCODE_SDKROOT__ -fobjc-arc '-mios-simulator-version-min=11.4' -fexceptions -fasm-blocks '-fobjc-abi-version=2' -fobjc-legacy-dispatch -O0 '-DDEBUG=1' -c ok.m -o bazel-out/ios_x86_64-fastbuild/bin/_objs/ok/arc/ok.o)

$ find -L  /private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/execroot/__main__ -name ok.o
/private/var/tmp/_bazel_obonilla/fb14d7dc34c7092d468bd3efcc1c8799/execroot/__main__/bazel-out/ios_x86_64-fastbuild/bin/_objs/ok/arc/ok.o

it works. Could this be a race?

hlopko · 2018-08-30T09:47:32Z

yup, it's because arc/noarc
Wow I didn't know objc rules don't provide the context they get from ccCompilationHelper.compile. That explains the confusion we talked about before. I was always looking into the C++ rules.
That seems like a bug to me. I'd assume they add objc specific headers to the context they get, not that they'll create context from scratch and try to mimic what is inside. I'd also assume that we want to merge private headers as well (what if a public header includes a private header? To me this is perfectly fine, and common in C++). So IMO the test is wrong. @c-parsons @calpeyser @sergiocampama do you know if the current behavior is intentional?

ob · 2018-08-30T16:41:16Z

It'd be worth also to double check what the rationale behind this other test is:

  @Test
  public void testDoesNotPropagateProtoIncludes() throws Exception {
    useConfiguration("--crosstool_top=" + MockObjcSupport.DEFAULT_OSX_CROSSTOOL);
    scratch.file(
        "x/BUILD",
        "proto_library(",
        "   name = 'protos',",
        "   srcs = ['data.proto'],",
        ")",
        "objc_proto_library(",
        "   name = 'objc_proto_lib',",
        "   deps = [':protos'],",
        "   portable_proto_filters = ['data_filter.pbascii'],",
        ")");
    createLibraryTargetWriter("//a:lib")
        .setList("srcs", "a.m")
        .setList("deps", "//x:objc_proto_lib")
        .write();
    createLibraryTargetWriter("//b:lib").setList("srcs", "b.m").setList("deps", "//a:lib").write();

    CommandAction compileAction1 = compileAction("//a:lib", "a.o");
    CommandAction compileAction2 = compileAction("//b:lib", "b.o");

    assertThat(Joiner.on(" ").join(compileAction1.getArguments())).contains("objc_proto_lib");
    assertThat(Joiner.on(" ").join(compileAction2.getArguments())).doesNotContain("objc_proto_lib");
  }

Aren't the headers of the objc_proto_lib needed by dependents?

hlopko · 2018-09-18T06:54:13Z

@sergiocampama might know more details?

ob · 2018-09-26T22:19:19Z

I'm very confused about what the right behavior is. I did a bit of digging and in comments on #1802 there are mentions that propagating the objc_proto_library is the right behavior. Most issues are still open though, and the test explicitly tests they are not propagated.

This is blocking both this PR and #6243.

ob · 2019-01-30T00:13:35Z

This is no longer being pursued in this form. I'll take another crack once the C++ rules are in skylark.

WIP: objc include_prefix/strip_include_prefix

21a28cf

ob requested review from DavidGoldman, allevato, dmaclach, lberki and sergiocampama as code owners August 21, 2018 22:09

googlebot added the cla: yes label Aug 21, 2018

Remove diffs committed by mistake

83ad4b7

iirina added the team-Rules-CPP Issues for C++ rules label Aug 22, 2018

ob added 5 commits August 22, 2018 12:35

Snap: this works

7e56fad

Remove debugging

64c4ac8

Clean up debugging

745b3b1

Add a way to avoid merging private headers

8d0d2e0

Merge remote-tracking branch 'bazel/master' into objc_include_strip_p…

92ed1c5

…refix

hlopko added z-team-Apple Deprecated. Send to rules_apple, or label team-Rules-CPP + platform:apple and removed team-Rules-CPP Issues for C++ rules labels Aug 30, 2018

ob mentioned this pull request Sep 26, 2018

[WIP] Objc modular imports #6243

Closed

jin added the WIP label Jan 3, 2019

ob closed this Jan 30, 2019

ob mentioned this pull request Mar 12, 2019

Implicit HeaderMaps #3712

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

WIP: objc include_prefix/strip_include_prefix#5954

WIP: objc include_prefix/strip_include_prefix#5954
ob wants to merge 7 commits intobazelbuild:masterfrom
ob:objc_include_strip_prefix

ob commented Aug 21, 2018 •

edited

Loading

Uh oh!

lberki commented Aug 22, 2018

Uh oh!

lberki commented Aug 22, 2018

Uh oh!

ob commented Aug 22, 2018

Uh oh!

ob commented Aug 22, 2018

Uh oh!

ob commented Aug 28, 2018 •

edited

Loading

Uh oh!

hlopko commented Aug 30, 2018

Uh oh!

ob commented Aug 30, 2018 •

edited

Loading

Uh oh!

hlopko commented Sep 18, 2018

Uh oh!

ob commented Sep 26, 2018

Uh oh!

ob commented Jan 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

Conversation

ob commented Aug 21, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lberki commented Aug 22, 2018

Uh oh!

lberki commented Aug 22, 2018

Uh oh!

ob commented Aug 22, 2018

Uh oh!

ob commented Aug 22, 2018

Uh oh!

ob commented Aug 28, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hlopko commented Aug 30, 2018

Uh oh!

ob commented Aug 30, 2018 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

hlopko commented Sep 18, 2018

Uh oh!

ob commented Sep 26, 2018

Uh oh!

ob commented Jan 30, 2019

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

6 participants

ob commented Aug 21, 2018 •

edited

Loading

ob commented Aug 28, 2018 •

edited

Loading

ob commented Aug 30, 2018 •

edited

Loading