Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Add support to ubuntu 22.04 and 24.04 in arm64 #181

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
+18 −3
Open

Add support to ubuntu 22.04 and 24.04 in arm64 #181

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions .gitignore
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,4 @@ bin/wkhtmltopdf_ubuntu_20.04_amd64
bin/wkhtmltopdf_ubuntu_20.04_arm64
bin/wkhtmltopdf_ubuntu_21.10_amd64
bin/wkhtmltopdf_ubuntu_22.04_amd64
bin/wkhtmltopdf_ubuntu_22.04_arm64
Copy link

Choose a reason for hiding this comment

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

Did you mean this to be bin/wkhtmltopdf_ubuntu_24.04_arm64? It's the same as the line above it.

Copy link
Author

@chileung-b4b chileung-b4b Sep 12, 2024 •
edited
Loading

Choose a reason for hiding this comment

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

Did you mean this to be bin/wkhtmltopdf_ubuntu_24.04_arm64? It's the same as the line above it.

No, I think it is correct. The line above is ignoring the amd64 one whereas this is for arm64. The file that should be ignored comes from https://github.com/zakird/wkhtmltopdf_binary_gem/pull/181/files#diff-5c40b9ebb9c6c8386d46fdafd1a1b8e564c5f6c6bc719676a2ce59337f8ea1cc when uncompressed (which happens when you run

wkhtmltopdf_binary_gem/bin/wkhtmltopdf

Line 89 in a29aa7d

Zlib::GzipReader.open("#{binary}.gz") { |gzip| file << gzip.read }
)

Also, it's only ignoring this specific one because for 24.04, it would use the 22.04 binary

Copy link
Collaborator

Choose a reason for hiding this comment

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

@drsharp I missed it at first too, it's not the 22.04 part, it's the arm64 vs amd64 part.

Copy link

Choose a reason for hiding this comment

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

Aha! My old eyes. 👓 🤦

I see it now. ✔️

Copy link
Author

Choose a reason for hiding this comment

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

I'm glad it is resolved :)

Does this PR need any additional changes before it's ready to be merged @unixmonkey?

Binary file added bin/wkhtmltopdf_ubuntu_22.04_arm64.gz
View file
Open in desktop
Binary file not shown.
14 changes: 14 additions & 0 deletions docker-compose-arm.yml
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -28,3 +28,17 @@ services:
dockerfile: .docker/Dockerfile-debian_12_arm
volumes:
- .:/root/wkhtmltopdf_binary_gem

ubuntu_22.04:
build:
context: .
dockerfile: .docker/Dockerfile-ubuntu_22.04
volumes:
- .:/root/wkhtmltopdf_binary_gem

ubuntu_24.04:
build:
context: .
dockerfile: .docker/Dockerfile-ubuntu_24.04
volumes:
- .:/root/wkhtmltopdf_binary_gem
6 changes: 3 additions & 3 deletions test/test_with_docker.rb
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -54,12 +54,12 @@ def test_with_ubuntu_20
end

def test_with_ubuntu_22
test_on_x86 with: 'ubuntu_22.04'
test_on_x86_and_arm with: 'ubuntu_22.04'
end

def test_with_ubuntu_24
test_on_x86 with: 'ubuntu_24.04'
end
test_on_x86_and_arm with: 'ubuntu_24.04'
end

def test_with_archlinux
test_on_x86 with: 'archlinux'
Expand Down