Skip to content

Conversation

@erikmackap
Copy link

No description provided.

natebird and others added 30 commits March 11, 2021 11:43
Ubuntu 21.04 was released today, and currently this gem errors because it's not supported. I've installed wkhtmltopdf manually on my system using the binary for Focal Fossa (20.04) and it's working fine based on my testing.
Use the 20.04 binary for 21.04
raphaelfeitoza and others added 23 commits August 15, 2024 16:30
Add support for alibaba cloud linux
Fix binary used for Amazon Linux 2023 AMI
Update CHANGELOG for version 0.12.6.8
Update wkhtmltopdf: Support KDE Neon 22.04 and 24.04
Add support to ubuntu 22.04 and 24.04 in arm64
Update wkhtmltopdf to support Debian 13
@Snyk-service-user
Copy link

Snyk-service-user commented Oct 24, 2025

Snyk checks have passed. No issues have been found so far.

Status Scanner Critical High Medium Low Total (0)
Code Security 0 0 0 0 0 issues

💻 Catch issues earlier using the plugins for VS Code, JetBrains IDEs, Visual Studio, and Eclipse.

@erikmackap erikmackap requested a review from patelvp October 24, 2025 21:07
unless File.exist? binary
raise 'Invalid platform, must be running on Ubuntu 18.04/20.04/22.04, ' \
'CentOS 7, Amazon Linux 2, or intel-based Cocoa macOS ' \
raise 'Invalid platform, must be running on Ubuntu 16.04/18.04/20.04/22.04, ' \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
raise 'Invalid platform, must be running on Ubuntu 16.04/18.04/20.04/22.04, ' \
raise 'Invalid platform, must be running on Ubuntu 16.04/18.04/20.04/22.04/24.04, ' \

os_based_on_ubuntu_20_04 = os.start_with?('ubuntu_20.')
os = 'ubuntu_20.04' if os_based_on_ubuntu_20_04

os_based_on_ubuntu_22_04 = os.start_with?('ubuntu_22.') || os.start_with?('ubuntu_24.')
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
os_based_on_ubuntu_22_04 = os.start_with?('ubuntu_22.') || os.start_with?('ubuntu_24.')
os_based_on_ubuntu_22_04 = os.start_with?('ubuntu_22.')

Why did we merge the condition here? Can we create a new line?

Copy link
Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This matches the upstream logic, which uses the 22 binary for either 22 or 24. Refer: https://github.com/zakird/wkhtmltopdf_binary_gem/blob/master/bin/wkhtmltopdf#L45-L50

@erikmackap erikmackap merged commit 772fd75 into master Oct 30, 2025
1 check passed
@erikmackap erikmackap deleted the POPS-1285-support-ubuntu-24 branch October 30, 2025 14:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.