From fcb038b9d80ef049b8d0c75d1fcce552b3cc0f0d Mon Sep 17 00:00:00 2001 From: Martin Renvoize Date: Tue, 27 Jan 2026 07:51:31 +0000 Subject: [PATCH] Bug 41651: (follow-up) Improve plugin dependency handling The original patch was a great start, but I saw some area's for improvement including listing all missing dependencies as apposed to just the first. - Add POD documentation for the new 'from' parameter in InstallPlugins() - Normalize staging directory path by removing trailing slashes to prevent double-slash issues in destination paths - Fix path concatenation to avoid double slashes (plugins_dir already ends without slash, destdir starts with slash after substitution) - Report all unmet dependencies at once instead of dying on the first one, providing better user feedback - Add 'last' to break out of metafiles loop once a file is found --- Koha/Plugins.pm | 21 ++++++++++++++++----- 1 file changed, 16 insertions(+), 5 deletions(-) diff --git a/Koha/Plugins.pm b/Koha/Plugins.pm index 4e137c4e66d..df8a656083d 100644 --- a/Koha/Plugins.pm +++ b/Koha/Plugins.pm @@ -295,6 +295,12 @@ Parameters: =item B: A list of class names to exclude from the process. +=item B: A staging directory path containing unpacked plugin files. +When provided, the method will first check plugin dependencies (from cpanfile +or META.json/META.yml) and only copy the files to the plugins directory if +all runtime requirements are met. Dies if dependencies are unmet or file +copying fails. + =item B: A list of class names to limit the process to. =item B: Print useful information. @@ -314,7 +320,10 @@ sub InstallPlugins { # We have "staged" plugin(s) passed in; check the dependencies, # copy to the regular location if we're happy - $self->_check_dependencies( $params->{from} ); + my $staging_dir = $params->{from}; + $staging_dir =~ s{/+$}{}; # Remove trailing slashes for clean path handling + + $self->_check_dependencies($staging_dir); my $plugins_dir = C4::Context->config("pluginsdir"); $plugins_dir = ref($plugins_dir) eq 'ARRAY' ? $plugins_dir->[0] : $plugins_dir; @@ -326,15 +335,15 @@ sub InstallPlugins { return unless -f $_; my ($basename) = fileparse($_); return if $basename =~ /^(cpanfile|META\.(json|yml))$/; - my $destdir = $File::Find::dir =~ s/^\Q$params->{from}\E//r; - $destdir = "$plugins_dir/$destdir"; + my $destdir = $File::Find::dir =~ s/^\Q$staging_dir\E//r; + $destdir = "$plugins_dir$destdir"; make_path($destdir); unless ( copy( $File::Find::name, $destdir ) ) { push @copy_errors, "Failed to copy $File::Find::name to $destdir: $!"; } }, }, - $params->{from} + $staging_dir ); die join( "\n", @copy_errors ) . "\n" if @copy_errors; } @@ -513,6 +522,7 @@ sub _check_dependencies { for (@metafiles) { if ( -f "$path/$_" ) { $prereqs = CPAN::Meta->load_file("$path/$_"); + last; } } } @@ -520,7 +530,8 @@ sub _check_dependencies { return unless $prereqs; # Good for both file variants: per the docs, "$meta should be a CPAN::Meta::Prereqs or CPAN::Meta object" - die "$_\n" for verify_dependencies( $prereqs, 'runtime', 'requires' ); + my @unmet = verify_dependencies( $prereqs, 'runtime', 'requires' ); + die "Unmet dependencies:\n" . join( "\n", map { " - $_" } @unmet ) . "\n" if @unmet; } 1; -- 2.52.0