https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=43546
Bug ID: 43546
Summary: Standardize command-line scripts on
Getopt::Long::Descriptive for safer, more consistent
option handling
Initiative type: ---
Sponsorship ---
status:
Product: Koha
Version: Main
Hardware: All
OS: All
Status: NEW
Severity: enhancement
Priority: P5 - low
Component: Command-line Utilities
Assignee: [email protected]
Reporter: [email protected]
QA Contact: [email protected]
CC: [email protected], [email protected]
Target Milestone: ---
Koha's command-line scripts (misc/cronjobs/*.pl and more broadly the roughly
150 scripts under misc/) use plain Getopt::Long, which has a structural gap:
its = and : spec modifiers only mean "this flag needs a value if it's used" --
there is no way to express "this flag itself is mandatory." Scripts that need a
genuinely required option have to bolt on their own ad hoc check after parsing,
for example:
GetOptions( 'h|hours=s' => \$hours );
unless ($hours) {
print $usage;
die "ERROR: No --hours (-h) option defined";
}
A survey of misc/cronjobs/ (58 scripts, 43 using GetOptions) found 14 scripts
doing this kind of manual required-option check, each with its own bespoke
error text, none of it declared anywhere Getopt::Long itself understands. This
is a reliability and maintainability gap:
Reliability -- a required option silently omitted is only caught by whatever ad
hoc check (if any) the script's author remembered to write, which is
inconsistent across scripts, easy to miss when writing a new one, and the
failure mode for missing checks is scripts running with undef or empty values
instead of failing fast. That is particularly risky for unattended cron
execution where nobody is watching stdout.
Security/robustness -- consistent, declarative validation of required input
reduces the chance of an unset or malformed option flowing further into a
script (file paths, SQL parameters, shell arguments) before anything checks it.
This is defense-in-depth rather than a specific known vulnerability.
Maintainability/readability -- option name, description, type, and
required-ness end up scattered across the GetOptions call, a separate POD
block, and a separate manual check, all of which can drift out of sync. A
declarative spec keeps them in one place.
Modernization -- this is a small, low-risk step toward more structured,
self-documenting CLI code, consistent with Koha's general direction of reducing
boilerplate and inconsistency across similar scripts.
Proposal:
Adopt Getopt::Long::Descriptive (pure Perl, no XS, actively maintained, built
directly on top of Getopt::Long -- see
https://metacpan.org/pod/Getopt::Long::Descriptive) as the standard for
new/converted scripts, using its native "required" constraint, for example:
my ($opt, $usage) = describe_options(
'%c %o',
[ 'hours|h=i', "hours since item was returned", { required => 1 } ],
...
);
This makes "mandatory option" an explicit, declarative property of the script
rather than something reconstructed from Getopt::Long spec syntax or scattered
ad hoc checks, and it auto-generates consistent --help/usage text from the same
declaration, removing the separately-maintained pod2usage boilerplate found in
roughly 32 of the surveyed scripts.
Suggested scope (staged, not a single patch):
1. Add Getopt::Long::Descriptive to cpanfile.
2. Document the convention for how new/converted scripts should declare and
parse options -- possibly formalized as a small helper alongside Koha::Script,
which currently only handles locking/logging/userenv and has no opinion on
option parsing.
3. Convert a small number of scripts as a proof of concept, chosen for having a
genuinely mandatory option today (e.g. cart_to_shelf.pl's -h/--hours).
4. Convert the remainder opportunistically over time, not as a single mass
patch -- this changes each script's help/error output, which is a real (if
minor) behavior change worth reviewing script by script.
One real-world motivating example: the third-party koha-plugin-crontab plugin
(still a work in progress, not recommended for production use yet --
https://github.com/openfifth/koha-plugin-crontab) lets staff schedule these
scripts from a UI and tries to surface which options are required in its form.
Lacking any core-level signal, it has to either mis-infer this from
Getopt::Long spec syntax or fall back to an administrator manually curating the
list per install -- a workaround, not a fix. That is a symptom of the
underlying gap this bug is about, not the primary motivation for it.
Test plan:
- For each converted script, confirm --help output is sensible and existing
documented options still work.
- Confirm omitting a required option now fails with a clear, consistent error
before any side effects occur.
- Confirm koha-qa.pl passes for converted scripts.
--
You are receiving this mail because:
You are the assignee for the bug.
You are watching all bug changes.
_______________________________________________
Koha-bugs mailing list -- [email protected]
To unsubscribe send an email to [email protected]
website : http://www.koha-community.org/
git : http://git.koha-community.org/
bugs : http://bugs.koha-community.org/
