https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Depends on| |43546 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43546 [Bug 43546] Standardize command-line scripts on Getopt::Long::Descriptive for safer, more consistent option handling -- 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=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43558 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43558 [Bug 43558] Convert misc/cronjobs/longoverdue.pl to the Getopt::Long::Descriptive convention (see bug 43546) -- 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=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43560 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43560 [Bug 43560] Convert misc/cronjobs/update_totalissues.pl to the Getopt::Long::Descriptive convention (see bug 43546) -- 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=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Assignee|koha-bugs@lists.koha-commun |martin.renvoize@openfifth.c |ity.org |o.uk Status|NEW |Needs Signoff -- 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=43557 Frédéric Demians <f.demians@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Signed Off -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 --- Comment #2 from Frédéric Demians <f.demians@tamil.fr> --- Created attachment 207905 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=207905&action=edit Bug 43557: Add declarative mutually-exclusive option support to Koha::Script->describe_options Adds an exclusive key to the trailing options hashref of Koha::Script->describe_options, alongside the existing required and args support. Declaring exclusive => [ [qw(code exclude-code)], ... ] makes a group of options mutually exclusive: if more than one is actually given a value, the script dies with a clear, consistent error and the usage text, the same as a missing required option already does, instead of every script rolling its own pod2usage/die check. Group members must be written using each option's first-declared spec name, hyphens and all (e.g. exclude-code, not the accessor's exclude_code) -- confirmed by testing that Getopt::Long::Descriptive's _specified() introspection method needs that exact form, not the canonicalised accessor name. An option only counts as "given" if it was specified on the command line and resolved to a non-empty value, so a bare optional-value (:s) flag left over as an empty string (bug 37075) does not by itself trigger the conflict -- this was tested directly against both scalar and repeatable (:s@) option types. Getopt::Long::Descriptive's own one_of constraint was considered as an alternative, since it already provides exclusivity checking for a group of options. Testing showed it does not enforce exclusivity at all when the group's options are array/repeatable types: passing two such options together with real values is silently accepted, which would have been a regression for scripts like process_message_queue.pl (bug 43551) whose --code/--exclude-code both take multiple values. Test plan: 1. prove t/Koha/Script.t 2. koha-qa.pl passes for Koha/Script.pm Signed-off-by: Frédéric Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Frédéric Demians <f.demians@tamil.fr> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA CC| |f.demians@tamil.fr --- Comment #4 from Frédéric Demians <f.demians@tamil.fr> --- Martin, could you rebase the patches on top of bug 43546? Bug 43546 passed QA with a new follow-up, "Exit with status 1 on command-line errors". It changes Koha::Script->describe_options and adds a subtest to t/Koha/Script.t, at the same place as the subtest added here. I've seen that trying to sign-of subsequent patches: 43549, 43551, etc. Or I can rebase it myself? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 --- Comment #5 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208576 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208576&action=edit Bug 43557: Add declarative mutually-exclusive option support to Koha::Script->describe_options Adds an exclusive key to the trailing options hashref of Koha::Script->describe_options, alongside the existing required and args support. Declaring exclusive => [ [qw(code exclude-code)], ... ] makes a group of options mutually exclusive: if more than one is actually given a value, the script dies with a clear, consistent error and the usage text, the same as a missing required option already does, instead of every script rolling its own pod2usage/die check. Group members must be written using each option's first-declared spec name, hyphens and all (e.g. exclude-code, not the accessor's exclude_code) -- confirmed by testing that Getopt::Long::Descriptive's _specified() introspection method needs that exact form, not the canonicalised accessor name. An option only counts as "given" if it was specified on the command line and resolved to a non-empty value, so a bare optional-value (:s) flag left over as an empty string (bug 37075) does not by itself trigger the conflict -- this was tested directly against both scalar and repeatable (:s@) option types. Getopt::Long::Descriptive's own one_of constraint was considered as an alternative, since it already provides exclusivity checking for a group of options. Testing showed it does not enforce exclusivity at all when the group's options are array/repeatable types: passing two such options together with real values is silently accepted, which would have been a regression for scripts like process_message_queue.pl (bug 43551) whose --code/--exclude-code both take multiple values. Test plan: 1. prove t/Koha/Script.t 2. koha-qa.pl passes for Koha/Script.pm Signed-off-by: Frédéric Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |43553 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43553 [Bug 43553] Convert misc/cronjobs/holds/holds_reminder.pl to the Getopt::Long::Descriptive convention (see bug 43546) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 --- Comment #7 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208589 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208589&action=edit Bug 43557: Add declarative mutually-exclusive option support to Koha::Script->describe_options Adds an exclusive key to the trailing options hashref of Koha::Script->describe_options, alongside the existing required and args support. Declaring exclusive => [ [qw(code exclude-code)], ... ] makes a group of options mutually exclusive: if more than one is actually given a value, the script dies with a clear, consistent error and the usage text, the same as a missing required option already does, instead of every script rolling its own pod2usage/die check. Group members must be written using each option's first-declared spec name, hyphens and all (e.g. exclude-code, not the accessor's exclude_code) -- confirmed by testing that Getopt::Long::Descriptive's _specified() introspection method needs that exact form, not the canonicalised accessor name. An option only counts as "given" if it was specified on the command line and resolved to a non-empty value, so a bare optional-value (:s) flag left over as an empty string (bug 37075) does not by itself trigger the conflict -- this was tested directly against both scalar and repeatable (:s@) option types. Getopt::Long::Descriptive's own one_of constraint was considered as an alternative, since it already provides exclusivity checking for a group of options. Testing showed it does not enforce exclusivity at all when the group's options are array/repeatable types: passing two such options together with real values is silently accepted, which would have been a regression for scripts like process_message_queue.pl (bug 43551) whose --code/--exclude-code both take multiple values. Test plan: 1. prove t/Koha/Script.t 2. koha-qa.pl passes for Koha/Script.pm Signed-off-by: Frédéric Demians <f.demians@tamil.fr> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 --- Comment #9 from Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> --- Created attachment 208591 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208591&action=edit Bug 43557: (follow-up) Accept legacy single-dash long options with a deprecation warning Scripts converted to Koha::Script->describe_options used to be parsed by plain Getopt::Long, which also accepted a long option with a single dash (-days 5). Several of them documented that form (holds_reminder.pl lists -days, -lettercode, -date, -library and -holidays), so existing cron lines use it. Getopt::Long::Descriptive parses with bundling, and -days is then read as the flags -d -a -y -s: the script prints its usage and exits 1, and the cron job silently stops running. Rewrite such an argument as the standard --days, and print a deprecation warning to STDERR (cron mails it to the administrator) so existing command lines keep working while they are migrated. An argument is only rewritten if it is a single dash and at least two characters, and names a declared long option or the unique prefix of one (as plain Getopt::Long allowed), and it is not a bundle of declared one-character flags (-vc is still -v -c). The values given to options, and anything after --, are left alone. -help and -man work too. This is a compatibility shim to be removed in a future release; --long remains the documented form. Test plan: 1. Apply the patch 2. prove t/Koha/Script.t 3. Run a converted script with a legacy option, for example misc/cronjobs/holds/holds_reminder.pl -days 3 (with the patch of bug 43553): it runs, and prints WARNING: -days is deprecated, use --days instead (...) 4. Run it with --days 3: it runs and prints no warning Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 Andrew Fuerste-Henry <andrew@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Passed QA QA Contact|testopia@bugs.koha-communit |andrew@bywatersolutions.com |y.org | -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 --- Comment #11 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 208605 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208605&action=edit Bug 43557: Add declarative mutually-exclusive option support to Koha::Script->describe_options Adds an exclusive key to the trailing options hashref of Koha::Script->describe_options, alongside the existing required and args support. Declaring exclusive => [ [qw(code exclude-code)], ... ] makes a group of options mutually exclusive: if more than one is actually given a value, the script dies with a clear, consistent error and the usage text, the same as a missing required option already does, instead of every script rolling its own pod2usage/die check. Group members must be written using each option's first-declared spec name, hyphens and all (e.g. exclude-code, not the accessor's exclude_code) -- confirmed by testing that Getopt::Long::Descriptive's _specified() introspection method needs that exact form, not the canonicalised accessor name. An option only counts as "given" if it was specified on the command line and resolved to a non-empty value, so a bare optional-value (:s) flag left over as an empty string (bug 37075) does not by itself trigger the conflict -- this was tested directly against both scalar and repeatable (:s@) option types. Getopt::Long::Descriptive's own one_of constraint was considered as an alternative, since it already provides exclusivity checking for a group of options. Testing showed it does not enforce exclusivity at all when the group's options are array/repeatable types: passing two such options together with real values is silently accepted, which would have been a regression for scripts like process_message_queue.pl (bug 43551) whose --code/--exclude-code both take multiple values. Test plan: 1. prove t/Koha/Script.t 2. koha-qa.pl passes for Koha/Script.pm Signed-off-by: Frédéric Demians <f.demians@tamil.fr> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43557 --- Comment #13 from Andrew Fuerste-Henry <andrew@bywatersolutions.com> --- Created attachment 208607 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=208607&action=edit Bug 43557: (follow-up) Accept legacy single-dash long options with a deprecation warning Scripts converted to Koha::Script->describe_options used to be parsed by plain Getopt::Long, which also accepted a long option with a single dash (-days 5). Several of them documented that form (holds_reminder.pl lists -days, -lettercode, -date, -library and -holidays), so existing cron lines use it. Getopt::Long::Descriptive parses with bundling, and -days is then read as the flags -d -a -y -s: the script prints its usage and exits 1, and the cron job silently stops running. Rewrite such an argument as the standard --days, and print a deprecation warning to STDERR (cron mails it to the administrator) so existing command lines keep working while they are migrated. An argument is only rewritten if it is a single dash and at least two characters, and names a declared long option or the unique prefix of one (as plain Getopt::Long allowed), and it is not a bundle of declared one-character flags (-vc is still -v -c). The values given to options, and anything after --, are left alone. -help and -man work too. This is a compatibility shim to be removed in a future release; --long remains the documented form. Test plan: 1. Apply the patch 2. prove t/Koha/Script.t 3. Run a converted script with a legacy option, for example misc/cronjobs/holds/holds_reminder.pl -days 3 (with the patch of bug 43553): it runs, and prints WARNING: -days is deprecated, use --days instead (...) 4. Run it with --days 3: it runs and prints no warning Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com> Signed-off-by: Andrew Fuerste Henry <andrew@bywatersolutions.com> -- You are receiving this mail because: You are watching all bug changes.