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

Fix the umbrella header #127

Closed
AliSoftware opened this issue Oct 1, 2015 · 20 comments
Closed

Fix the umbrella header #127

AliSoftware opened this issue Oct 1, 2015 · 20 comments

Comments

@AliSoftware
Copy link
Owner

It seems to be broken (see #121)

Probably because:

  1. The file named OHHTTPStubs.h, which should be my umbrella header, is also the header for one of the classes in the framework (actually the main one, but not the only one), so that's not really a header dedicated for umbrella imports
  2. When splitting the library into subspecs, that could be optionally opt-in, I have to conditionally import the other headers in that umbrella header, and this logic might not work in all environments / integration contexts

Maybe one possible solution would be to start by having a dedicated umbrella header, even if it's not called OHHTTPStubs.h but rather OHHTTPStubs-Umbrella.h for example (I think we can override the default umbrella header in the build settings).

@jshier
Copy link

jshier commented Oct 22, 2015

This may be related to what I'm seeing in Xcode 7.1. In OHHTTPStubs.h, line 32, I get an error: Include of non-modular header inside framework module OHHTTPStubs.OHHTTPStubs. This is using CocoaPods as such:

  pod 'OHHTTPStubs/Core'
  pod 'OHHTTPStubs/NSURLSession'
  pod 'OHHTTPStubs/JSON'
  pod 'OHHTTPStubs/OHPathHelpers'
  pod 'OHHTTPStubs/Swift'

@keith
Copy link

keith commented Oct 22, 2015

We're seeing this error as well.

@billyto
Copy link

billyto commented Oct 22, 2015

I tried settting "Allow Non-modular includes in Framework Modules" in Build Settings to YES, but no luck, anyone has another workaround?

@keith
Copy link

keith commented Oct 22, 2015

I was able to fix this by manually changing the imports in OHHTTPStubs.h with:

#import "Compatibility.h"
#import "OHHTTPStubsResponse.h"

But this isn't really a long term solution

@AliSoftware
Copy link
Owner Author

@billyto

I tried setting "Allow Non-modular includes in Framework Modules" to YES

Yeah, even if that worked, that would be a workaround to cheat the compiler, wouldn't solve the core issue

@keith Ah interesting. I was wondering if it would be necessary to create a custom umbrella header and a custom modulemap to use it, but if this fix of yours is enough I'd say let's do that instead!

@keith
Copy link

keith commented Oct 22, 2015

I'm not sure how that affects users not pulling this in via CocoaPods though.

@AliSoftware
Copy link
Owner Author

I'm currently downloading Xcode 7.1 (I have seen this issue with Xcode 7, but it was not deterministic and sometimes worked, sometimes failed, so hopefully Xcode 7.1 will make it fail every time so I can be sure to know whether the patch fix it or not.

Will then indeed need to try a lot of contexts / configurations then, especially how it affects people not using CocoaPods, or using some subspecs but not all, etc. Will have to double-check with that trick using #if __has_include which I'm not sure is working properly in every environment (CocoaPods vs. Carthage vs. Rome etc)

Will keep you posted, but interested if you guys could then test this in multiple different configurations afterwards.

@AliSoftware
Copy link
Owner Author

@keith did you only do these changes and nothing else?

-#import <OHHTTPStubs/Compatibility.h>
-#import <OHHTTPStubs/OHHTTPStubsResponse.h>
+#import "Compatibility.h"
+#import "OHHTTPStubsResponse.h"

Because if I'm doing that, and then try to build the framework directly from Xcode in the OHHTTPStubs.xcworkspace, I then get the "Include of non-modular header inside framework module" error on lines 188/191/194/197 😒

@keith
Copy link

keith commented Oct 22, 2015

Yep that's all I changed.

@AliSoftware
Copy link
Owner Author

Mmmmh strange it doesn't work here then.

Could you try with the fix I started in fix/umbrella-header branch? (Like make your Podfile point to that branch and tell if it fixes it)? Thx!

@keith
Copy link

keith commented Oct 22, 2015

Yep, it built fine for me!

@AliSoftware
Copy link
Owner Author

Ok made some more fixes in #131, @keith @billyto @jshier could you test by making your Podfiles point to that branch and confirm it works for y'all? If so, I'll merge and release a new version!

@billyto
Copy link

billyto commented Oct 23, 2015

Pointing at

:branch => 'fix/umbrella-header'

It worked in my tests!
screen shot 2015-10-22 at 8 53 30 pm

@jshier
Copy link

jshier commented Oct 23, 2015

Fixed my build as well.

@AliSoftware
Copy link
Owner Author

Cool! Will do a release probably in the week-end (too busy today). Thx for the feedback 🎉

@incanus
Copy link

incanus commented Oct 24, 2015

The fix branch is working great in my testing on several projects, too.

AliSoftware added a commit that referenced this issue Oct 24, 2015
@AliSoftware
Copy link
Owner Author

Version 4.4.0 is out fixing all this. Thanks to you all for the feedback!

AliSoftware added a commit that referenced this issue Nov 3, 2015
AliSoftware added a commit that referenced this issue Nov 3, 2015
@madoke
Copy link

madoke commented Feb 1, 2016

Can still reproduce this issue on XCode 7.2 / CocoaPods 0.39.0 when doing a fresh pod install (after removing the Pods folder) with the following Podfile configuration:

target 'AppCoreTests' do
    use_frameworks!

    pod 'OHHTTPStubs'
    pod 'OHHTTPStubs/Swift'
end

I can workaround it, however by commenting out the line pod 'OHHTTPStubs/Swift', doing a pod install, then uncommenting it and doing a pod install again.

@AliSoftware
Copy link
Owner Author

@madoke That's a known bug of CP 0.39 i believe.

@AliSoftware
Copy link
Owner Author

Yeah just confirmed. Should be fixed in latest betas of CP though, so I encourage you to test the latest CocoaPods 1.0.0 beta.

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

No branches or pull requests

6 participants