Newlywords / heroku-buildpack-vips

Heroku buildpack with vips and pdf support via poppler
MIT License
39 stars 81 forks source link

HEIF/HEIC support seems broken #41

Closed davidcelis closed 1 year ago

davidcelis commented 1 year ago

I've been trying to get HEIC/HEIF images working in both your buildpack as well as https://github.com/hardpixel/heroku-buildpack-vips, but to no avail. I noticed you opened libvips/libvips#1945 at one point but, presumably, you then got libheif working properly with Vips? Does it still work for you by any chance? I was trying to debug this in both buildpacks but couldn't figure it out. The other buildpack wasn't reporting any helpful info (aside from libheif being installed and listed places like vips --vips-config and vips -l. However, when I tried the same commands with your buildpack, I actually do get a warning about the heif module when trying to run any vips command. For example:

$ dokku run vips --vips-config

(process:13): VIPS-WARNING **: 18:51:11.173: unable to load "/app/vendor/vips/lib/vips-modules-8.14/vips-heif.so" -- libheif.so.1: cannot open shared object file: No such file or directory
enable debug: false
enable deprecated: true
enable modules: true
enable cplusplus: true
enable RAD load/save: true
enable Analyze7 load/save: true
enable PPM load/save: true
enable GIF load: true
use fftw for FFTs: true
accelerate loops with ORC: true
ICC profile support with lcms: true
zlib: true
text rendering with pangocairo: true
font file support with fontconfig: true
EXIF metadata support with libexif: true
JPEG load/save with libjpeg: true
JXL load/save with libjxl: false (dynamic module: false)
JPEG2000 load/save with OpenJPEG: false
PNG load/save with libspng: true
PNG load/save with libpng: false
selected quantisation package: imagequant
TIFF load/save with libtiff: true
image pyramid save with libgsf: true
HEIC/AVIF load/save with libheif: true (dynamic module: true)
WebP load/save with libwebp: true
PDF load with PDFium: true
PDF load with poppler-glib: false (dynamic module: false)
SVG load with librsvg: true
EXR load with OpenEXR: true
OpenSlide load: false (dynamic module: false)
Matlab load with libmatio: false
NIfTI load/save with niftiio: false
FITS load/save with cfitsio: false
GIF save with cgif: true
selected Magick package: MagickCore (dynamic module: true)
Magick API version: magick6
Magick load: true
Magick save: true

Did something maybe change in your Dockerfile after you originally added Vips support? I can reproduce these errors in a fresh Rails application simply by adding ruby-vips to the Gemfile and then adding your buildpack to index 1 before pushing to my Dokku server.

davidcelis commented 1 year ago

Ahh, so I guess that that VIPS-WARNING load error was because I didn't add the apt buildpack with an Aptfile containing libheif-dev. Once I did that, the load error went away and vips -l correctly lists VipsForeignLoadHeif and VipsForeignSaveHeif. I think I had assumed that the Aptfile was no longer necessary because I saw libheif-related build items in the main Dockerfile now.

However, even after adding the Aptfile and installing libheif-dev manually, opening up a Rails console and running Vips.get_suffixes returns a list that doesn't contain .heif or .heic, so I get the same VipsForeignLoad error you described in your original issue on the libvips repo.

brandoncc commented 1 year ago

Thanks for reporting this. I did recently change the way libvips is built, which changed the build of libheif from a static library to a shared library. It seemed like this worked fine in my testing, but maybe I missed something. If you would like to test the buildpack before my changes, you can add the buildpack to an app using the URL:

https://github.com/brandoncc/heroku-buildpack-vips#154584a37ef2ab945de5156bd54de7267c327ea2

I would love to know if this resolves your issue, if you would be willing to test.

jcupitt commented 1 year ago
$ dokku run vips --vips-config

(process:13): VIPS-WARNING **: 18:51:11.173: unable to load "/app/vendor/vips/lib/vips-modules-8.14/vips-heif.so" -- libheif.so.1: cannot open shared object file: No such file or directory

By default, libvips compiles the heif loader and saver as a plugin (or module). This is supposed to make it easy to add and remove formats at runtime.

