Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

0.36.0.Beta.2: Pods-Framework-umbrella.h generation breaks LLDB symbol lookup #3092

Closed
chrislconover opened this issue Jan 30, 2015 · 19 comments

Comments

@chrislconover
Copy link

Description

Using Cocoapods 0.36.0.Beta.2, you can build a mixed language framework, and run it in a client.

However, symbols are not available in the client, with LLDB giving errors like this:

error: Error in auto-import:
failed to get module 'MixedMacClient' from AST context:
/Users/User/Library/Developer/Xcode/DerivedData/MixedMacClient-clluilwecswymtbkmocqdqhbgxvd/Build/Products/Debug/Pods/Mixed.framework/Headers/Pods-Mixed-umbrella.h:3:9: note: in file included from /Users/User/Library/Developer/Xcode/DerivedData/MixedMacClient-clluilwecswymtbkmocqdqhbgxvd/Build/Products/Debug/Pods/Mixed.framework/Headers/Pods-Mixed-umbrella.h:3:
#import "Mixed.h"
...

It is interesting to note, that deleting this line from the file generated in the build output directory like:

Build/Products/Debug/Pods/FrameworkName.framework/Versions/A/Headers/Pods-FrameworkName-umbrella.h

does not break the building process, but does allow client code to see both Swift and Obj-C symbols in the framework. I am not sure if this amounts to complete symbol information, but it is a pronounced improvement:

(lldb) po swiftFoo
0x0000618000042f40
(name = "Swift")

Also note, that the existing bug #3062 in which cocoapods incorrectly strips symbols during deployment of debug builds, exposes a crash bug in Xcode during symbol lookup. To get the symbol data above, and to avoid Xcode's crash bug, you must first clear the strip-symbols on copy setting.

Steps to Reproduce:

  1. Download project at this link:
    https://www.dropbox.com/s/trhe5vwhzoa9bf5/umbrella_file_breaks_symbol_lookup.tar.gz?dl=0

  2. Run app, it should stop at the break point on println("done"):

    let swiftFoo = SwiftPublicFoo(name:"Swift")
    swiftFoo.fooDo()
    
    let objFoo = ObjectiveFoo(name: "Objective-C")
    objFoo.fooDo()
    
    ->  println("done")
    
  3. type po swiftFoo, as above

  4. see baseline, broken case as in first output

Steps to Workaround

  1. open generated Pods-Mixed-umbrella.h, comment out include for "Mixed.h"
  2. run app again, type po swiftFoo, note that symbol lookup now works.

This may well be an Xcode bug, but the fact that you deleting the include fixes the debugger issue while not affecting the build, suggests that this problem can be avoided.

@neonichu
Copy link
Member

The generated umbrella header is under your own control, only public headers will be added to it.

For your Pod, you can use:

s.public_header_files = 'Mixed/ObjectiveFoo.h'

to no longer include Mixed.h and LLDB works as expected.

@chrislconover
Copy link
Author

Thanks for your response, @neonichu , but there is actually more nuance to this problem. It is not simply a manner of making an umbrella file private (that would serve no purpose).

The crux of it is that:

  1. There are two consumers of the umbrella header file: LLDB, and Clang / the client Objective-C app that needs to import the mixed framework.
  2. LLDB and Clang are not consistent and LLDB does not follow the guidance given by Apple, for Xcode/Clang.

From digging deeper, I see this as three things:

  1. a LLDB bug
  2. possibly incorrect documentation (or just the LLDB bug)
  3. a commendable but less than idea implementation of umbrella header file generation
  4. The LLDB bug:
    1. it is not using the build paths to resolve symbol information / does not correctly parse modular header includes within a modular header, in the way that Clang does during compilation. That is to say that an import statement in the explicit umbrella file header of the recommended form #import <Mixed/SomePublicFile.h> will result in LLDB failing to resolve the symbols.

      I have tried the allow non modular includes (etc) in the Pods' build settings, but still got the error. I did not see any different path settings for LLDB symbol lookup, but could I be missing something here - could cocoapods configure LLDB with the framework include path?

    2. it produces misleading error messages when trying to parse (what I think to be modular) includes of the form recommended by Xcode: #import <Mixed/Mixed.h> in the umbrella file

  5. Potential documentation errors:
    Apple's documentation and directives in the default generated umbrella header file dictate that you should use qualified, modular header includes with angular brackets, of the form
    #import <Mixed/Mixed.h>, when in fact this breaks LLDB symbol lookup.
  6. Cocoapods behavior could be improved:
    From reading, and re-reading both Apple Docs and @mrackwitz's Guide-to-CocoaPods , I have arrived at the understanding that the umbrella header serves two purposes in this context:
    1. Provides external client code convenient access to a modules external api:
      #import <Mixed/Mixed.h> (the original role)
    2. It allows Swift framework code to use Objective-C framework code (emphasis on from within the framework, ie like a bridging header for frameworks). From my understanding, the mechanism for this is that the umbrella header is registered in / as the (Swift) module map, so the Swift compiler can pull in the configured Obj-C umbrella header from the module map, and make available the symbols contained within.
      Cocoapods now generates umbrella files of the form Pods-FrameworkName-umbrella.h, and registers this with / in / as the module map. It does this by including all public header files declared in the podspec. This would seem to completely satisfy this constraint. However, it does not satisfy the expected import convention used in Objectice-C, requiring the less intuitive
      #import <Mixed/Pods-Mixed-umbrella.h>
      To work around that, and achieve the intent of 2.i above, and since Cocoapods does not default to the standard umbrella file name, I added an umbrella file of the standard form: <FrameworkName/FrameworkName.h>. The problem is that since cocoapods includes all public header files, it also includes the explicit umbrella file. This header file, when configured as Apple suggests, breaks LLDB symbol lookup.

The non obvious (to me) work around, is to ignore the form described in Xcode's generated umbrella file and import files using double quotes instead of angular brackets (I realized this distinction after posting the original bug). Many people will fall victim to this (following the directions). That, and this still leaves the framework author with having to generate what cocoapods is now smart enough to do on it's own - the umbrella file for the normal, established use-case of #import <Mixed/Mixed.h>.

My points in summary are that
A. if you follow the existing documentation, you won't get symbols
B. you can work around this by ignoring Apple's directions and using double quotes, and
C. it would be better for cocoapods to use the default umbrella file name if it does not already exist, and to warn and use the current behavior if there is an existing umbrella file.

For now, I highly recommend updating Cocoapods guides to direct people to ignore Apple's guidance and not use angular brackets in any umbrella or otherwise public header they have. I also recommend that Cocoapods default to the expected FrameworkName.h umbrella file name, as the Cocoapods implementation appears to be otherwise complete and functional, since it includes all public headers with double quotes and not angular brackets (ie it correctly ignore's Apple's directives).

As it is now though, there are a number of posts on stackoverflow struggling with this and issues like this.

Thanks!

@kylef kylef reopened this Jan 31, 2015
@neonichu
Copy link
Member

neonichu commented Feb 1, 2015

To your point 3.ii: it is best to also use the module import @import FrameworkName in ObjC as well. That way, users don't have to know the actual name of the umbrella header used in the module map and will have all the other advantages of using Clang modules.

@chrislconover
Copy link
Author

Good point, that is the best (only good) approach as it pulls in the Swift symbols as well - something worth emphasizing in guides and docs.

@chrislconover
Copy link
Author

I don't know how or why, but in two separate test projects I have seen that Xcode will still try to reference an Umbrella file of the expected Framework.h form. I have tried this both by moving the existing umbrella file out of the project directory, and by adding a filter to remove it from the set of public headers. In both cases, the results where the same.

Here is what I did to filter out the existing umbrella file:

s.source_files =        'Mixed/**/*.{h,m,swift}'
s.public_header_files = 'Mixed/**/*.h'
s.private_header_files = 'Mixed/Mixed.h'

Here is my spot check afterwards:

MyMachine:MixedMacClient Me$ find . -name "Mixed.h"
./Pods/Headers/Private/Mixed/Mixed.h

Obviously when I removed the file from the project directory, it did not appear at all. In either case though, the result was the same, Swift / Clang / Xcode would try to include it in Framework-Swift.h:

<snip>
#if !defined(OBJC_DESIGNATED_INITIALIZER)
# if defined(__has_attribute) && __has_attribute(objc_designated_initializer)
#  define OBJC_DESIGNATED_INITIALIZER __attribute__((objc_designated_initializer))
# else
#  define OBJC_DESIGNATED_INITIALIZER
# endif
#endif
#if defined(__has_feature) && __has_feature(modules)
#endif

#import <Mixed/Mixed.h>
<snip>

Hopefully this is just something that I am missing (I did nuke the Pods and DerivedData directories though).

Also, is there a setting for "Defines Modules"? This needs to see Objective-C code in Swift, within a framework.

I will attach an updated zip illustrating the problem with include of the phantom umbrella file.

@chrislconover
Copy link
Author

