[Bug 41076] New: Perltidy config needs to be refined to not cause changes with perltidy 20250105
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Bug ID: 41076 Summary: Perltidy config needs to be refined to not cause changes with perltidy 20250105 Initiative type: --- Sponsorship --- status: Product: Koha Version: Main Hardware: All OS: All Status: NEW Severity: normal Priority: P5 - low Component: Architecture, internals, and plumbing Assignee: koha-bugs@lists.koha-community.org Reporter: victor@tuxayo.net QA Contact: testopia@bugs.koha-community.org 1. KOHA_IMAGE=main-bookworm ktd up 2. prove xt/perltidy.t 3. leave running for like a minute, it passes, don't bother with full run 4. KOHA_IMAGE=main-trixie ktd up 5. prove xt/perltidy.t 6. failures quickly pile up D12: perltidy --version This is perltidy, v20230309 dpkg -l | grep tidy 20230309-1~koha3 (ours) D13: perltidy --version This is perltidy, v20250105 dpkg -l | grep tidy 20250105-1 So in KTD, the D13 image doesn't ship our perltidy so it uses Debian's which is more recent. And might use some different default that aren't covered by our current config. I guess it would be better to complete the config so that both version of perltidy output the same code, right? Rather than making the D13 image ship an old version of perltidy which would postpone the root issue. -- You are receiving this mail because: You are watching all bug changes. You are the assignee for the bug.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |41077 --- Comment #1 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Also, it makes the qa script fail if working on Debian 13 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41077 [Bug 41077] [OMNIBUS] Debian 13 and Perl 5.40 compatibility -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |jonathan.druart@gmail.com --- Comment #2 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Victor Grousset/tuxayo from comment #0)
So in KTD, the D13 image doesn't ship our perltidy so it uses Debian's which is more recent. And might use some different default that aren't covered by our current config. I guess it would be better to complete the config so that both version of perltidy output the same code, right?
Yes, that is what needs to be done. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |mtj@kohaaloha.com Assignee|koha-bugs@lists.koha-commun |jonathan.druart@gmail.com |ity.org | Status|NEW |ASSIGNED --- Comment #3 from Jonathan Druart <jonathan.druart@gmail.com> --- Actually there are not many changes, and they are actually bugfixes https://perltidy.sourceforge.net/ChangeLog.html I would suggest to make v20250105 our default versions for the different OSes. -- You are receiving this mail because: You are the assignee for the bug. You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Jonathan Druart <jonathan.druart@gmail.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|ASSIGNED |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #4 from Jonathan Druart <jonathan.druart@gmail.com> --- Created attachment 191896 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=191896&action=edit Bug 41076: Adjust our perl tidiness for v20250105 Generated using `perl misc/devel/tidy.pl --perl` -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #5 from Jonathan Druart <jonathan.druart@gmail.com> --- It's actually a lot of changes to test files, but they are greatly improving the readability! -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #6 from Jonathan Druart <jonathan.druart@gmail.com> --- Do not push if v20230309 is not the default everywhere! See with Mason. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #7 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Which rules actually change? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #8 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Marcel de Rooy from comment #7)
Which rules actually change? As said there are bug fixes.
I found this one: https://perltidy.sourceforge.net/ChangeLog.html 2025 01 05 - Space around here doc delimiters follow spacing controls better. For example, a space is now added before the closing paren here: OLD: (without the here doc): push( @script, <<'EOT'); NEW: push( @script, <<'EOT' ); Also, any spaces between the '<<' and here target are removed (git #174): OLD: push( @script, << 'EOT'); NEW: push( @script, <<'EOT' ); The other changes made sense to me (and felt like bug fixes as well) so it seems better to include the changes than adding options to ignore them. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #191896|0 |1 is obsolete| | -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 192028 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=192028&action=edit Bug 41076: Adjust our perl tidiness for v20250105 Generated using `perl misc/devel/tidy.pl --perl` Signed-off-by: Martin Renvoize <martin.renvoize@openfifth.co.uk> -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |martin.renvoize@openfifth.c | |o.uk Status|Needs Signoff |Signed Off --- Comment #10 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- I@m happy with the changes here.. I would have liked to indent params in search calls, but perltidy can't do that consistently.. We could add nesting to our code to get around it, but that also feels a bit dirty for little reward. Certainly need to wait for Mason to get the go ahead before pushing this one though. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #11 from Jonathan Druart <jonathan.druart@gmail.com> --- Do not push if v20250105 is not the default everywhere! See with Mason. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #12 from Jonathan Druart <jonathan.druart@gmail.com> --- I am now seeing this on D12: $ apt-cache policy perltidy perltidy: Installed: 20250105-1 Candidate: 20250105-1 Version table: *** 20250105-1 500 500 http://debian.koha-community.org/koha-staging dev/main amd64 Packages 100 /var/lib/dpkg/status 20220613-1 500 500 http://deb.debian.org/debian bookworm/main amd64 Packages -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #13 from Mason James <mtj@kohaaloha.com> --- (In reply to Jonathan Druart from comment #12)
I am now seeing this on D12:
$ apt-cache policy perltidy perltidy: Installed: 20250105-1 Candidate: 20250105-1 Version table: *** 20250105-1 500 500 http://debian.koha-community.org/koha-staging dev/main amd64 Packages 100 /var/lib/dpkg/status 20220613-1 500 500 http://deb.debian.org/debian bookworm/main amd64 Packages
hiya, i've added perltidy 20250105-1 to the koha-staging repo i just used debian's package, and did not build/rename a koha version -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |lucas@bywatersolutions.com Keywords| |RM_priority -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- QA Contact|testopia@bugs.koha-communit |martin.renvoize@openfifth.c |y.org |o.uk -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #14 from Mason James <mtj@kohaaloha.com> --- i've addeed this to prod APT repo, for all active branches $ apt policy perltidy perltidy: Installed: 20250105-1~koha1 Candidate: 20250105-1~koha1 Version table: *** 20250105-1~koha1 500 500 https://debian.koha-community.org/koha stable/main amd64 Packages 500 https://debian.koha-community.org/koha stable/main armhf Packages 100 /var/lib/dpkg/status 20200110-1 500 500 http://ftp.nz.debian.org/debian bullseye/main amd64 Packages 500 http://ftp.nz.debian.org/debian bullseye/main armhf Packages -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA --- Comment #15 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Checking KTD's, we now have: D13: This is perltidy, v20250105 D12: This is perltidy, v20250105 D11: This is perltidy, v20250105 U24: Fails to start on my machine U22: This is perltidy, v20250105 So.. all the KTD's now have the same modern perltidy.. which means we need these patches applied and any new code needs to by retidied on push to match the new rules if it now fails due to the new perltidy version. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|RM_priority |rel_25_05_candidate, | |rel_25_11_candidate Text to go in the| |26.05.00 release notes| | --- Comment #16 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Pushed to main for 26.05.00. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Lucas Gass (lukeg) <lucas@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Passed QA |Pushed to main -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Owen Leonard <oleonard@myacpl.org> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |david@davidnind.com --- Comment #17 from Owen Leonard <oleonard@myacpl.org> --- *** Bug 41735 has been marked as a duplicate of this bug. *** -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #18 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Martin Renvoize (ashimema) from comment #15)
Checking KTD's, we now have:
D13: This is perltidy, v20250105
D12: This is perltidy, v20250105
D11: This is perltidy, v20250105
U24: Fails to start on my machine
U22: This is perltidy, v20250105
So.. all the KTD's now have the same modern perltidy.. which means we need these patches applied and any new code needs to by retidied on push to match the new rules if it now fails due to the new perltidy version.
I am running on Debian 12 and note that not everyone runs KTD ! perltidy -v This is perltidy, v20230309 Please help the non-KTD minority ;) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #19 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- (In reply to Marcel de Rooy from comment #18)
(In reply to Martin Renvoize (ashimema) from comment #15)
Checking KTD's, we now have:
D13: This is perltidy, v20250105
D12: This is perltidy, v20250105
D11: This is perltidy, v20250105
U24: Fails to start on my machine
U22: This is perltidy, v20250105
So.. all the KTD's now have the same modern perltidy.. which means we need these patches applied and any new code needs to by retidied on push to match the new rules if it now fails due to the new perltidy version.
I am running on Debian 12 and note that not everyone runs KTD ! perltidy -v This is perltidy, v20230309
Please help the non-KTD minority ;)
Note that D12 even has an older version. This one already came from debian.koha-community. So a rebuild should fix my container :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #20 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Just a side note but just intrigued by the first perltidy change in Auth.pm: @@ -1236,7 +1236,7 @@ sub checkauth { : Koha::Cash::Registers->search( { branch => $branchcode, branch_default => 1 }, { rows => 1 } - )->single; + )->single; I would have preferred to indent the two lines after ->search here.. Now the search parameters in the ternary else branch are on the same level as the single applied on the whole ternary result. -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #21 from Jonathan Druart <jonathan.druart@gmail.com> --- (In reply to Marcel de Rooy from comment #20)
Just a side note but just intrigued by the first perltidy change in Auth.pm: @@ -1236,7 +1236,7 @@ sub checkauth { : Koha::Cash::Registers->search( { branch => $branchcode, branch_default => 1 }, { rows => 1 } - )->single; + )->single;
I would have preferred to indent the two lines after ->search here.. Now the search parameters in the ternary else branch are on the same level as the single applied on the whole ternary result.
Martin asked the same on Mattermost: https://chat.koha-community.org/koha-community/pl/4j1pnibow7n9tdhcjddgzs1w4c The whole statement (now): 1233 my $register = 1234 $query->param('register_id') 1235 ? Koha::Cash::Registers->find( $query->param('register_id') ) 1236 : Koha::Cash::Registers->search( 1237 { branch => $branchcode, branch_default => 1 }, 1238 { rows => 1 } 1239 )->single; 1239 was definitely wrong, and it's fixed now. However I do agree that 1237 and 1238 should have an additional indent level. Maybe it will be fixed in the next update? :) -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #22 from Lucas Gass (lukeg) <lucas@bywatersolutions.com> --- Created attachment 193768 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=193768&action=edit Bug 41076: [25.05.x] Adjust perl tidiness -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Jacob O'Mara <jacob.omara@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)| |25.11.03 released in| | Status|Pushed to main |Pushed to stable -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 --- Comment #23 from Jacob O'Mara <jacob.omara@openfifth.co.uk> --- Thanks all, pushed to 25.11.x -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- Text to go in the|26.05.00 | release notes| | Version(s)|25.11.03 |26.05.00,25.11.03 released in| | CC| |dcook@prosentient.com.au --- Comment #24 from David Cook <dcook@prosentient.com.au> --- Looks like 25.05.00 accidentally went into the release notes rather than the versions released in. Don't know if this got released to 25.05.x? -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> changed: What |Removed |Added ---------------------------------------------------------------------------- Version(s)|26.05.00,25.11.03 |26.05.00,25.11.03,25.05.08 released in| | Status|Pushed to stable |Needs documenting CC| |wainuiwitikapark@catalyst.n | |et.nz --- Comment #25 from Wainui Witika-Park <wainuiwitikapark@catalyst.net.nz> --- (In reply to David Cook from comment #24)
Looks like 25.05.00 accidentally went into the release notes rather than the versions released in.
Don't know if this got released to 25.05.x?
Looks like it was released in 25.05.08 -- You are receiving this mail because: You are watching all bug changes.
https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=41076 David Nind <david@davidnind.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs documenting |RESOLVED Resolution|--- |FIXED --- Comment #26 from David Nind <david@davidnind.com> --- Developer focused, no changes to the manual required. -- You are receiving this mail because: You are watching all bug changes.
participants (1)
-
bugzilla-daemon＠bugs.koha-community.org