Bug 25549 - Broken plugins should not break Koha (Install plugin script/method should highlight broken plugins)
Summary: Broken plugins should not break Koha (Install plugin script/method should hig...
Status: CLOSED FIXED
Alias: None
Product: Koha
Classification: Unclassified
Component: Plugin architecture (show other bugs)
Version: Main
Hardware: All All
: P5 - low major (vote)
Assignee: Nick Clemens
QA Contact: Testopia
URL:
Keywords:
Depends on: 23191
Blocks:
  Show dependency treegraph
 
Reported: 2020-05-20 11:12 UTC by Nick Clemens
Modified: 2021-06-14 21:29 UTC (History)
8 users (show)

See Also:
Change sponsored?: ---
Patch complexity: Small patch
Documentation contact:
Documentation submission:
Text to go in the release notes:
Version(s) released in:
20.11.00, 20.05.06, 19.11.12


Attachments
Bug 25549: Remove plugin methods for broken plugins (3.01 KB, patch)
2020-05-20 11:18 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25549: Remove plugin methods for broken plugins (3.07 KB, patch)
2020-09-29 09:22 UTC, Fridolin Somers
Details | Diff | Splinter Review
Bug 25549: Remove plugin methods for broken plugins (3.01 KB, patch)
2020-10-07 11:15 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25549: Remove plugin methods for broken plugins (4.62 KB, patch)
2020-10-07 14:05 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25549: Remove plugin methods for broken plugins (4.68 KB, patch)
2020-10-07 15:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25549: Add error reporting to plugins-home (4.50 KB, patch)
2020-10-07 15:58 UTC, Martin Renvoize
Details | Diff | Splinter Review
Bug 25549: Remove plugin methods for broken plugins (4.74 KB, patch)
2020-10-08 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25549: Add error reporting to plugins-home (4.58 KB, patch)
2020-10-08 13:12 UTC, Nick Clemens
Details | Diff | Splinter Review
Bug 25549: Remove plugin methods for broken plugins (4.80 KB, patch)
2020-10-15 23:13 UTC, Katrin Fischer
Details | Diff | Splinter Review
Bug 25549: Add error reporting to plugins-home (4.63 KB, patch)
2020-10-15 23:13 UTC, Katrin Fischer
Details | Diff | Splinter Review

Note You need to log in before you can comment on or make changes to this bug.
Description Nick Clemens 2020-05-20 11:12:01 UTC
bug 23191 added a script to allow installation of plugins from the command line.

There can be an issue when a plugin no longer compiles after an upgrade, Koha will not load, the script will not run either.

We should repair the script so that it can remove plugin methods that no longer function or have been deleted.
Comment 1 Nick Clemens 2020-05-20 11:18:11 UTC
Created attachment 105143 [details] [review]
Bug 25549: Remove plugin methods for broken plugins

To test:
 1 - Enable plugins in the koha-conf
 2 - Install the kitchen sink plugin
 3 - Your staff client should be orange now :-)
 4 - edit the plugin module
     /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
 5 - Add a line to break compilation, like:
     this won't compile
 6 - Restart all
 7 - Your koha is now broken
 8 - kshell
 9 - perl misc/devel/install_plugins.pl
10 - Restart all
11 - Koha remains broken
12 - Apply patch
13 - kshell
14 - perl misc/devel/install_plugins.pl
15 - Koha now works!
16 - Koha is not orange because the plugin methods are removed
Comment 2 Fridolin Somers 2020-09-29 09:22:06 UTC
Whaoo super, I had this issue when playing with plugins install.
All perl is broken if a plugins is broken or its source removed.
Comment 3 Fridolin Somers 2020-09-29 09:22:39 UTC
Created attachment 110896 [details] [review]
Bug 25549: Remove plugin methods for broken plugins

To test:
 1 - Enable plugins in the koha-conf
 2 - Install the kitchen sink plugin
 3 - Your staff client should be orange now :-)
 4 - edit the plugin module
     /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
 5 - Add a line to break compilation, like:
     this won't compile
 6 - Restart all
 7 - Your koha is now broken
 8 - kshell
 9 - perl misc/devel/install_plugins.pl
10 - Restart all
11 - Koha remains broken
12 - Apply patch
13 - kshell
14 - perl misc/devel/install_plugins.pl
15 - Koha now works!
16 - Koha is not orange because the plugin methods are removed

