Bacon / BaconQrCode

QR Code Generator for PHP
BSD 2-Clause "Simplified" License
1.83k stars 208 forks source link

Failing tests on i386 and armhf #76

Closed williamdes closed 2 years ago

williamdes commented 3 years ago
There was 1 failure:
1) BaconQrCodeTest\Common\BitArrayTest::testGetArray
Failed asserting that -2147483647-1 is identical to 2147483648.0.
/builds/php-team/pear/php-bacon-baconqrcode/debian/output/source_dir/test/Common/BitArrayTest.php:191
There was 1 failure:
1) BaconQrCodeTest\Common\BitArrayTest::testGetArray
Failed asserting that -2147483647-1 is identical to 2147483648.0.
/builds/php-team/pear/php-bacon-baconqrcode/debian/output/source_dir/test/Common/BitArrayTest.php:191

https://salsa.debian.org/php-team/pear/php-bacon-baconqrcode/-/jobs/1048488#L1460

More errors here: https://salsa.debian.org/php-team/pear/php-bacon-baconqrcode/-/jobs/1048488#L1412

williamdes commented 3 years ago

Also armhf https://ci.debian.net/data/autopkgtest/testing/armhf/b/baconqrcode/7215168/log.gz

williamdes commented 3 years ago

Hi @ausi Would you like to solve this issue? I am asking just in case, maybe I will open a pull-request someday anyway :)

ausi commented 3 years ago

I will probably not have time for this in the near future.
Tests failing on i386 and armhf is probably due to 32bit CPU architecture, did I understand that correctly?

williamdes commented 3 years ago

I will probably not have time for this in the near future. Tests failing on i386 and armhf is probably due to 32bit CPU architecture, did I understand that correctly?

I am quite sure this is the cause too :)

DASPRiD commented 3 years ago

Indeed, 32bitr is the issue. Specifically though that PHP only has signed ints. With unsigned ints, this problem would not exist

I already had a report about that about three years ago, but closed it back then as there was no further response (and the amount of people using this library on 32-bit platforms is rather diminishing).

ausi commented 3 years ago

But you would be open to accept pull requests that make this library 32bit compatible?

DASPRiD commented 3 years ago

Sure – as long as it doesn't affect performance on 64 bit systems :+1:

williamdes commented 3 years ago

I already had a report about that about three years ago, but closed it back then as there was no further response (and the amount of people using this library on 32-bit platforms is rather diminishing).

Well phpMyAdmin's on raspberry pi's are still some good users to support :)

williamdes commented 3 years ago

I already had a report about that about three years ago

Seems it was #31 by @remicollet a RPM packager

williamdes commented 3 years ago

Well, I will have to fix this soon because it blocks my package from migrating ... https://tracker.debian.org/pkg/baconqrcode

DASPRiD commented 2 years ago

@williamdes According to your link, the package got removed by now. If that is correct and this issue does not affect you anymore, feel free to close it.

williamdes commented 2 years ago

@williamdes According to your link, the package got removed by now. If that is correct and this issue does not affect you anymore, feel free to close it.

Hi @DASPRiD I need to resume packaging this repository but the issue is still there, I would like it to be fixed. You can find how to make multi arch GitHub CIs to run tests on the phpmyadmin repository.

DASPRiD commented 2 years ago

@williamdes I gave this a quick test and tried to get it fixed, but the lack of unsigned integers makes it extremely hard to perform the required bit operations on 32-bit systems.

I'm going to close this issue for now. If you want to get this working on 32-bit systems, feel free to open a pull request, but I don't have the time for a major refactoring of the bit shifting logic (especially one which doesn't affect 64-bit systems performance-wise).

williamdes commented 2 years ago

Hi @DASPRiD I just enabled more 64 bit systems and all tests passed amd64 arm64 ppc64el Just wanted to let you know https://tracker.debian.org/pkg/baconqrcode

And even more at Ubuntu Builds