So ... libvips is trying to load the vips module /app/vendor/vips/lib/vips-modules-8.14/vips-heif.so, and in turn that is trying to load libheif.so.1, the libheif library, and failing.

I would check where libheif.so.1 is on your system, then check that that directory is on your library search path. Maybe running ldconfig would be enough?

The build dockerfile has:

ARG HEIF_VERSION=1.10.0
ARG HEIF_URL=https://github.com/strukturag/libheif/releases/download

RUN wget ${HEIF_URL}/v${HEIF_VERSION}/libheif-${HEIF_VERSION}.tar.gz \
  && tar xf libheif-${HEIF_VERSION}.tar.gz \
  && cd libheif-${HEIF_VERSION} \
  && ./configure --prefix=$VIPS_PREFIX \
  && make V=0 \
  && make install

Should that be ./configure --prefix=$PREFIX? I'm probably missing something.

You could also remove the examples and pixbuf support, eg.:

./configure --prefix=$PREFIX --disable-examples --disable-go --disable-gdk-pixbuf
davidcelis commented 1 year ago

@brandoncc Specifying that SHA did indeed work 🎉 Weirdly enough, Vips.get_suffixes still wasn't listing .heic and .heif, but I was able to load an HEIC file despite that. Still, the surprising get_suffixes behavior lead me to test the latest ref one last time just to be sure and it does raise the VipsForeignLoad, so it does seem that the shared library changes are at play.

@jcupitt thanks for your insights (and sorry about also opening that issue in the ruby-vips repo; i just honestly wasn't sure where the problem was 😓 I think you're probably right about the prefix being wrong! $VIPS_PREFIX doesn't seem to be set anywhere so I'm building a new version of the buildpack with that change and can report back once I've tested it out.

davidcelis commented 1 year ago

Well, maybe not that simple… Changing $VIPS_PREFIX to $PREFIX caused Vips to fail to build and test: https://gist.github.com/davidcelis/c658f1c7f3b3735429b22dbcb6c73b17

brandoncc commented 1 year ago

Thank you both for the further debugging. I suspect it is the variable rename that I missed when I reworked the Dockerfile as part of making this buildpack successfully build libvips 8.14.x. I have opened a PR with the fix.

@davidcelis Can you try with commit hash 3af8c48b1b1935087c3775940247663123fae459 and let me know if that resolves your issue?

davidcelis commented 1 year ago

@brandoncc No dice 😓 I added https://github.com/brandoncc/heroku-buildpack-vips#3af8c48b1b1935087c3775940247663123fae459 but still get the VipsForeignLoad error.

brandoncc commented 1 year ago

Thanks for trying, I'll see what else I can figure out using John's advice above.

brandoncc commented 1 year ago

The test I used before I released the 8.14 update with the new build process was from John's comment here. I do:

wget http://www.rollthepotato.net/~john/demo.heic
vips copy demo.heic x.jpg

This test still works fine, no errors.

I couldn't identify the x.jpg file signature using https://en.wikipedia.org/wiki/List_of_file_signatures, but I can tell that the bytes are changing by using:

~ $ head -c 20 demo.heic
$ftypheicmif1

~ $ head -c 20 x.jpg
����:ExifMM

When I run the lines below, however, I do get an error:

~ $ bin/rails c
Loading production environment (Rails 7.0.7.2)
irb(main):001:0> Vips::Image.new_from_file('demo.heic')
/app/vendor/bundle/ruby/3.2.0/gems/ruby-vips-2.1.4/lib/vips/image.rb:281:in `new_from_file': VipsForeignLoad: "demo.heic" is not a known file format (Vips::Error)

I can load the copied x.jpg file fine, though:

irb(main):002:0> Vips::Image.new_from_file('x.jpg')
=> #<Image 3024x4032 uchar, 3 bands, srgb>

It seems like the cli vips binary can work with heif files just fine, but ruby-vips is not picking up on the libheif library for some reason. I don't currently have any idea why this would be.

davidcelis commented 1 year ago

@brandoncc Your test went the same way for me.

@jcupitt Any insight here? Why would the vips CLI tool seem to be able to read/convert HEIC files but not the ruby bindings?

davidcelis commented 1 year ago

@brandoncc Actually, scratch that; I might've had a previous version of the buildpack still loaded 😓 Trying your test resulted in a VipsForeignLoad error for me:

$ wget http://www.rollthepotato.net/~john/demo.heic -o tmp/demo.heic
$ vips copy tmp/demo.heic tmp/x.jpg
VipsForeignLoad: "tmp/demo.heic" is not a known file format

Also, I also wanted to bring up a question from earlier again, just in case you'd missed it:

so I guess that that VIPS-WARNING load error was because I didn't add the apt buildpack with an Aptfile containing libheif-dev. Once I did that, the load error went away and vips -l correctly lists VipsForeignLoadHeif and VipsForeignSaveHeif. I think I had assumed that the Aptfile was no longer necessary because I saw libheif-related build items in the main Dockerfile now.

Given that this buildpack is attempting to build libheif itself, is sticking libheif-dev in an Aptfile still supposed to be necessary? It seems surprising, but maybe I just don't understand the difference between the buildpack compiling and installing libheif vs. installing libheif-dev via apt (or the interaction between the two).

brandoncc commented 1 year ago

@davidcelis how weird! My test was on a brand new Rails app with only the Ruby and livbips buildpacks...although I might have also had the apt community buildpack. If so, the content of Aptfile I would have been using is:

libglib2.0-0
libglib2.0-dev
libpoppler-glib8

As for your other question, I don't know unfortunately. I don't know very much about these build systems. In fact, John helped me get the very first iteration of this buildpack working because I wasn't able to from the start. I won't have any free time to look at this for the next two weeks. If you figure anything out in the meantime, I'd be happy to merge a fix sooner than that.

jcupitt commented 1 year ago

I'm not sure this is right:

$ wget http://www.rollthepotato.net/~john/demo.heic -o tmp/demo.heic
$ vips copy tmp/demo.heic tmp/x.jpg
VipsForeignLoad: "tmp/demo.heic" is not a known file format

That'll save demo.heic to the current directory and save the download log to tmp/demo.heic.

Try just:

$ wget http://www.rollthepotato.net/~john/demo.heic
... snip
$ vips copy demo.heic x.jpg
jcupitt commented 1 year ago

I had a go at an updated build for heroku22:

https://github.com/jcupitt/docker-builds/blob/master/libvips-heroku22/Dockerfile

They ship a lot of the required libraries as stack packages now, so you no longer need to build much stuff. All it makes now is libspng (and libvips, obvs). I updated pdfium too, the buildpack is using a very old version.

demo.heic seems to work fine in ruby, though I've not tried from rails.

With so many stack binaries it's nice and small now:

-rw-r--r-- 1 root root 4354142 Sep 16 04:25 libvips-heroku22.tar.gz

Just 4.4mb, and most of that is pdfium.

brandoncc commented 1 year ago

Thank you for this John, I'll take a look soon. By any chance, do you happen to know the answer to the question about Aptfile? I have a couple of questions on that topic:

Thank you for your help!

brandoncc commented 1 year ago

Also, this buildpack should still support heroku-18 and heroku-20 for current users. Do you have any idea why this is happening when the buildpack is configured as https://github.com/brandoncc/heroku-buildpack-vips#3af8c48b1b1935087c3775940247663123fae459 which uses this commit?

davidcelis commented 1 year ago

Thanks, @jcupitt! I adapted the Dockerfile in this repo based on your updated build, but left the packaging bits at the end alone. For reference, that Dockerfile is here: https://github.com/davidcelis/heroku-buildpack-vips/blob/master/container/Dockerfile

I got it to build correctly for my heroku-22 apps' containers, but I'm still not seeming to get demo.heic to work from Ruby. This works fine:

wget http://www.rollthepotato.net/~john/demo.heic
vips copy demo.heic x.jpg

But as soon as I enter a Rails console and try to work with that same file, I get foreign load errors:

Vips::Image.new_from_file(Rails.root.join("x.jpg").to_s)
# => #<Image 3024x4032 uchar, 3 bands, srgb>
Vips::Image.new_from_buffer(File.read("x.jpg"), "")
# => #<Image 3024x4032 uchar, 3 bands, srgb>

Vips::Image.new_from_file(Rails.root.join("demo.heic").to_s)
# => /app/vendor/bundle/ruby/3.2.0/gems/ruby-vips-2.1.4/lib/vips/image.rb:281:in `new_from_file': VipsForeignLoad: "/app/demo.heic" is not a known file format (Vips::Error)
Vips::Image.new_from_buffer(File.read("demo.heic"), "")
# => /app/vendor/bundle/ruby/3.2.0/gems/ruby-vips-2.1.4/lib/vips/image.rb:319:in `new_from_buffer': VipsForeignLoad: buffer is not in a known format (Vips::Error)

This happens in both my production app and that fresh Rails app that I set up just to test this HEIF/HEIC problem. That app is here, but all I did was rails new, bundle add ruby-vips, and then push it up to my dokku server.

davidcelis commented 1 year ago

@brandoncc Also, with regards to the Aptfile, I'm not sure but I suspect that adding libheif-dev to an Aptfile shouldn't be necessary… I could be wrong, but I'd expect that compiling and installing libheif from source would make an apt-get install libheif-dev superfluous? This makes me think that there's something wrong with how the buildpack's Dockerfile is doing that build step, but I have no idea what that would be… Without the Aptfile, it doesn't seem like anything related to libheif is loaded properly

jcupitt commented 1 year ago

Also, this buildpack should still support heroku-18 and heroku-20 for current users.

I agree, but for heroku22 you can make it significantly smaller since they now bundle (almost) all the runtime libs that libvips needs. It'll make maintenance simpler too, since platform security updates will (obvs.) be applied to the libvips dependencies for you.

jcupitt commented 1 year ago

heroku-18 and heroku-20 don't have libglib2.0-0 and libglib2.0-dev, but heroku-22 does. These libraries still need to be installed for the 18 and 20 stacks, right?

Yes, for 18 and 20 you'll have to build these things yourself.

is it the presence of a -dev version of a library or the non--dev library that make it so we don't need to compile with libvips? For example, is it libjpeg or libjpeg-dev that make it so that we don't need to compile libjpeg in the build?

It's the -dev, usually. If you are compiling libvips, then it'll need to have all the headers for things like libjpeg. At runtime you only need the non-dev package (provided that package is part of the platform stack, obvs).

Compile time You need the headers, the .pc file, and the library binary to build code that uses that library. These can come from compiling that package yourself, or installing the -dev package. The -dev package contains just the headers, but it will automatically pull in the library binary too.

Run time You only need the library binary. This can come from the platform, if the library is always installed by default, or from the output ogf the buildpack, if you built the library yourself.

jcupitt commented 1 year ago

Do you have any idea why https://github.com/brandoncc/heroku-buildpack-vips/issues/41#issuecomment-1705777879 is happening

I wasn't able to reproduce this. My dockerfile ^^ above works fine in this case (or seems to).

jcupitt commented 1 year ago

But I've not used rails for a while and perhaps I'm not starting it up right? You'd need to make a thing I could use to reproduce the problem.

jcupitt commented 1 year ago

Ah I do see a crash though. Am I doing anything wrong?

$ docker build -t libvips-heroku22 .
...
$ docker run -it --rm libvips-heroku22 
root@1d36798ed99c:/usr/local/src# apt install rails
... snip
root@1d36798ed99c:/usr/local/src# rails new x
... good lord, this used to be quick in rails3
root@1d36798ed99c:/usr/local/src# cd x
root@1d36798ed99c:/usr/local/src/x# bundle add ruby-vips
...
root@1d36798ed99c:/usr/local/src/x# wget http://www.rollthepotato.net/~john/demo.heic
...
root@1d36798ed99c:/usr/local/src/x# bundle exec bin/rails c
Running via Spring preloader in process 4735
Loading development environment (Rails 6.1.4.1)
irb(main):001:0> require 'vips'
=> false

That's odd, it should be true I think. I then see:

irb(main):002:0> Vips::Image.black(10,10)
=> #<Image 10x10 uchar, 1 bands, multiband>
irb(main):003:0> Vips::Image.new_from_file("demo.heic")
/var/lib/gems/3.0.0/gems/ruby-vips-2.1.4/lib/vips/operation.rb:225: [BUG] Segmentation fault at 0x0000000000000000

So it looks like it's not starting up correctly.

davidcelis commented 1 year ago

It's normal for require "vips" to return false in a Rails app; it initializes with Bundler.require so, as long as ruby-vips is in the Gemfile and not explicitly with the require: false option, it'll get required when the app boots.

Just to double check, though, I entered a non-rails IRB console and saw the same thing I was seeing in Rails:

irb(main):001:0> require "vips"
=> true

irb(main):002:0> Vips::Image.new_from_file("demo.heic")
/app/vendor/bundle/ruby/3.2.0/gems/ruby-vips-2.1.4/lib/vips/image.rb:281:in `new_from_file': VipsForeignLoad: "demo.heic" is not a known file format (Vips::Error)                                                       
        from (irb):3:in `<main>'                             
        from /app/vendor/ruby-3.2.2/lib/ruby/gems/3.2.0/gems/irb-1.6.2/exe/irb:11:in `<top (required)>'
        from /app/bin/irb:31:in `load'                       
        from /app/bin/irb:31:in `<main>
davidcelis commented 1 year ago

But I've not used rails for a while and perhaps I'm not starting it up right? You'd need to make a thing I could use to reproduce the problem.

I really wish Heroku hadn't done away with free dynos 😭 @jcupitt If you're willing and able to continue helping, I can try to spin up a fresh server with Dokku installed and get one of your public keys onto it, along with collaborator permissions on the small rails app I've been using to debug this (or you're free to operate on a fork or anything else that you wish). Then you would have access to the same problematic build/environment and maybe, together, we'd be able to find the right tweaks to the buildpack/Dockerfile that would get this working right in ruby-vips. Would that be helpful?

jcupitt commented 1 year ago

It's working for regular irb for me:

root@1d36798ed99c:/usr/local/src/x# irb
irb(main):001:0> require 'vips'
=> true
irb(main):002:0> Vips::Image.new_from_file("demo.heic")
=> #<Image 3024x4032 uchar, 3 bands, srgb>
irb(main):003:0> 
jcupitt commented 1 year ago

I'll see if I can find out why it's failing in bin/rails c. It doesn't look like quite the error you're seeing, but it's something that probably ought to work.

brandoncc commented 1 year ago

I have added both of you as collaborators to a heroku app where you can experiment:

After running heroku run bash -a libvips-test, I just replicated the issue again:

Running bash on ⬢ libvips-test... up, run.1991 (Eco)
~ $ wget http://www.rollthepotato.net/~john/demo.heic
vips copy demo.heic x.jpg
--2023-09-16 21:47:38--  http://www.rollthepotato.net/~john/demo.heic
Resolving www.rollthepotato.net (www.rollthepotato.net)... 93.93.129.149
Connecting to www.rollthepotato.net (www.rollthepotato.net)|93.93.129.149|:80... connected.
HTTP request sent, awaiting response... 200 OK
Length: 1325112 (1.3M)
Saving to: ‘demo.heic’

demo.heic                                                       100%[====================================================================================================================================================>]   1.26M  2.66MB/s    in 0.5s

2023-09-16 21:47:38 (2.66 MB/s) - ‘demo.heic’ saved [1325112/1325112]

~ $ bin/rails c
Loading production environment (Rails 7.0.7.2)
irb(main):001:0> Vips::Image.new_from_file(Rails.root.join("x.jpg").to_s)
=> #<Image 3024x4032 uchar, 3 bands, srgb>
irb(main):002:0> Vips::Image.new_from_buffer(File.read("x.jpg"), "")
=> #<Image 3024x4032 uchar, 3 bands, srgb>
irb(main):003:0> Vips::Image.new_from_file(Rails.root.join("demo.heic").to_s)
/app/vendor/bundle/ruby/3.2.0/gems/ruby-vips-2.1.4/lib/vips/image.rb:281:in `new_from_file': VipsForeignLoad: "/app/demo.heic" is not a known file format (Vips::Error)
jcupitt commented 1 year ago

Wow this is a pain to debug. For reference, I had to run without spring (it does everything in a separate persistent process which is tricky to attach to), and I had to install npm, yarn and webpacker.

apt install npm
npm install --global yarn
rails webpacker:install
DISABLE_SPRING=1 bundle exec gdb ruby

Then:

run bin/rails c

And I was able to set and catch break points in libvips.

jcupitt commented 1 year ago

I was able to find the crash when stepping through heifload:

Thread 1 "ruby" hit Breakpoint 2, vips_foreign_load_heif_build (object=0x556f1aa49120) at ../libvips/foreign/heifload.c:248
248 {
(gdb) n
249     VipsForeignLoadHeif *heif = (VipsForeignLoadHeif *) object;
(gdb) 
255     if( heif->source &&
(gdb) 
256         vips_source_rewind( heif->source ) )
(gdb) 
255     if( heif->source &&
(gdb) 
259     if( !heif->ctx ) {
(gdb) 
262         heif->ctx = heif_context_alloc();
(gdb) 
265             heif->unlimited ? USHRT_MAX : 0x4000 );
(gdb) 
264         heif_context_set_maximum_image_size_limit( heif->ctx,
(gdb) 
268             heif->reader, heif, NULL );
(gdb) 
267         error = heif_context_read_from_reader( heif->ctx, 
(gdb) 

Thread 1 "ruby" received signal SIGSEGV, Segmentation fault.
0x00007f79c35592b7 in allocator_shim::internal::PartitionFree(allocator_shim::AllocatorDispatch const*, void*, void*) () from /usr/local/lib/libpdfium.so

So PDFium and libheif are both written in C++, but pdfium is built against a different C++ runtime. You can't mix C++ compilers in an executable, so they are crashing into each other :(

Going back to poppler fixes this:

root@ae7a01aefc34:/usr/local/src/x# DISABLE_SPRING=1 bundle exec gdb ruby
GNU gdb (Ubuntu 12.1-0ubuntu1~22.04) 12.1
Copyright (C) 2022 Free Software Foundation, Inc.
License GPLv3+: GNU GPL version 3 or later <http://gnu.org/licenses/gpl.html>
This is free software: you are free to change and redistribute it.
There is NO WARRANTY, to the extent permitted by law.
Type "show copying" and "show warranty" for details.
This GDB was configured as "x86_64-linux-gnu".
Type "show configuration" for configuration details.
For bug reporting instructions, please see:
<https://www.gnu.org/software/gdb/bugs/>.
Find the GDB manual and other documentation resources online at:
    <http://www.gnu.org/software/gdb/documentation/>.

For help, type "help".
Type "apropos word" to search for commands related to "word"...
Reading symbols from ruby...
(No debugging symbols found in ruby)
(gdb) run bin/rails c
Starting program: /usr/bin/ruby bin/rails c
warning: Error disabling address space randomization: Operation not permitted
[Thread debugging using libthread_db enabled]
Using host libthread_db library "/lib/x86_64-linux-gnu/libthread_db.so.1".
[New Thread 0x7f07862a8640 (LWP 13332)]
[New Thread 0x7f07860a7640 (LWP 13333)]
[New Thread 0x7f0785ea6640 (LWP 13334)]
[New Thread 0x7f0785ca5640 (LWP 13335)]
[New Thread 0x7f0785a9a640 (LWP 13336)]
[New Thread 0x7f0785899640 (LWP 13337)]
[New Thread 0x7f0785517640 (LWP 13338)]
Loading development environment (Rails 6.1.7.6)
irb(main):001:0> 
irb(main):002:0> 
irb(main):003:0> 
irb(main):004:0> require 'vips'
=> false
irb(main):005:0> 
irb(main):006:0> 
irb(main):007:0> 
irb(main):008:0> 
irb(main):009:0> Vips::Image.new_from_file "demo.heic"
=> #<Image 3024x4032 uchar, 3 bands, srgb>
irb(main):010:0> 

Woo!

jcupitt commented 1 year ago

I updated my dockerfile, it all seems to work now:

https://github.com/jcupitt/docker-builds/blob/master/libvips-heroku22/Dockerfile

I see:

$ docker build -t libvips-heroku22 .
...
$ docker run -it --rm libvips-heroku22 
root@1d36798ed99c:/usr/local/src# apt install rails
... snip
root@1d36798ed99c:/usr/local/src# rails new x
... 
root@1d36798ed99c:/usr/local/src# cd x
root@1d36798ed99c:/usr/local/src/x# bundle add ruby-vips
...
root@1d36798ed99c:/usr/local/src/x# wget http://www.rollthepotato.net/~john/demo.heic
...
root@1d36798ed99c:/usr/local/src/x# bundle exec bin/rails c
Running via Spring preloader in process 4735
Loading development environment (Rails 6.1.4.1)
irb(main):001:0> require 'vips'
=> false
irb(main):002:0> Vips::Image.new_from_file "demo.heic"
=> #<Image 3024x4032 uchar, 3 bands, srgb>
irb(main):003:0> ^D
root@1d36798ed99c:/usr/local/src/x# 
brandoncc commented 1 year ago

irb(main):001:0> require 'vips' => false



That's odd, it should be `true` I think. I then see:

That means ruby had already required the file:

irb(main):001> require 'date'
=> true
irb(main):002> require 'date'
=> false
irb(main):002:0> Vips::Image.black(10,10)
=> #<Image 10x10 uchar, 1 bands, multiband>
irb(main):003:0> Vips::Image.new_from_file("demo.heic")
/var/lib/gems/3.0.0/gems/ruby-vips-2.1.4/lib/vips/operation.rb:225: [BUG] Segmentation fault at 0x0000000000000000

So it looks like it's not starting up correctly.

I see you figured out the cause of this seg fault, which is awesome. I wonder if it is happening because you are using the heroku-provided libheif? It doesn't happen to me with this buildpack.

I looked at your updated Dockerfile and made these changes based on it:

image

The heroku application I added you both to can now load heif files no problem:

irb(main):001:0> Vips::Image.new_from_file(Rails.root.join("x.jpg").to_s)
=> #<Image 3024x4032 uchar, 3 bands, srgb>
irb(main):002:0> Vips::Image.new_from_file(Rails.root.join("demo.heic").to_s)
=> #<Image 3024x4032 uchar, 3 bands, srgb>

I then removed -Dmodules=disabled \, which broke the loading of heif using ruby-vips again.

I can now definitively say that loading libheif as a module is breaking ruby-vips' ability to load the files.

Are there any possible downsides if I keep these lines? I don't know what these features do, so I'm not sure if any of the buildpack's users use them.

      -Dradiance=false \
      -Danalyze=false \
      -Dintrospection=false \
jcupitt commented 1 year ago

Yes, I would disable modules, and also swap back to poppler.

      -Dradiance=false \
      -Danalyze=false \

These are loaders for very obscure file formats. No one will be using them, so you might as well turn them off for safety.

      -Dintrospection=false \

This lets you remove gobject-introspection from the list of apt packages.

jcupitt commented 1 year ago

That means ruby had already required the file:

Yes, spring was confusing me :(

brandoncc commented 1 year ago

Thank you for all of your explanations and help in this issue, @jcupitt! I learned a lot while debugging this issue with you both.

@davidcelis can you check if https://github.com/brandoncc/heroku-buildpack-vips#dda65024b370265d7423c44a9059020339e23104 resolves all of your issues? That is the latest commit to #42, and I will merge the PR once you confirm that all is well.

In my testing, I wasn't using an Aptfile at all and the heic stuff worked. I don't think the Aptfile is necessary anymore for any file types, but I could be wrong about that.

brandoncc commented 1 year ago

@davidcelis I just pushed a new build commit based on John's PR review. Please use https://github.com/brandoncc/heroku-buildpack-vips#a12bf1052accdbe5776cd423f151edf05f2cb028 for testing instead.

davidcelis commented 1 year ago

@brandoncc @jcupitt It works! 🎉

herokuishuser@0284fbb3fd1b:~$ wget http://www.rollthepotato.net/~john/demo.heic
herokuishuser@0284fbb3fd1b:~$ bin/rails c
Loading production environment (Rails 7.0.7.2)
irb(main):001:0> Vips::Image.new_from_file "demo.heic"
=> #<Image 3024x4032 uchar, 3 bands, srgb>

Thank you two so much for solving this issue and for all of the help in general!

brandoncc commented 1 year ago

Thank you for testing, merged!