Here is the link to the project without the umbrella file, that fails to build. I could well me missing something, but I don't know why Xcode / Clang would be including that header, or what the mechanism is for that: https://www.dropbox.com/s/trwtwt2nbd7o0oh/FailsWithoutUmbrellaFile.zip?dl=0

Thanks -c

@neonichu
Copy link
Member

neonichu commented Feb 2, 2015

Good catch! It appears that swiftc -emit-objc-header doesn't know about module maps or we are missing some kind of additional option. The process is unfortunately a bit opaque, the invocation of this is absent from the build log. Maybe @mrackwitz knows more about it?

@neonichu
Copy link
Member

neonichu commented Feb 2, 2015

This shows the full invocation which generates it: https://gist.githubusercontent.com/neonichu/366b2b10aa5b49ec3b82/raw/36b1b2a9b1dd07b74b5fb9d2caf230b7e318b1a9/emit-header.sh - obtained by putting a logging shim in place of swiftc.

@chrislconover
Copy link
Author

@neonichu, @mrackwitz,

I think I traced this one to the source and it should be easy to fix. I was originally diverted by the error message, thinking it was reporting the familiar problem that a non modular include was found in the header, when in fact it literally meant header:

/Users/User/Library/Developer/Xcode/DerivedData/MixedMacClient-clluilwecswymtbkmocqdqhbgxvd/Build/Products/Debug/Pods/Mixed.framework/Headers/Mixed.h:19:9: error: include of non-modular header inside framework module 'Mixed.Mixed'
#import <Mixed/ObjectiveFoo.h>
        ^
/Users/User/Library/Developer/Xcode/DerivedData/MixedMacClient-clluilwecswymtbkmocqdqhbgxvd/Build/Products/Debug/Pods/Mixed.framework/Headers/Pods-Mixed-umbrella.h:4:9: note: in file included from /Users/User/Library/Developer/Xcode/DerivedData/MixedMacClient-clluilwecswymtbkmocqdqhbgxvd/Build/Products/Debug/Pods/Mixed.framework/Headers/Pods-Mixed-umbrella.h:4:
#import "ObjectiveFoo.h"
        ^
/Users/User/Library/Developer/Xcode/DerivedData/MixedMacClient-clluilwecswymtbkmocqdqhbgxvd/Build/Products/Debug/Pods/Mixed.framework/Headers/ObjectiveFoo.h:11:1: error: duplicate interface definition for class 'ObjectiveFoo'
@interface ObjectiveFoo : NSObject
^
/Users/User/development/tests/umbrella_file_breaks_symbol_lookup/MixedMacClient/Pods/Headers/Private/Mixed/ObjectiveFoo.h:11:12: note: previous definition is here
@interface ObjectiveFoo : NSObject

Evidently, Xcode / LLDB was actually reporting that it found a non modular header in the search path, and it wasn't happy with this. Specifically, it was finding the headers stashed in the

Project/Pods/Headers/{Private,Public}/Framework/

directories when it expected to find them in the build output directory.

I then trace this problem back to the Pods-Mixed-Private.xconfig file, specifically the line:

HEADER_SEARCH_PATHS = "${PODS_ROOT}/Headers/Private" "${PODS_ROOT}/Headers/Private/Mixed" "${PODS_ROOT}/Headers/Public" "${PODS_ROOT}/Headers/Public/Mixed"

In my testing, this is not needed, as the framework is built first, and the project then finds the necessary testing in the framework's include path. This could use more testing, but I think the best final fixes for this bug are to:

  1. Remove the aforementioned include path
  2. Default to generating the umbrella file in the form of FrameworkName/FrameworkName.h unless someone wants to make things harder on themselves - at which point they own the problem
  3. For completeness, change the includes in the generated header to use the modular / angular bracket form.

I think trying to solve the problem wherein swiftc / Xcode includes the non-existing default umbrella file is a red-herring and a case of solving the wrong problem. If #2 above solves two problems simply and elegantly (and without breaking convention), then it is the best choice. Put the other way, it's hard to argue for putting a lot of effort into a solution that breaks the convention and complicates the easy case where the user/client just "wants it to work" without having to create a redundant umbrella file.

I don't know this codebase or speak Ruby, but I think the above changes should not be difficult. It should be more of a problem of removing logic that we don't need.

@neonichu neonichu added this to the 0.36.0 milestone Feb 6, 2015
@neonichu
Copy link
Member

neonichu commented Feb 6, 2015

Thanks for your findings!

