zakird / wkhtmltopdf_binary_gem

Ruby gem containing easily installable access to wkhtmltopdf application
https://rubygems.org/gems/wkhtmltopdf-binary
Apache License 2.0
184 stars 346 forks source link

wrong logic for AMZN_2 validation #124

Closed catsky closed 6 months ago

catsky commented 2 years ago

//for amz 1: os = "amzn_2018.03", // for amz 2: os="amzn2.0 (2017.12)" os = `. /etc/os-release 2> /dev/null && echo ${ID}${VERSION_ID}`.strip

//it's wrong to check which amzn version using the following line, which return false for both amzn versions os.startwith?('amzn') && !os.start_with?('amzn_2')

FYI

amz linux 1

[ec2-user@ip-x-x-x- ~]$ cat /etc/os-release
NAME="Amazon Linux AMI"
VERSION="2018.03"
ID="amzn"
ID_LIKE="rhel fedora"
VERSION_ID="2018.03"
PRETTY_NAME="Amazon Linux AMI 2018.03"
ANSI_COLOR="0;33"
CPE_NAME="cpe:/o:amazon:linux:2018.03:ga"
HOME_URL="http://aws.amazon.com/amazon-linux-ami/"

amz linux 2


[ec2-user@x-x-x-x ~]$ cat /etc/system-release
Amazon Linux release 2.0 (2017.12) LTS Release Candidate
[ec2-user@fresh-amazon-host ~]$ cat /etc/os-release
NAME="Amazon Linux"
VERSION="2.0 (2017.12)"
ID="amzn"
ID_LIKE="centos rhel fedora"
VERSION_ID="2.0"
PRETTY_NAME="Amazon Linux 2.0 (2017.12) LTS Release Candidate"
ANSI_COLOR="0;33"
CPE_NAME="cpe:2.3:o:amazon:amazon_linux:2.0"
HOME_URL="https://amazonlinux.com/"```
pedrofurtado commented 2 years ago

Hello, @catsky ! 👋

Thanks for contact us.

Recently, we've merged a PR that fixes this kind of verification: https://github.com/zakird/wkhtmltopdf_binary_gem/pull/120

It's present in master branch (but not in rubygems yet). Can you test to check if works in your scenario?

🍻

catsky commented 2 years ago

cool, yes, it works if using the master branch

pedrofurtado commented 2 years ago

That's a great news 🎉

@unixmonkey Maybe this can be a good time for an official release of gem, what do you think about?

We can try the automated way to release the gem, using this PR: https://github.com/zakird/wkhtmltopdf_binary_gem/pull/116

tricknotes commented 2 years ago

a good time for an official release of gem

+1 for me.

@unixmonkey How do you think about this idea?

joeyparis commented 2 years ago

https://wkhtmltopdf.org/downloads.html now has binaries specifically for Amazon Linux 2. The current gem has amzn_2 defined as an os based on centos_7, which may be true but now that wkhtmltopdf supports Amazon Linux 2 directly I feel that would be a better binary to build off

unixmonkey commented 6 months ago

I'm pretty sure this was fixed already in a prior version of the gem, but I'm just now noticing this issue as I'm going through and closing ones for version 0.12.6.7. Please reopen if you find there's still an issue.