Fix libheif build #42
Conversation
Per John Cupitt, these are not necessary #41 (comment)
brandoncc
force-pushed
the
fix-libheif-build
branch
from
0d7bddb
to
fc62dfe
Compare
John Cuppit (libvips creator and maintainer) found issues with the heroku-built libheif and the compiled version of pdfium. He recommended switching back to poppler as a result. #41 (comment) #41 (comment)
The stacks are getting more and more native libraries that we depend on installed by Heroku. This allows us to remove them from our build and just include the -dev packages that match the runtime packages that Heroku installed.
brandoncc
force-pushed
the
fix-libheif-build
branch
from
87c5272
to
c010f98
Compare
container/Dockerfile.heroku-20
Outdated
| @@ -39,7 +39,11 @@ RUN apt-get install -y \ | |||
| libfftw3-dev \ | |||
| libopenexr-dev \ | |||
| libgirepository1.0-dev \ | |||
| libmagickcore-dev | |||
| libmagickcore-dev \ | |||
There was a problem hiding this comment.
Is this needed? It might be safer without it.
There was a problem hiding this comment.
I don't know, to be honest. I think that this is used as a fallback for some file types, is that right? I've been worried about negatively affecting users of the buildpack by removing it, which is why I haven't. What do you think the chance of a negative impact is?
There was a problem hiding this comment.
It's still used for very minor things like BMP and ICO, but nothing more important than that I think.
Getting rid of imagemagick and its patchy security record was one of the drivers behind rails switching to libvips, so I think it probably shouldn't be part of the libvips build.
There was a problem hiding this comment.
That's a great point, I didn't know that. I'm comfortable with that small chance of a negative impact 👍
container/Dockerfile.heroku-22
Outdated
| libxml2-dev \ | ||
| libfftw3-dev \ | ||
| libopenexr-dev \ | ||
| libgirepository1.0-dev \ |
There was a problem hiding this comment.
Oh, thanks for pointing that out. I searched for "intro" and didn't find anything...I forgot it was just gi.
container/Dockerfile.heroku-22
Outdated
| libfftw3-dev \ | ||
| libopenexr-dev \ | ||
| libgirepository1.0-dev \ | ||
| libmagickcore-dev \ |
container/Dockerfile.heroku-22
Outdated
| wget \ | ||
| python3-pip \ | ||
| ninja-build \ | ||
| intltool \ |
There was a problem hiding this comment.
I thought so, but wasn't sure. I guess I could have removed it and just looked for a failing build.
container/Dockerfile.heroku-20
Outdated
|
|
||
| # clean and package | ||
| ARG STACK_VERSION | ||
| RUN cd $PREFIX \ | ||
| && rm -rf bin/gif* bin/orc* bin/gsf* bin/batch_* bin/vips-* \ | ||
| && rm -rf bin/vipsprofile bin/light_correct bin/shrink_width \ | ||
| && rm -rf bin/gif* \ |
There was a problem hiding this comment.
I don't think this is needed
There was a problem hiding this comment.
I thought it might have been needed for the cgif build, thanks for pointing that out. I see that bin/vipsprofile does still exist. You used to recommend deleting that, should we keep deleting it?
There was a problem hiding this comment.
Oh I missed that. Yes, I would delete vipsprofile too.
One of the drivers of Rails switching to libvips was the security risks posed by imagemagick. It would be better if we don't add that risk back to Rails apps by including it in the libvips build.
As reported in #41, the build was broken when I rebuilt the configuration to work with libvips 8.14 by using meson. I missed one variable rename, which caused the libheif shared library to be put in the wrong place.
I also updated the libheif build to remove unnecessary extras, per
@jcupittin #41 (comment)Closes #41