Signed-off-by: Fridolin Somers <fridolin.somers@biblibre.com>
Comment 4 Martin Renvoize 2020-10-05 14:42:14 UTC
Whilst this is great and resolves the bug mentioned (I've hit this too), it breaks the output of the script as the 'existing_plugins' array used to contain the 'plugins before' call to InstallPlugins but now runs after the call to InstallPlugins and so will not be a comparison.
Comment 5 Martin Renvoize 2020-10-05 14:53:02 UTC
I think we need to move the call to GetPlugins in install_plugins.pl back up to where it we and add the option to run GetPlugins with 'can_load' in this case.

It would be nice to report the deletions too.
Comment 6 Nick Clemens 2020-10-07 11:15:29 UTC
Created attachment 111324 [details] [review]
Bug 25549: Remove plugin methods for broken plugins

To test:
 1 - Enable plugins in the koha-conf
 2 - Install the kitchen sink plugin
 3 - Your staff client should be orange now :-)
 4 - edit the plugin module
     /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
 5 - Add a line to break compilation, like:
     this won't compile
 6 - Restart all
 7 - Your koha is now broken
 8 - kshell
 9 - perl misc/devel/install_plugins.pl
10 - Restart all
11 - Koha remains broken
12 - Apply patch
13 - kshell
14 - perl misc/devel/install_plugins.pl
15 - Koha now works!
16 - Koha is not orange because the plugin methods are removed
Comment 7 Nick Clemens 2020-10-07 14:05:12 UTC
Created attachment 111335 [details] [review]
Bug 25549: Remove plugin methods for broken plugins

To test:
 1 - Enable plugins in the koha-conf
 2 - Install the kitchen sink plugin
 3 - Your staff client should be orange now :-)
 4 - edit the plugin module
     /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
 5 - Add a line to break compilation, like:
     this won't compile
 6 - Restart all
 7 - Your koha is now broken
 8 - kshell
 9 - perl misc/devel/install_plugins.pl
10 - Restart all
11 - Koha remains broken
12 - Apply patch
13 - kshell
14 - perl misc/devel/install_plugins.pl
15 - Koha now works!
16 - Koha is not orange because the plugin methods are removed
Comment 8 Martin Renvoize 2020-10-07 15:58:15 UTC
Created attachment 111345 [details] [review]
Bug 25549: Remove plugin methods for broken plugins

To test:
 1 - Enable plugins in the koha-conf
 2 - Install the kitchen sink plugin
 3 - Your staff client should be orange now :-)
 4 - edit the plugin module
     /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
 5 - Add a line to break compilation, like:
     this won't compile
 6 - Restart all
 7 - Your koha is now broken
 8 - kshell
 9 - perl misc/devel/install_plugins.pl
10 - Restart all
11 - Koha remains broken
12 - Apply patch
13 - kshell
14 - perl misc/devel/install_plugins.pl
15 - Koha now works!
16 - Koha is not orange because the plugin methods are removed

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 9 Martin Renvoize 2020-10-07 15:58:21 UTC
Created attachment 111346 [details] [review]
Bug 25549: Add error reporting to plugins-home

This patch adds handling for the newly introduced 'errors' option in
GetPlugins and reports erroroneos plugins to the user.

To test:
1 - Enable plugins in the koha-conf
2 - Install the kitchen sink plugin
3 - Your staff client should be orange now :-)
4 - The plugin should display as installed in the table at
    tools/plugins.
5 - edit the plugin module
    /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
6 - Add a line to break compilation, like:
    this won't compile
7 - Restart all
8 - Your Koha is not broken, but is no longer orange (The plugin is not
    loading).
9 - The plugin should display as unable to load in the table at
    tools/plugins.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>
Comment 10 Martin Renvoize 2020-10-07 16:00:39 UTC
This works really well.. the switch to using can_load in the GetPlugins call has the added benefit that the initial 'broken koha' issue goes away entirely.. just the plugin functionality won't load.

I've added a followup that restores the InstallPlugins call such that erroneoes plugins are no longer removed by calling it (as they no longer cause an issue) and now use such occurrences along with the errors flag on GetPlugins to highlight broken plugins from the plugin management page.
Comment 11 Nick Clemens 2020-10-08 13:12:09 UTC
Created attachment 111375 [details] [review]
Bug 25549: Remove plugin methods for broken plugins

To test:
 1 - Enable plugins in the koha-conf
 2 - Install the kitchen sink plugin
 3 - Your staff client should be orange now :-)
 4 - edit the plugin module
     /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
 5 - Add a line to break compilation, like:
     this won't compile
 6 - Restart all
 7 - Your koha is now broken
 8 - kshell
 9 - perl misc/devel/install_plugins.pl
10 - Restart all
11 - Koha remains broken
12 - Apply patch
13 - kshell
14 - perl misc/devel/install_plugins.pl
15 - Koha now works!
16 - Koha is not orange because the plugin methods are removed

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 12 Nick Clemens 2020-10-08 13:12:16 UTC
Created attachment 111376 [details] [review]
Bug 25549: Add error reporting to plugins-home

