-
Notifications
You must be signed in to change notification settings - Fork 14
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
Dependency version bumps ahead of v8.8.0 #26
Conversation
- Update Fontconfig to 2.13.1. - Update FreeType to 2.10.0. - Update HarfBuzz to 2.4.0. - Update GDK-PixBuf to 2.38.0. - Update GLib to 2.61.0. - Update libcroco to 0.6.13. - Update libjpeg-turbo to 2.0.2. - Update libpng to 1.6.37. - Update pango to 1.43.0. - Update Pixman to 0.38.4. - Update FFTW to 3.3.8. - Update matio to 1.5.15. - Update OpenJPEG to 2.3.1. - Update Poppler to 0.76.1. - Update SQLite to 3.28.0. - Update ImageMagick to 6.9.10-42. - Enable CFITSIO and update to 3.45. - Enable FriBidi and update to 1.0.5. - Enable Orc and update to 0.4.29. - Enable libheif / libde265. - Build with Meson for dependencies that no longer support autotools. - Prefer building with CMake over autotools. - Clean-up package-vipsdev.sh. - Add peldd utility to copy only the relevant *.dll files. - Switch to MinGW-w64 with posix threads to enable C++11/C11 multithreading features.
# prefix=c:/devel/target/f3002dbeeb43fd4ea5385fb6fa4de06f | ||
# at the top ... we need to override this prefix with our own | ||
# don't use "--define-variable=prefix=/poop", some pkg-config get confused | ||
os.environ['PKG_CONFIG'] = '/usr/bin/pkg-config --define-variable prefix=' + prefix |
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.
Removed, otherwise the build of poppler fails:
https://cgit.freedesktop.org/poppler/poppler/tree/CMakeLists.txt?id=248540fbe44886f7ede290c216822df21ad06d7d#n679
@@ -15,9 +15,6 @@ fi | |||
|
|||
deps="${1:-all}" | |||
|
|||
# export this to vips.modules ... cmake needs it | |||
export BASEDIR=$basedir |
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.
Removed, I've set cmakeargs
and mesonargs
in jhbuildrc
instead:
https://github.com/libvips/build-win64/pull/26/files#diff-f876610988e7cf5e587ff77317650becR133
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.
We should update 8.8/README.md to remove the ref to this. It'll need to note that you need a recent jhbuild for meson support as well.
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.
Oh, there are loads of other things too, like pe-util. We should just point people to the Dockerfile for hints on setting up for a non-Docker build.
buildroot = None | ||
# we want to force out of tree builds ... some packages (like harfbuzz) | ||
# break with in-tree builds | ||
buildroot = os.path.join(basedir, 'build') |
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.
Changed, otherwise Harfbuzz fails:
https://github.com/harfbuzz/harfbuzz/blob/14e1fabc41a9a5ead3d91d560773050469982f54/CMakeLists.txt#L20
Matio seems now also to work with out of tree builds.
|
||
( cd $repackagedir/bin ; strip --strip-unneeded *.exe ) | ||
# TODO Do we need to keep /share/doc and /share/gtk-doc? | ||
rm -rf $repackagedir/share/{aclocal,bash-completion,doc,glib-2.0,gtk-2.0,info,gtk-doc,installed-tests,man,thumbnailers,xml} |
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.
I've removed the gtk-doc
and doc
directories. Most people will look for the manuals online, I think.
@@ -3,275 +3,251 @@ | |||
<?xml-stylesheet type="text/xsl" href="moduleset.xsl"?> | |||
<moduleset> | |||
|
|||
<!-- git repos --> |
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.
Sorry for the auto-format changes. I should switch from this fancy IDE to VIM next time.
with labeled statements named `OUT`. | ||
|
||
switch `autogen-sh="autoreconf"` to `autogen-sh="configure"` | ||
once the patch is accepted by upstream. |
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.
I haven't opened a PR at OpenSlide yet.
8.8/vips.modules
Outdated
version="6.8.9" | ||
/> | ||
module="ImageMagick-6.9.10-42.tar.xz" | ||
version="6.9.10-42"/> |
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.
Version bumped a 4-year-old dependency, it'll reduce the attack surface (the latest versions of ImageMagick are continuous fuzzed by OSS-Fuzz).
The only disadvantage of this version bump is that it can suddenly break a build, see:
libvips/build-win64-mxe#7
(ImageMagick removes older versions on their website).
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.
We were stuck on an old imagemagick because of intsafe. I guess this in mingw now?
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.
I think this was fixed with ImageMagick 6.9.5-3 (released on 22 July 2016). They have excluded the intsafe.h
header for MinGW-w64:
ImageMagick/ImageMagick6@23a6645#diff-6a582070a57b62127f59170511cae756R25
@@ -693,56 +592,41 @@ | |||
</dependencies> | |||
</autotools> | |||
|
|||
<!-- build vips with orc and you get segv if you try to use it, not tried | |||
to track down why, leave it off now for |
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.
I couldn't reproduce the segv.
</autotools> | ||
|
||
<!-- cfitsio can't make a .dll with mingw ... only include it in nip2 builds | ||
where we don't care about making a libvips.dll |
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.
This is fixed with the cfitsio-3-fixes.patch
.
@@ -770,60 +651,44 @@ | |||
|
|||
--> | |||
|
|||
<!-- glib 2.55.0 is available but the g_stat refactoring is slightly buggy | |||
https://github.com/GNOME/glib/commit/53bd6a359f2c48e7729f89902097c892c8aa6fea |
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.
I'm unsure if the g_stat
is still buggy, perhaps it has been fixed with:
GNOME/glib@d3d6ef6
/cc @lovell
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.
I hope so; the sharp unit tests will fail if not so there's one way to find out ;)
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.
I downloaded the libvips-8.7.4-win32-x64.tar.gz
tarball and replaced all lib/*.{dll,lib}
and include/*
files with the 8.8.0-rc1 (-web
) binaries from this PR. Then I re-packed it and uploaded it to S3.
The following commands:
# Note: remove C:\Users\[user]\AppData\Roaming\npm-cache\_libvips before running these commands.
git clone https://github.com/lovell/sharp.git
cd sharp
npm config set sharp_dist_base_url "https://libvips-packaging.s3.amazonaws.com/"
npm install --build-from-source
npm test
Outputs:
...
info sharp Downloading https://libvips-packaging.s3.amazonaws.com/libvips-8.7.4-win32-x64.tar.gz
...
796 passing (48s)
Hooray! 🎉
If you want to test this as well, you can download the binaries of this PR here:
https://libvips-packaging.s3.amazonaws.com/vips-dev-w64-web-8.8.0.zip
https://libvips-packaging.s3.amazonaws.com/vips-dev-w64-all-8.8.0.zip
This is fantastic Kleis! I feel we all owe you pretty much a lake of beer. I'll try to read through this over the weekend. |
- reformat Dockerfile in a slightly more standard way - clean build dir as well - update 8.8/README - fetch imagemagick.gar.gz from github releases - move the transform patch in tree
version="6.9.10-42"/> | ||
module="6.9.10-43.tar.gz" | ||
version="6.9.10-43" | ||
checkoutdir="ImageMagick6-6.9.10-43"/> |
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.
-42 has already been deleted :-( perhaps downloading from the github repo will be safer.
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.
Indeed downloading from the GitHub repo will be safer. I didn't try it because I'm not sure if they're doing any post-processing on the tarballs.
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.
I assumed since they'd set up releases it was OK, and it seems to build. I'll see if I can see any differences.
I put the binaries I made with this on https://github.com/libvips/libvips/releases/tag/v8.8.0-rc1 ... I'll try to test them a bit on a win10 machine. |
@@ -951,7 +957,7 @@ | |||
module="v8.8.0-rc1/vips-8.8.0-rc1a.tar.gz" | |||
version="8.8.0" | |||
checkoutdir="vips-8.8.0"> | |||
<patch file="file:///home/john/develop/transform-7.32/transform.patch" strip="1"/> | |||
<patch file="patches/transform.patch" strip="1"/> |
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 the transform.patch
intentionally committed to version control? I could remember an old issue:
libvips/build-win32#2 (comment)
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.
It's 25 years now and he's retired, so I think he's no longer worried about this.
Running the libvips test suite with the provided pip3 install --user --upgrade -e git+https://github.com/libvips/pyvips.git#egg=pyvips[test]
git clone https://github.com/libvips/libvips.git libvips-master
python -m pytest -v libvips-master/test/test-suite Output
I think the By the way, |
This all looks great, let's merge! Nice job again Kleis. Re heifload / heifsave, let's make a new issue on libvips for that. |
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.
I've finally got around to looking properly at the proposed improvements here and all is good. Thanks Kleis for this sterling work!
Ha, John must have merged whilst I was reviewing ;) |
Sorry @lovell, I didn't realize you were planning to review too, I should have waited. |
Oh no need to wait for me, especially as I've been relatively busy recently, it was merely an amusing coincidence that the few minutes I'd found to checkout, run and test this locally were the very same minutes you chose to merge. |
@kleisauke git master (but not rc2) now spots libheif encode being disabled. It disables heifsave and the related tests. |
Quick thought: this change means the libvips org will soon be hosting binaries that can de/encode HEVC, which may infringe patents. Are we OK with this? |
Thanks for reviewing and merging! @jcupitt The @lovell I had the same thought and added some notes here: Lines 913 to 915 in 9930ae7
Partly for this reason, I did not include the libx265 dependency in the I hope that in the future, royalty-free encoding formats such as AVIF will become more widely used. |
One of the HEVC patent pools is no longer charging royalties for software decoding so the risk of distributing binaries to do this is definitely lower than it used to be. I guess the risk of patent claims will be lower than the risk of security vulnerabilities from these new, relatively large code bases. I agree that AVIF will win this battle; there are still patents but they are royalty-free. |
We were looking for "yes" from the builtin_h265_decoder variable to enable heifload, but this variable was only added in libheif 1.3, so we failed to turn on heifload correctly. We now look for not "no", ie. default on. See libvips/build-win64#26 (comment)
I made heifload/save default on, so it should now work with older libheif, good spot Kleis. I found this Nov 2016 announcement: No licence or royalty on software-only encode/decode of h265. It's designed to allow web browsers etc. to include HEVC players, and I guess encoders too. Of course I've no idea if that's enough to make us safe. |
Sadly "HEVC Advance" is only one of the HEVC patent pools/holders. The MPEG-LA pool starts to charge after 100000 "devices", which includes software, and would explain why the RedHat legal team refuse it in Fedora given the fixed US$0.20 cost per download even if that software/device is free. I guess this has to be balanced with there being little reward for and a lot of bad press towards any patent holders chasing an open source project without corporate backing for royalties. |
Argh! Software patents don't really exist in Europe. Again, no idea if that gives us any protection. It would be nice if we could move less-used loaders out to a plugin, though I think that's tricky on Windows. |
Hi John,
I had some spare-time left to make this repo more in line with the build-win64-mxe repo by updating all dependencies to the latest version (where possible), improving the existing patches and adding some new patches for dependencies that cause failures during building. I tested the binaries on the libvips test suite and did not encounter any problems.
This PR closes #22 because there's no need to switch to a new cross-environment. The only differences between this repo and build-win64-mxe are (after merge):
heifsave
because thelibx265
DLL is 14.1 MiB.I also found some motives to cross-compile with JHBuild rather than with MXE:
Improvements:
package-vipsdev.sh
.(needed by Poppler, OpenJPEG and libheif)
Dependency changes:
-web
variant:-all
variant: