Update wkhtmltopdf to support Debian 13 #192
Conversation
|
@zakird is this the only change needed to support a new version? |
bruno-costanzo
force-pushed
the
patch-1
branch
from
77ec115 to
311309b
Compare
bin/wkhtmltopdf
Outdated
|
|
||
| os = 'debian_12' if !os_based_on_debian_9 && os.start_with?('debian_12') | ||
|
|
||
| os = 'debian_13' if !os_based_on_debian_9 && os.start_with?('debian_13') |
There was a problem hiding this comment.
This change makes it so that this script looks for a wkhtmltopdf binary named wkhtmltopdf_debian_13_amd64.gz to unpack from the bin dir in this project, but I don't see that you added one in this PR.
Sometimes though, you can run the debian_12 binary on 13. Is that how you are doing it now? Unless I'm missing something, this looks like it would fail the unless File.exist? check on line 97 below this and still output an error mesage.
There was a problem hiding this comment.
No, I think you are right and this will fail. Not sure how to address this, I'm running on a macbook.
There was a problem hiding this comment.
@unixmonkey, if you can give me a guideline, I will follow it
There was a problem hiding this comment.
That's kind of the problem. To get the binaries to include in this project, you'd typically download them from https://wkhtmltopdf.org/downloads.html , but there are currently no binaries for Debian 13, and GitHub is read-only because wkhtmltopdf is now a dead project.
When Debian 12 support was added here, it was only possible because the official project published debian packages for it that somehow didn't get posted to the downloads page above.
At this point, the only way to create a Debian 13-specific binary is to compile one from source, or find someone else who has already (and you can trust it to not have malware because it's not official).
But, as I said earlier, it might be possible that the Debian 12 binary still works on Debian 13, so changing it to os = 'debian_12' if !os_based_on_debian_9 && os.start_with?('debian_13') might work. I'd try that first if you can.
There was a problem hiding this comment.
So I unpacked the debian13 binary you added to this PR, and it's MD5 (1da58f1851666118321f4a6316d49ad9) is identical to the unpacked debian12 binary already in this project.
If the 12 (bookworm) binaries are compatible with 13 (trixie), then the only change that is needed is this line to:
os = 'debian_12' if !os_based_on_debian_9 && os.start_with?('debian_13')and no extra binaries. Does that work for you?
There was a problem hiding this comment.
Yes, perfectly
There was a problem hiding this comment.
Do you want me to create the PR? I guess the test file and dockerfile are still good to go
bruno-costanzo
force-pushed
the
patch-1
branch
2 times, most recently
from
e0f6cbe to
55492a0
Compare
bruno-costanzo
force-pushed
the
patch-1
branch
from
f16692d to
2a07395
Compare
bruno-costanzo
force-pushed
the
patch-1
branch
from
2a07395 to
180299a
Compare
|
This is now released in version |
|
hey @unixmonkey the gem CI is failing on two checks after this merge. |
|
@alexwbuschle It doesn't appear related (build issues) & it's been failing for over a year. If you can help fix it, it would be appreciated. |
3 participants
Issue 193
Add Support for Debian 13.