-
Notifications
You must be signed in to change notification settings - Fork 225
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
Ios12commoncrypto #114
base: master
Are you sure you want to change the base?
Ios12commoncrypto #114
Conversation
- remove CommonCrypto file(s) from the file system - added new aggregate target which conditionally builds CommonCrypto - link to that target in all 4 of our targets Per: https://stackoverflow.com/a/42852743/108859 re kylef#102
- no additional target - run script before Compile Sources re kylef#102
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 better solution, compatibility for old version
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.
Great work. This should work.
Can we get this merged, please? |
@kylef Can you accept this PR? Xcode 10 GM is already released Thanks |
@radianttap could you resolve conflicts? |
@quver I would gladly, but not sure how. What I should do..? In master there is no project file. This whole fix is actually updating the project file. |
Have you tried to merge kylef:master to your branch? |
Just tried locally and ends up with unbuildable mess. :( Conflict resolution suggests deleting the pbxproj file. There is no solution I can pull of here. The problem here is that kylef/jwa branch (which seems like a private dev branch) was cherry picked into master for no reason I can understand. |
Should simplify CocoaPods integration
c6e764e
Take a look at the master branch on my fork, where I did just that - reverted to mentioned commit and merge ios12commoncrypto branch into it. It merges without conflicts. |
@radianttap It breaks compatibility with Xcode 9.4.1 for me. Is it expected ? |
@jeannustre From last week yes. My fork/master is my personal space :) thus I already updated it to Swift 4.2. Also with podspec branch, which I'm personally using in a client project. I could not wait anymore, I needed this to work in Xcode 10, Swift 4.2. The ios12commoncrypto branch remains intact so it could be merged here (after proposed changes to the master here). |
@radianttap About podspec file, Is this line correct if the CommonCrypto folder was deleted? spec.preserve_paths = 'CommonCrypto/{shim.h,module.modulemap}' thks a lot |
Probably not. I did not see any error, thus |
Exists compatibility with Xcode 9.4.1 and Xcode 10.0 if we add next line in podspec:
|
Please guys, merge this... |
Hi, we are providing a third-party library which contains a dependency to JSONWebToken. Could we help in any way to make this fix merged? |
Anything we can do to move this along? |
Just another bump :) |
This approach, as explained in this SO answer works rather well. It checks if CommonCrypto is available and if yes (on iOS 12) it does nothing.
On earlier version it automatically creates CommonCrypto module map.
Fixes #102