Update the libwebp dependency to support using 1.0 version and above #2625
Conversation
… which can fix some rare WebP issues
Codecov Report
@@ Coverage Diff @@
## master #2625 +/- ##
=========================================
Coverage ? 75.11%
=========================================
Files ? 37
Lines ? 4066
Branches ? 0
=========================================
Hits ? 3054
Misses ? 1012
Partials ? 0Continue to review full report at Codecov.
|
| @@ -59,6 +59,6 @@ Pod::Spec.new do |s| | |||
| 'USER_HEADER_SEARCH_PATHS' => '$(inherited) $(SRCROOT)/libwebp/src' | |||
| } | |||
| webp.dependency 'SDWebImage/Core' | |||
| webp.dependency 'libwebp', '~> 0.5' | |||
| webp.dependency 'libwebp', '>= 0.5' | |||
There was a problem hiding this comment.
... We can't do this IMO, it typically indicates no constrain about the version of libwebp. What if libwebp changed APIs in the future? we'll need to fix.
There was a problem hiding this comment.
emmmmm....
A good fix, but Is this really useful ? libwebp now have only v1.0.2 version. So even we limit it with <=2.0, this does not actually do anything.
There was a problem hiding this comment.
If you care about something in the future, people who use SDWebImage with API-breaked libwebp (maybe 2 years later after libwebp become v2.0 and change the API we use...Since Google libwebp spent 4 years to become v1.0), can still specify the version in the Podfile pod 'libwebp', '~> 1.0'
And, we will release SDWebImage 5.0.0 soon. Which the WebP coder dependency is ~> 1.0 and have the sem version to limit < 2.0...
There was a problem hiding this comment.
We assume Google not do evil things, libwebp only may changes API in 2.x, then 4.x would break, and users issue to us, we tell them how to fix them, list the libwebp version explicity or upgrade to 5.x, TBO, this is not good, from the view of project management, we need to prevent these things, in theory, my PR #2628 also needs to discuss, should we really trust libwebp would not happen very bad things like regressions until 2.0?
|
😰 |
|
@KeymonWong Any issue about this ? |
which can fix some rare WebP issues
New Pull Request Checklist
I have read and understood the CONTRIBUTING guide
I have read the Documentation
I have searched for a similar pull request in the project and found none
I have updated this branch with the latest master to avoid conflicts (via merge from master or rebase)
I have added the required tests to prove the fix/feature I am adding
I have updated the documentation (if necessary)
I have run the tests and they pass
I have run the lint and it passes (
pod lib lint)This merge request fixes / refers to the following issues: #2431
Pull Request Description
See #2431
Our current 4.x branch's WebP subspec, use the libwebp dependency like this.
However, since libwebp already bumped version to 1.0 (current latest is 1.0.2), which fix a lots of bugs of WebP decoding && performance. Some of our user and the people from my company want to upgrade to libwebp 1.0. However, current dependency resolve cause we could not upgrade the version.
So, let's update the dependency to use a looser limit of libwebp. I check the code, the libwebp 1.0 is API-compatible with 0.6.1, so it's safe to upgrade.