Add the support of IBM Power and ARM architectures for wkhtmltopdf on Odoo 16

[ludovic-gasc]

Hi,

This pull request takes care of the CPU architecture to be able to install the right architecture version for the wkhtmltopdf package.

We have tested with Podman on RHEL 9 + IBM Power and RedHat OpenShift on IBM Power.

I have also added one line for arm64 support, but we have not tested it.

Thanks for your feedback and the merge.

[samip5]

Did you also test that PDF generation works eg invoice preview/print? I had the problem with Debian based image that I created for multi-arch, but it went away with using fedora 36 and the RPM from nightly package repo.

[d-fence]

Tested with a cross build linux/ppc64le and it worked. Same for linux/arm64. We are testing wkhtmltopdf 0.12.6.1-3 and at a first glance, it seems to be ok. But, in order to avoid breaking existing deployements, I think that it would be better to keep 0.12.5 for the amd64 platform.

[ludovic-gasc]

Hi @d-fence ,

Thanks for the tests, I have seen your comments.

About your comment using 0.12.5 instead of 0.12.6 for amd64, I fully understand the risk of regression, nevertheless, I permit you to point to the changelog: https://github.com/wkhtmltopdf/wkhtmltopdf/releases/tag/0.12.6 And the diff between 0.12.5 and 0.12.6.1: https://github.com/wkhtmltopdf/wkhtmltopdf/compare/0.12.5...0.12.6

The changes are very minimal and mainly to support ppc64le and ARM. Moreover, I'm not a big fan of using different versions depending on the architecture, we might have another behavior depending on the architecture, not the best for a multi-architecture application. Last but not least, the release date of 0.12.5 is 2018, no packages for the new stable version of Debian, 12 - bookworm, but it seems we have working bookworm packages for 0.12.6: https://github.com/wkhtmltopdf/packaging/releases/tag/0.12.6.1-3

The day you change the Debian image in the Dockerfile for the latest Debian stable, you must also use the latest version of wkhtmltopdf.

I might have another suggestion: Maybe for the next stable release of Odoo, 17, you might change the version recommended for wkhtmltopdf? Moreover, we might put the official support of the two new architectures only for Odoo 17?

Thanks for your feedback, and in case you prefer to keep 0.12.5 for amd64, then I will adapt my pull request.

[d-fence]

Hello,

I agree that the changes are minimal and the basic tests that we made did not reveal any issue. So let's go with 0.12.6.

I just ask you to remove the indentation fix to avoid useless diff.

Thanks

[ludovic-gasc]

Hi @d-fence ,

I have made the changes. FYI, we have planned to do more tests next week on IBM Power, just to be sure it's OK.

Thanks for your comments

[d-fence]

OK I will wait for the tests

[ludovic-gasc]

Hi @d-fence ,

We have done more tests and we confirm the PDF generation works pretty well on IBM Power and RedHat OpenShift. A few PDF examples generated on IBM Power are in the attachment.

INV-2023-00004 (1).pdf Production Order - WH_MO_00001.pdf Picking Operations - Wood Corner - WH_OUT_00007.pdf Delivery Slip - Wood Corner - WH_OUT_00007.pdf

Could you merge this pull request ?

Thanks.

[ludovic-gasc]

Did you also test that PDF generation works eg invoice preview/print? I had the problem with Debian based image that I created for multi-arch, but it went away with using fedora 36 and the RPM from nightly package repo.

Hi @samip5 ,

Yes, we have tested invoice preview/print, it was the main goal of this pull request :-) See my previous comment with a few PDF examples.

I have used the Debian package from the creator of this tool instead of the Debian repository: https://github.com/ludovic-gasc/docker/blob/ppc64le/16.0/Dockerfile#L45C56-L45C56

If you could test this Dockerfile on ARM, it would be awesome, I have no ARM motherboard to be able to test.

Kind regards

[ashuio]

I am getting the following error when building on x86

Running hooks in /etc/ca-certificates/update.d...
done.
+ WKHTMLTOPDF_ARCH=
+ case ${TARGETARCH} in
+ curl -o wkhtmltox.deb -sSL https://github.com/wkhtmltopdf/packaging/releases/download/0.12.6.1-3/wkhtmltox_0.12.6.1-3.bullseye_.deb
+ echo wkhtmltox.deb
+ sha1sum -c -
sha1sum: 'standard input': no properly formatted SHA1 checksum lines found

am i the only one?

I am using Docker version 24.0.5, build 24.0.5-0ubuntu1~22.04.1

[ludovic-gasc]

Hi @ashuio ,

Let me test from my side if I have the same issue, I will test this week.

Thanks for the potential bug report.

[ludovic-gasc]

Hi @ashuio , I have just tested on Ubuntu 22.04.1 and Docker 24.0.5 with snap, as suggested by the installer of Ubuntu, and it works pretty well, cf. screenshot:

Maybe your installation has an issue ? How you have installed Docker on this Ubuntu ? Could you try again, maybe on another system?

Thanks

[ludovic-gasc]

hi @d-fence , do you still see something to change before merging this pull request ?

If not, it would be wonderful if you could merge this PR :-)

Thanks, and have a nice week!

[d-fence]

Hello,

I cherry-picked your commit and merged it here d98cfdf61. I did a small change to support a default case when TARGETARCH argument is not given (I think it's the bug that was mentioned above when you try a simple docker build).

Also, I re-tested cross platform build for linux/ppc64le and linux/arm64. So the PR is waiting for a review on the Docker Hube side docker-library/official-images#15606

[ludovic-gasc]

hi @d-fence ,

Good catch for the empty variable.

We are waiting the official publication on DockerHub to do extra tests on IBM Power to be sure everything will be OK.

Thank you a lot for your help

[d-fence]

Good catch for the empty variable.

I had to change it anyway because the docker hub doesn't use TARGETARCH. So I added a fixing commit to use dpkg --print-architecture as recommended

[hoangtrann]

I tested this PR on a Mac running M1 Pro and it works like a charm. To generate the PDF, I set the report.url to http://0.0.0.0:8069 and wkhtmltopdf works without any problems.

[ludovic-gasc]

hi @d-fence , thank you a lot for your awesome help: Now that Docker Inc has merged your pull request, I have tested the official new Odoo container image on the top of RedHat OpenShift and IBM Power, it works as expected.

I confirm that the ARM and IBM Power support is now visible on the Docker Hub: https://hub.docker.com/search?q=odoo&image_filter=official

I am now this pull request.

Have a nice week