I'm still torn on generating the framework header with the default name, because there can already be a header in the user's codebase which has a naming conflict with it. This can raise some backwards compatibility concerns, because there used to be no special technical meaning to this header file.

@chrislconover
Copy link
Author

Can you see any drawbacks to this approach?

generated-umbrella-name = standard-umbrella-file-name
    if standard-umbrella-file-name does not exist
    else alternate-pod-umbrella-file-name

I cannot see how this could break any legacy projects, and would be the simplest in the simplest case. If someone already had an umbrella file by that name, then cocoapods could note / log that fact, and follow the current form of Pods-Framework-umbrella.h. The user would then own the problem of maintaining their own umbrella file.

Really though, if you're using cocoapods to create a framework, then you're much better off treating cocoapods as the "one source of truth". The alternative is worse in every way. You would have to first create a project simply to create the otherwise unnecessary umbrella file. To make this umbrella file complete, you should really copy the contents of the generated umbrella file to get all the public includes. This is a lot of effort for something that could work out of the box.

In short, the Pods-Framework-umbrella.h approach should be the edge case, and not the other way around - and without any loss of generality.

Thanks!

@chrislconover
Copy link
Author

It would be nice to split this one apart, and at least remove the conflicting search path entries that point to the local Pods. The question of what to do about the Umbrella file generation can be left to later, but the search path issue would seemingly be a simpler fix and would help a lot of people.

Also, I should / need to write this up as a second bug, but it appears that Cocoapods erroneously treats private headers as a negation of public headers, when in fact, they are intended as a secondary, internal interface useful for debugging and the like - and not meant to be exposed to the final consumer. Currently cocoapods uses the private-header path filter as a means of filtering out public headers, but they are two concepts and deserve their own negative filters.

And I posted this to #3062, but at least in Xcode 6.3 beta, the Swift compiler is defaulting to optimize code for Debug builds, which is wrong (slow to build, and harder to debug)

@neonichu
Copy link
Member

Some changes to how we treat private headers in frameworks will be done via #3145

@chrislconover
Copy link
Author

@orta

Can / should we split this into a separate issue for removing the pods-level search paths, or could just this portion be fixed? That would fix the LLDB errors and potentially enable symbol lookup (Xcode permitting).

The separate issue of the umbrella file override could still be an Xcode bug, and is possible to work around. But out of the gate, it seems imperative that Cocoapods doesn't break anything obvious - the erroneous header includes (detailed above). If just these could be removed, then it would unblock debugging - at least to the extent that Xcode supports it.

Thanks.

@chrislconover
Copy link
Author

There is a new wrinkle with umbrella header file generation. Xcode / Swift wants you to include / import the -Swift.h file within the umbrella file and warns if you don't, but at least in my testing of the linked project, I get an error only with Cocoapods due to build order in that it is trying to consume the umbrella file that and consequently -Swift.h file, before the -Swift.h has been created.

As a workaround, if you don't try to include the -Swift.h file, it will build fine and the Swift.h file will be generated. You can then modify the umbrella file and it will continue to compile. This is in contrast to the direct / non cocoapods case, where it will generate the -Swift.h file before consuming the umbrella file. I'm not sure where it's coming from, I just know that it is different than the non cocoapods case.

So two things, the -Swift.h file should be included in the generated umbrella file, and the build order needs to be fixed so that the -Swift file is generated before the umbrella file is consumed (and this is strictly while building the framework, not the client that consumes the framework).

@orta orta modified the milestones: 0.37.0, Next Up Bug Fixes Mar 11, 2015
@jeremiegirault
Copy link

Bump, this is blocking for me with transitive mixed framework dependencies.

More info :

If framework A depends on B and both are mixed, xcode generates -Swift files <A/A-Swift.h> and <B/B-Swift.h>
Each -Swift file tries to import <A/A.h> and < B/B.h> respectively (this seem to be hardcoded in emit-objc-header when using mixed frameworks).
When doing @import B from A, the B-Swift.h is imported which in turn imports the < B/B.h> (not B-umbrella.h) so I must create B.h.
To expose objc features to swift, i have to #import all my objc headers in B.h
However B.h must be public and therefore included in B-umbrella.h.

If B.h is imported before any other header in B-umbrella.h a non-modular header import is triggered in xcode.

The detection of an existing umbrella header or creating a default one proposed above seem a great solution

@segiddins
Copy link
Member

Closing as a duplicate of #3767, which describes a plan of action.

@jasonvasquez
Copy link

@segiddins Should this issue be closed based on #3767?

@segiddins
Copy link
Member

Yup!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants