Summary: | Warnings from GD::Barcode::QRcode on U24 | ||
---|---|---|---|
Product: | Koha | Reporter: | Jonathan Druart <jonathan.druart> |
Component: | Test Suite | Assignee: | Jonathan Druart <jonathan.druart> |
Status: | Pushed to main --- | QA Contact: | Marcel de Rooy <m.de.rooy> |
Severity: | minor | ||
Priority: | P5 - low | CC: | dcook, m.de.rooy |
Version: | unspecified | ||
Hardware: | All | ||
OS: | All | ||
GIT URL: | Change sponsored?: | --- | |
Patch complexity: | Small patch | Documentation contact: | |
Documentation submission: | Text to go in the release notes: | ||
Version(s) released in: |
25.11.00
|
Circulation function: | |
Bug Depends on: | 40348 | ||
Bug Blocks: | |||
Attachments: |
Bug 40490: Remove warnings from GD::Barcode::QRcode - U24
Bug 40490: Remove warnings from GD::Barcode::QRcode - U24 Bug 40490: Remove warnings from GD::Barcode::QRcode - U24 |
Description
Jonathan Druart
2025-07-24 05:27:59 UTC
Created attachment 184575 [details] [review] Bug 40490: Remove warnings from GD::Barcode::QRcode - U24 01:18:38 koha_1 | # Failed test 'no warnings' 01:18:38 koha_1 | # at /usr/share/perl/5.38/Test/Builder.pm line 193. 01:18:38 koha_1 | # There were 6 warning(s) 01:18:38 koha_1 | # Previous test 4 '401 Unauthorized' 01:18:38 koha_1 | # Use of uninitialized value $aMask[1079] in bitwise xor (^) at /usr/share/perl5/GD/Barcode/QRcode.pm line 217. 01:18:38 koha_1 | # at /usr/share/perl5/GD/Barcode/QRcode.pm line 217. jammy (22.04LTS) (perl): 1.15-7 noble (24.04LTS) (perl): 2.00-1 I guess it's fixed by https://github.com/mbeijen/GD-Barcode/commit/85e7911e65bf1c4746d9ce2e743580bbf51da793 But it is not released yet: version 2 is from September 2023 and commit from March 2024. Test plan: Install version 2 of GD::Barcode: `cpanm GD::Barcode@2.0.0` Run the tests Without this patch the "no warnings" test fails With this patch applied the tests pass That's an interesting one. I'll ask on their Github if they'd be willing to upload a new version to CPAN. (In reply to David Cook from comment #2) > That's an interesting one. > > I'll ask on their Github if they'd be willing to upload a new version to > CPAN. https://github.com/mbeijen/GD-Barcode/issues/10 Created attachment 184576 [details] [review] Bug 40490: Remove warnings from GD::Barcode::QRcode - U24 01:18:38 koha_1 | # Failed test 'no warnings' 01:18:38 koha_1 | # at /usr/share/perl/5.38/Test/Builder.pm line 193. 01:18:38 koha_1 | # There were 6 warning(s) 01:18:38 koha_1 | # Previous test 4 '401 Unauthorized' 01:18:38 koha_1 | # Use of uninitialized value $aMask[1079] in bitwise xor (^) at /usr/share/perl5/GD/Barcode/QRcode.pm line 217. 01:18:38 koha_1 | # at /usr/share/perl5/GD/Barcode/QRcode.pm line 217. jammy (22.04LTS) (perl): 1.15-7 noble (24.04LTS) (perl): 2.00-1 I guess it's fixed by https://github.com/mbeijen/GD-Barcode/commit/85e7911e65bf1c4746d9ce2e743580bbf51da793 But it is not released yet: version 2 is from September 2023 and commit from March 2024. Test plan: Install version 2 of GD::Barcode: `cpanm GD::Barcode@2.0.0` Run the tests Without this patch the "no warnings" test fails With this patch applied the tests pass Signed-off-by: David Cook <dcook@prosentient.com.au> I don't have a U24 system on hand, but the tests pass with the patch on D12, so there's no harm in them. -- Looking at the code, I wonder if we could have used one of Test::Output's stderr_* methods here to check if STDERR was empty or had the error we expect in U24? But this should work fine. I wonder how we'll know to undo this change when there's a newer version of GD::Barcode available? Would it be worth wrapping this in a GD::Barcode::VERSION check? (In reply to David Cook from comment #5) > Looking at the code, I wonder if we could have used one of Test::Output's > stderr_* methods here to check if STDERR was empty or had the error we > expect in U24? But this should work fine. I had a look at it last week (because there is already an occurrence of a similar problem, see bug 18772), but it's yet another dependency and so I decided to keep the STDERR redirect. But I wouldn't mind adjusting them if we have more occurrences coming. > I wonder how we'll know to undo this change when there's a newer version of > GD::Barcode available? Would it be worth wrapping this in a > GD::Barcode::VERSION check? Yes, I thought about that (again for bug 18772) but didn't find an elegant solution. What do you mean by "wrapping in a version check"? Only the expected output or to let us know the STDERR redirect can be removed? (In reply to Jonathan Druart from comment #6) > > I wonder how we'll know to undo this change when there's a newer version of > > GD::Barcode available? Would it be worth wrapping this in a > > GD::Barcode::VERSION check? > > Yes, I thought about that (again for bug 18772) but didn't find an elegant > solution. > What do you mean by "wrapping in a version check"? Only the expected output > or to let us know the STDERR redirect can be removed? Yeah, letting us know when the STDERR redirect can be removed. I was thinking maybe we could do the STDERR redirect just for $GD::Barcode::VERSION == 2.00 or something like that, so that systems using older or newer versions of GD::Barcode would still get checked by Test::NoWarnings. Otherwise, I'm not sure when we'll know to come back to undo it. But it's just a thought. (In reply to David Cook from comment #3) > (In reply to David Cook from comment #2) > > That's an interesting one. > > > > I'll ask on their Github if they'd be willing to upload a new version to > > CPAN. > > https://github.com/mbeijen/GD-Barcode/issues/10 Michiel was super responsive, and he's uploaded a version 2.01 to CPAN with the fix. Probably not worth packaging it ourselves just to solve this problem, but I'll ask the Debian maintainer if they can try out the new version, so that it flows through at some point. (In reply to David Cook from comment #8) > (In reply to David Cook from comment #3) > > (In reply to David Cook from comment #2) > > > That's an interesting one. > > > > > > I'll ask on their Github if they'd be willing to upload a new version to > > > CPAN. > > > > https://github.com/mbeijen/GD-Barcode/issues/10 > > Michiel was super responsive, and he's uploaded a version 2.01 to CPAN with > the fix. > > Probably not worth packaging it ourselves just to solve this problem, but > I'll ask the Debian maintainer if they can try out the new version, so that > it flows through at some point. I've emailed a Debian bug report asking for a version bump and potentially a backport. We'll see how that goes. It's not showing up on bugs.debian.org yet, but hopefully it will. Looks like bug 18772 is another case where the bug is already fixed upstream but hasn't flowed through to CPAN and Debian. Weird. Having issues with get_session called in two_factor_auth.t. Seems unrelated to this patch. Getting no session while passing empty string? (In reply to Marcel de Rooy from comment #11) > Having issues with get_session called in two_factor_auth.t. > Seems unrelated to this patch. > Getting no session while passing empty string? Resolved. Interference with development on koha scripts, changing paths ;) Created attachment 184613 [details] [review] Bug 40490: Remove warnings from GD::Barcode::QRcode - U24 01:18:38 koha_1 | # Failed test 'no warnings' 01:18:38 koha_1 | # at /usr/share/perl/5.38/Test/Builder.pm line 193. 01:18:38 koha_1 | # There were 6 warning(s) 01:18:38 koha_1 | # Previous test 4 '401 Unauthorized' 01:18:38 koha_1 | # Use of uninitialized value $aMask[1079] in bitwise xor (^) at /usr/share/perl5/GD/Barcode/QRcode.pm line 217. 01:18:38 koha_1 | # at /usr/share/perl5/GD/Barcode/QRcode.pm line 217. jammy (22.04LTS) (perl): 1.15-7 noble (24.04LTS) (perl): 2.00-1 I guess it's fixed by https://github.com/mbeijen/GD-Barcode/commit/85e7911e65bf1c4746d9ce2e743580bbf51da793 But it is not released yet: version 2 is from September 2023 and commit from March 2024. Test plan: Install version 2 of GD::Barcode: `cpanm GD::Barcode@2.0.0` Run the tests Without this patch the "no warnings" test fails With this patch applied the tests pass Signed-off-by: David Cook <dcook@prosentient.com.au> Signed-off-by: Marcel de Rooy <m.de.rooy@rijksmuseum.nl> Thanks for all the upstream work, David! Nice work everyone! Pushed to main for 25.11 |