This patch adds handling for the newly introduced 'errors' option in
GetPlugins and reports erroroneos plugins to the user.

To test:
1 - Enable plugins in the koha-conf
2 - Install the kitchen sink plugin
3 - Your staff client should be orange now :-)
4 - The plugin should display as installed in the table at
    tools/plugins.
5 - edit the plugin module
    /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
6 - Add a line to break compilation, like:
    this won't compile
7 - Restart all
8 - Your Koha is not broken, but is no longer orange (The plugin is not
    loading).
9 - The plugin should display as unable to load in the table at
    tools/plugins.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>
Comment 13 Katrin Fischer 2020-10-15 23:13:08 UTC
Created attachment 111816 [details] [review]
Bug 25549: Remove plugin methods for broken plugins

To test:
 1 - Enable plugins in the koha-conf
 2 - Install the kitchen sink plugin
 3 - Your staff client should be orange now :-)
 4 - edit the plugin module
     /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
 5 - Add a line to break compilation, like:
     this won't compile
 6 - Restart all
 7 - Your koha is now broken
 8 - kshell
 9 - perl misc/devel/install_plugins.pl
10 - Restart all
11 - Koha remains broken
12 - Apply patch
13 - kshell
14 - perl misc/devel/install_plugins.pl
15 - Koha now works!
16 - Koha is not orange because the plugin methods are removed

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 14 Katrin Fischer 2020-10-15 23:13:16 UTC
Created attachment 111817 [details] [review]
Bug 25549: Add error reporting to plugins-home

This patch adds handling for the newly introduced 'errors' option in
GetPlugins and reports erroroneos plugins to the user.

To test:
1 - Enable plugins in the koha-conf
2 - Install the kitchen sink plugin
3 - Your staff client should be orange now :-)
4 - The plugin should display as installed in the table at
    tools/plugins.
5 - edit the plugin module
    /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm
6 - Add a line to break compilation, like:
    this won't compile
7 - Restart all
8 - Your Koha is not broken, but is no longer orange (The plugin is not
    loading).
9 - The plugin should display as unable to load in the table at
    tools/plugins.

Signed-off-by: Martin Renvoize <martin.renvoize@ptfs-europe.com>

Signed-off-by: Nick Clemens <nick@bywatersolutions.com>

Signed-off-by: Katrin Fischer <katrin.fischer.83@web.de>
Comment 15 Katrin Fischer 2020-10-15 23:13:49 UTC
Should the change in Plugins.pm be covered by a regression test?

I think it would be nice if we offered the uninstall option for broken plugins.  Otherwise you are stuck with them forever until you can get someone on your server to remove them.
Comment 16 Martin Renvoize 2020-10-16 05:38:30 UTC
(In reply to Katrin Fischer from comment #15)
> Should the change in Plugins.pm be covered by a regression test?
> 

Probably, though I'm not entirely sure how to write one off the top of my head.

> I think it would be nice if we offered the uninstall option for broken
> plugins.  Otherwise you are stuck with them forever until you can get
> someone on your server to remove them.

This is more difficult: The uninstall button actually calls an uninstall method from within the plugins themselves to allow for some cleanup that's plugin specific. If we wanted to expose something for broken plugins we would need to write a specific method for it and it would be more of a 'Partial remove' than an uninstall.. we would likely be left with data in the database as there's no easy way we could tell where the plugin put it's data.  I think leave that one for another bug 😉
Comment 17 Katrin Fischer 2020-10-16 10:00:57 UTC
> > I think it would be nice if we offered the uninstall option for broken
> > plugins.  Otherwise you are stuck with them forever until you can get
> > someone on your server to remove them.
> 
> This is more difficult: The uninstall button actually calls an uninstall
> method from within the plugins themselves to allow for some cleanup that's
> plugin specific. If we wanted to expose something for broken plugins we
> would need to write a specific method for it and it would be more of a
> 'Partial remove' than an uninstall.. we would likely be left with data in
> the database as there's no easy way we could tell where the plugin put it's
> data.  I think leave that one for another bug 😉

It was meant to be - this one is passed :)
Comment 18 Jonathan Druart 2020-10-22 08:25:05 UTC
Pushed to master for 20.11, thanks to everybody involved!
Comment 19 Lucas Gass 2020-11-13 17:37:26 UTC
backported to 20.05.x for 20.05.06
Comment 20 Aleisha Amohia 2020-11-17 04:14:15 UTC
backported to 19.11.x for 19.11.12
Comment 21 Victor Grousset/tuxayo 2020-11-17 10:30:49 UTC
Missing dependencies for 19.05.x, can't backport.