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 linking of vendored libraries in pod targets #8455
Merged
dnkoutso
merged 7 commits into
CocoaPods:1-6-stable
from
Westacular:fix_linking_frameworks_as_libraries
Jan 28, 2019
Merged
Fix linking of vendored libraries in pod targets #8455
dnkoutso
merged 7 commits into
CocoaPods:1-6-stable
from
Westacular:fix_linking_frameworks_as_libraries
Jan 28, 2019
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
…work and vendored library
…es not vendored_static_frameworks
dnkoutso
reviewed
Jan 26, 2019
dnkoutso
approved these changes
Jan 26, 2019
amorde
reviewed
Jan 26, 2019
amorde
approved these changes
Jan 26, 2019
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.
Although would like the function for library naming its not a blocker 👍
segiddins
approved these changes
Jan 27, 2019
thanks! |
dnkoutso
added a commit
to dnkoutso/CocoaPods
that referenced
this pull request
Jan 28, 2019
* 1-6-stable: Update fourflusher version Fix linking of vendored libraries in pod targets (CocoaPods#8455)
dnkoutso
added a commit
to dnkoutso/CocoaPods
that referenced
this pull request
Jan 28, 2019
* 1-6-stable: Update fourflusher version Fix linking of vendored libraries in pod targets (CocoaPods#8455)
dnkoutso
added a commit
to dnkoutso/CocoaPods
that referenced
this pull request
Jan 28, 2019
* 1-6-stable: Update fourflusher version Fix linking of vendored libraries in pod targets (CocoaPods#8455)
dnkoutso
added a commit
to dnkoutso/CocoaPods
that referenced
this pull request
Jan 28, 2019
* 1-6-stable: Update fourflusher version Fix linking of vendored libraries in pod targets (CocoaPods#8455)
dnkoutso
added a commit
to dnkoutso/CocoaPods
that referenced
this pull request
Jan 28, 2019
* 1-6-stable: Update fourflusher version Fix linking of vendored libraries in pod targets (CocoaPods#8455)
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Closes 8453.
Also, updates unit tests to avoid this in future.
Specifically: the build_settings unit tests should have caught this issue, however, the banana-lib test project that tested the relevant functionality had both Bananalib.framework and libBananalib.a, which both have the same name when stripped down, creating an ambiguity.
I've updated the banana-lib project to instead have BananaFramework.framework and libBananaStaticLib.a, and updated all the unit tests in accordance with the new names.
I've verified that the relevant build_setting test now fails without the fix, and passes with it.
One curiosity was that the
'detects duplicate library names'
test was failing following the rename, and not raising a warning as expected. Not entirely sure why that is... in any case, I've modified that test (based on the example set in'detects duplicate framework names'
) and it now passes again.