-
Notifications
You must be signed in to change notification settings - Fork 296
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
Add custom FBLPromises modulemap to cocoapods #131
base: master
Are you sure you want to change the base?
Conversation
Thanks for your pull request. It looks like this may be your first contribution to a Google open source project (if not, look below for help). Before we can look at your pull request, you'll need to sign a Contributor License Agreement (CLA). 📝 Please visit https://cla.developers.google.com/ to sign. Once you've signed (or fixed any issues), please reply here with What to do if you already signed the CLAIndividual signers
Corporate signers
ℹ️ Googlers: Go here for more info. |
@googlebot I signed it! |
All (the pull request submitter and all commit authors) CLAs are signed, but one or more commits were authored or co-authored by someone other than the pull request submitter. We need to confirm that all authors are ok with their commits being contributed to this project. Please have them confirm that by leaving a comment that contains only Note to project maintainer: There may be cases where the author cannot leave a comment, or the comment is not properly detected as consent. In those cases, you can manually confirm consent of the commit author(s), and set the ℹ️ Googlers: Go here for more info. |
@googlebot I consent |
CLAs look good, thanks! ℹ️ Googlers: Go here for more info. |
PromisesObjC.podspec
Outdated
s.xcconfig = { | ||
'HEADER_SEARCH_PATHS' => "\"${PODS_TARGET_SRCROOT}/Sources/#{s.module_name}/include\"" | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Will this still work if we remove s.xcconfig
and use s.pod_target_xcconfig
instead?
PromisesObjC.podspec
Outdated
@@ -23,9 +23,14 @@ Pod::Spec.new do |s| | |||
s.public_header_files = "Sources/#{s.module_name}/include/**/*.h" | |||
s.private_header_files = "Sources/#{s.module_name}/include/FBLPromisePrivate.h" | |||
s.source_files = "Sources/#{s.module_name}/**/*.{h,m}" | |||
s.module_map = "Sources/#{s.module_name}/include/framework.modulemap" | |||
s.preserve_path = "Sources/#{s.module_name}/include/framework.modulemap" |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Is this needed?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hi @temrich! do you specifically mean preserve_path
or do you mean both preserve_path
and module_map
?
Ill try playing with a combination of both.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
preserve_path
, but also curious why module_map
is needed as well. Also, which version of CocoaPods are you using?
Thank you for the PR! Left a couple of comments. Are you able to verify that these changes work for iOS as well? |
Thanks @temrich, Ill give each one of your comments a go, and get back to you. |
@temrich I updated the PR with only the lines necessary for the module map to be included in the pod. Let me know if there's anything else I can do to help! |
Thank you! Were you able to verify that this change works for both macOS and iOS? Also, which version of CocoaPods are you using? |
@temrich I tried it on osx, and ios (and tvos just for good measure). Im currently on cocoapods |
Thank you for verifying! Any idea why specifying the |
@temrich I can't explain why this only affects osx. The issue peaked my understanding of cocoapods and Xcode builds. This isn't a problem with Carthage or SPM, but im guessing that's because they both are using the project or targets to actually build the framework (which defines and uses the custom modulemap). In the case of cocoapods. it seems to be creating the project from the source files (including creating its own modulemap), without taking anything from the project/target itself. Why this seems to affect osx and not any other platform is beyond my understanding. I tried to get to the root cause, but didnt come up with anything |
I tried looking into this a bit more. It's interesting, I am seeing test errors when I run: but not when I run:
Do you see the same? |
Hi @temrich I just did a clean checkout and im not seeing error?
Again, I'm on cocoapods 1.7.5. If there's a version you'd like me to test with, im more than happy to do so. |
Also, did you mean to run |
I am using 1.7.5 as well. @paulb777, anything else we should try to verify to determine whether this is a CocoaPods bug or not? |
The default Xcode version might make a difference |
Hi @paulb777 I tried it with Xcode 11 and Xcode 10.3, both seem to give me the error without patch, and pass with the patch. |
Hey, I think this also causes missing modulemaps when using Xcode's preprocessing of a file, error: Module map file '~/Library/Developer/Xcode/DerivedData/app-aqiyiglyrsbmvzhjitgjbclucaxz/Build/Products/AppStore-iphonesimulator/PromisesSwift/Promises.modulemap' not found Is there any reason not to merge this PR? |
dyld[50618]: Library not loaded: @rpath/FBLPromises.framework/Versions/A/FBLPromises |
This is to address the issue described in this ticket: #130