test.sh Test fails on Fedora 31/Rawhide #75
Loading…
Reference in a new issue
No description provided.
Delete branch "%!s()"
Deleting a branch is permanent. Although the deleted branch may continue to exist for a short time before it actually gets removed, it CANNOT be undone in most cases. Continue?
By Georg Sauthoff on 2020-04-26T13:47:44.279Z
I've just updated the Fedora package to 0.3.4 and ran the tests:
(cf. https://download.copr.fedorainfracloud.org/results/gsauthof/fedora/fedora-31-x86_64/01350160-python-img2pdf/build.log.gz)
Similar error under rawhide.
The python unittests execute successfully.
Since the test script is redireting stderr to /dev/null potentially helpful error messages aren't part of the log:
082f999ac8/test.sh (L124)
By josch on 2020-04-26T14:20:04.803Z
Thanks for reporting that issue!
I see that you are running test.sh with
bash -x
. Note that the script does not require bash. A posix shell is sufficient. The reason that stderr is redirected to /dev/null is indeed just for brevity in the normal case. Maybe a better way would be to output status messages on stdout and debug messages on stderr -- that way one can just call./test.sh 2>/dev/null
for the short output and simply./test.sh
for verbose output. Another way would be using an environment variable or a cli switch.What is the simplest way to obtain a fedora chroot tarball? I'd like to investigate what's going on.
Did you try running the script without this redirection to /dev/null?
By Georg Sauthoff on 2020-04-26T15:47:18.834Z
Perhaps the simplest way to get a Fedora chroot is to install it with dnf, e.g. similar to this description: https://unix.stackexchange.com/a/444696/1131
A definitely faster way to get a Fedora system is to install one of the small pre-packaged virt images with
virt-builder
. You can then either run the image with qemu or copy/
out of the image into a chroot directory withvirt-copy-out
.I've removed the redirection and the resulting output shows what causes the error - the
img2pdf.py
script is interpreted as shell script. This is caused by my package stripping the bang line ofimg2pdf.py
before installing it. This is done to comply with Fedora's packaging policy, which requires that all package files are installed without such bang lines.I've patched the call in test.sh and restarted the job:
https://download.copr.fedorainfracloud.org/results/gsauthof/fedora/fedora-31-x86_64/01350921-python-img2pdf/build.log.gz
With that now the first test completes successfully, but the 2nd fails because of a grep not matching expected output, it seems.
Regarding verbose test output: I would simply use pytest for such a test suite instead of a big shell script. With pytest you can still easily execute a bunch of external commands with
subprocess
but you don't have to re-invent the wheel for counting errors, dealing with too much/not enough output, etc. You could even execute shell pipelines with e.g.subprocess....('.....', universal_newlines=True, shell=True)
. With Pytest you have this killer feature: Pytest automagically captures all the stdout/stderr of each test and only presents it if the test fails. This also works with output produced by external programs.With Pytest you would also automatically get standard features like continuing with remaining tests in case of a test failure and then get a summary of number of failed/succeeded tests. All useful stuff you don't have if you start from scratch with a shell script.
By josch on 2020-04-26T16:29:39.776Z
I'm puzzled -- how does Fedora then start any script if shebang lines must be stripped?
Yes, the grep line that fails makes sure that the resulting image indeed has orientation=6 as it was set by exiftool beforehand. Since the test works fine on Debian I'll have to create myself a fedora chroot and see what the difference is.
I agree that pytest is superior to the shell script. It's just that I currently don't have time to rewrite a 1500 line shell script in Python. If somebody has time for doing the conversion and sends in a merge request, I'll definitely accept it though.
By Georg Sauthoff on 2020-04-26T16:35:57.195Z
Well, the rule applies only to Python files that end up under
/usr/lib/.../site-packages/
not ones under/usr/*bin/
. With Python packages, the main script under/usr/*bin
usually is autogenerated bysetup.py
and just a small shim that calls the package's entry point.By josch on 2020-04-26T16:53:41.340Z
I installed the
Requires
andBuildRequires
from here: https://copr-dist-git.fedorainfracloud.org/cgit/gsauthof/fedora/python-img2pdf.git/tree/python-img2pdf.spec but when runningtest.sh
I getcmp: command not found
. I installed the packagediffutils
which fixes this problem. Maybe that's a missing build dependency?I also figured out why that
grep
fails. The imagemagick version in fedora (6.9.10.86) does not output theexif:Orientation: 6
line at all. The version in Debian is6.9.10.23
and I didn't see any such change in the changelog of imagemagick... weird...By josch on 2020-04-26T17:11:07.217Z
commit
9d184ad0cd
makes test.sh work both in debian unstable and fedora 31By Georg Sauthoff on 2020-04-26T18:55:07.581Z
diffutils
is a dependency ofrpm-build
which is guaranteed to be available in a Fedora buildroot environment. Thus, adding diffutils as explicit direct dependency to the img2pdf package would introduce a superfluous dependency.I've added your commit as a patch to my package and now the build succeeds on Fedora 31. :-)
Unfortunately, the Rawhide build breaks because of another issue: :-(
https://download.copr.fedorainfracloud.org/results/gsauthof/fedora/fedora-rawhide-x86_64/01351067-python-img2pdf/build.log.gz
What do you think? Issue inside the more recent PIL/pillow or how the 'old' API is used?
Rawhide has these versions:
https://download.copr.fedorainfracloud.org/results/gsauthof/fedora/fedora-rawhide-x86_64/01351067-python-img2pdf/root.log.gz
Whereas Fedora 31 is at Python 3.7 and Pillow 6.2.2.
By Georg Sauthoff on 2020-04-26T18:58:51.546Z
PS: Btw, my Firefox complains that some images are delivered over http for this Gitlab instance instead of https like the rest ('connection not secure').
As a consequence I can't add a thumps-up reaction to your last comment. :-)
By Georg Sauthoff on 2020-04-26T19:02:36.859Z
Ok, it's a Pillow bug, upstream fixed this in 7.1.2: https://github.com/python-pillow/Pillow/issues/4518#issuecomment-619461664
By josch on 2020-04-26T19:23:58.669Z
wrt diffutils: ah okay, we have the same in debian called
build-essential
packageswrt http: yes, the http thing comes from these pesky gravatar images... but that's not the biggest problem -- i really should update this gitlab to a more recent version...
wrt pillow: okay, so I guess that there is actually nothing more for me to do about this?
By Georg Sauthoff on 2020-04-26T19:33:09.624Z
Yes, the new Pillow version should already be available in Rawhide. I'll give it some hours to arrive in Copr. I'll retry the build tomorrow.
By Georg Sauthoff on 2020-04-28T21:18:16.528Z
I've good and bad news. ;-)
First the good: COPR builds now complete successfully, after a rawhide sync.
I've pushed the updates to the main repository.
The bad: Unfortunately, there is again a case where the test suite fails on aarch64 (under Fedora 32):
https://koji.fedoraproject.org/koji/taskinfo?taskID=43886542
i.e. see the bottom of https://kojipkgs.fedoraproject.org//work/tasks/6546/43886546/build.log:
For comparison, when I force the build to x86_64, it succeeds:
https://koji.fedoraproject.org/koji/taskinfo?taskID=43886604
By josch on 2020-04-28T21:26:06.337Z
This is indeed bad news. I wrote magick.py specifically to have control over every bit of the test input and not depend on subtle changes in imagemagick. Additionally, there is no problem on Debian arm64... sigh...
By josch on 2020-04-29T00:21:41.262Z
it's the dreaded zlib... python's
zlib.compress()
yields different results on arm64 on fedora than on x86_64...By josch on 2020-04-29T11:04:16.839Z
Stay tuned, I'm currently writing my own implementation of
zlib.compress()
in pure Python to fix this.By josch on 2020-04-29T16:43:52.890Z
With commit
559d42cd4a
the problem should be solved because the zlib payload is now generated in Python without the zlib module (which is only used for its checksum functions).By Georg Sauthoff on 2020-04-30T18:52:57.409Z
Ok, this reminds me of #51. ;)
I can confirm that the fix works with Fedora Rawhide/32 on x86_64 and aarch64.
The package push to Feodra Rawhide, 31 and 32 is now complete and the current img2pdf in available in the stable repos in 7 days or less.
By Georg Sauthoff on 2020-04-30T18:52:57.587Z
Status changed to closed
By josch on 2020-04-30T20:20:10.044Z
yup, it's precisely the problem from #51 XD
I just made a new release with this fix then you don't have to carry a patch. Thanks for verifying that this works!
By josch on 2020-05-27T22:06:15.456Z
Mentioned in merge request !6