Better check libzim version #1124
Conversation
kelson42
force-pushed
the
better-libzim-version-check
branch
from
70653ba
to
a3f5a65
Compare
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #1124 +/- ##
=======================================
Coverage 41.44% 41.44%
=======================================
Files 59 59
Lines 4244 4244
Branches 2322 2322
=======================================
Hits 1759 1759
Misses 990 990
Partials 1495 1495 ☔ View full report in Codecov by Sentry. |
| @@ -35,7 +35,9 @@ else | |||
| error('Cannot found header mustache.hpp') | |||
| endif | |||
|
|
|||
| libzim_dep = dependency('libzim', version : '>=9.0.0', static:static_deps) | |||
| libzim_dep = dependency('libzim', version:'>=9.0.0', static:static_deps) | |||
| libzim_dep = dependency('libzim', version:'<10.0.0', static:static_deps) | |||
There was a problem hiding this comment.
IMHO, this doesn't add a version requirement. It rather overrides the preceding one (so only version:'<10.0.0' stays in effect). Yes, the build will likely fail if libzim newer than 9.0.0 is not found, but then meson is allowed to pick up a version older than 10.0.0 (including a version lower than 9.0.0).
There was a problem hiding this comment.
The correct way is to specify multiple restrictions: version:['>=9.0.0', '<10.0.0']
I am going to fix it in #1133
There was a problem hiding this comment.
Hmmm, this approach has been validated on an other repo. by @mgautierfr. I don't remember the results of testing I made at that time... but AFAIK it tests the first one and tests then the second one... which is what we want.
@veloman-yunkan Do you have a better formalism in mind?
We check maximum version of libzim to avoid linking against a version which might have breaking changes.