-
Notifications
You must be signed in to change notification settings - Fork 42
Rewrite the CocoaPods spec files, to declare the correct Public Headers for libwebp, fix the import issue when using use_frameworks
#2
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
Conversation
…rs for libwebp, fix the import issue when using `use_frameworks`
…e user integrate project
@bpoplauschi Another suggestion. https://chromium.googlesource.com/webm/libwebp to https://github.com/webmproject/libwebp This mirror is faster than the original source. Another reason, for China's company, this The source code between these two repo is the same, because it's just a mirror and maintained by WebM project themselves. |
This PR perfectly fixes my problem that some header files imported by libwebp itself can't be found in the case of |
@dreampiggy my 2 cents:
I remember we used to change between the chromium and the github sources for the libwebp project (on the main SDWebImage repo). Not sure which is the best, but we can take a look at that too. |
I check again about the libwebp's their CMakeFile.txt, it has actually, 4 different build target for library
Maybe I can achieve the same thing like those, with 4 subspecs (just need some time to translate the CMake into Podspec :)) But I'm wonder a problem, does this will effect the existing WebP downstream dependency. For example, our Another issue, the |
I can have a try now. I'll do my best to split them into 4 subspecs which are both modular and can be compiled seperated. But if I failed on this, we just use one spec. Actually, current subspecs does not works as you want. If you just use |
…`, `dec`, `demux` and `mux`, all of these subspecs can be compiled without extra dependency
@bpoplauschi I achieve this goal. There are 5 subspecs, reprsent the 4 components. The
All of the combination of subspecs (up to 7 combination), can be compile. And I can run The Demo project with |
@bpoplauschi I test the new podspec again. The 4 subspecs actually, can not pass So I try again. The result that I can only split them into 3 standalone sub-components:
And I test using the remote git dependency in SDWebImageWebPCoder, with some modification, the standalone
|
…ubspec includes both WebP encoding && decoding
1642707
to
90d20c0
Compare
😅Seems you're busy now. This patch can fix our feedback users's issue (including myself). Maybe we need another patch version For version, maybe it's not suitable to use something like Anyplan to merge this and push to CocoaPods trunk ? I found myself is not listed in the pod author for this
|
@dreampiggy I just added you as an owner of the CP libwebp proj |
Regarding the issue itself, I don't have the time to investigate it properly, so I'll let you decide. I would not recommend we release virtual versions (that don't exist on the real libwebp repo). |
@bpoplauschi libwebp v1.0.3 is released. I think it's the time to merge this and fix the issue ? |
Good @dreampiggy |
Podspec structure issue
Current libwebp.podspec, using this subspec to declare headers:
I guess the reason is that you want to help to solve this source code import in libwebp
However, it's useless. The correct and easy way to do so, just keep the original
src
folder structure. We have Podspec syntaxs.preserve_path
for this.Umbrella header
And, curernt libwebp does not works for
use_frameworks!
and using this code to importBecause the umbrella headers generated by CocoaPods, include all the private headers, which have their own macro to detect whether or not to import. For example,
neon.h
only can be include when build on arm64 device. This should not be exported to public.So this PR, fix all this problem, and keep compatible with current SDWebImageWebPCoder. And works for
use_frameworks!
and Swift.