https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Julian Maurice <julian.maurice@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |julian.maurice@biblibre.com --- Comment #1 from Julian Maurice <julian.maurice@biblibre.com> --- +1 Having to load the class is also a potential security breach, as it allows disabled plugins to execute code. -- 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=24631 --- Comment #2 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103092 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103092&action=edit Bug 24631: Plugin metadata should be outside the main class Currently, plugin metadata is stored within the main plugin class file but this means one has to load the class to read it. We should pull this data out into a manifest/metadata file in a standard format (JSON/YAML) to allow simpler external parsing. Test Plan: 1) Apply this patch 2) Install the latest Kitchen Sink plugin: https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases/downlo... 3) Browse to plugins home, note that nothing appears to have changed 4) Browse to your pluginsdir, find the META.yml file for the Kitchen Sink plugin, edit some of the metadata 5) Reload the plugins home page, note your changes to META.yml appear in the plugins table! -- 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=24631 David Cook <dcook@prosentient.com.au> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |dcook@prosentient.com.au --- Comment #3 from David Cook <dcook@prosentient.com.au> --- +1 Just getting started for the morning, but will put this to the side to look at later. Thinking about doing more plugin work soon... -- 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=24631 --- Comment #4 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- Thanks for jumping onto this one Kyle :) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #5 from Julian Maurice <julian.maurice@biblibre.com> --- Hi Kyle, Can you comment on why you chose to name the file META.yml ? Is it related to https://metacpan.org/pod/CPAN::Meta::Spec ? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |victor@tuxayo.net --- Comment #7 from Victor Grousset/tuxayo <victor@tuxayo.net> ---

3) Browse to plugins home, note that nothing appears to have changed

Except my screen turned orange :o (first time installing a plugin, and kitchen sink, that was a surprise ^^") More seriously: Is that actually expected to have the background changed to orange when the plugin is disabled?

5) Reload the plugins home page, note your changes to META.yml appear in the plugins table!

I don't see changes. Is the correct file to edit this one? /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink/META.yml I'm using koha-testing-docker, I even ran restart_all what could be missing? (git log from the container shows that HEAD is bug 24631) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #9 from David Cook <dcook@prosentient.com.au> --- (In reply to Julian Maurice from comment #8)

(In reply to Kyle M Hall from comment #6)

...

(In reply to Julian Maurice from comment #5)

...

Hi Kyle,

Can you comment on why you chose to name the file META.yml ? Is it related to https://metacpan.org/pod/CPAN::Meta::Spec ?

You are essentially correct. The file name is inspired by CPAN's META.yml/META.json file.

Won't that be a problem if people want to publish their plugins on CPAN ?

I was wary of using META.yml for a name due to its CPAN significance, but publishing plugins on CPAN seems problematic. It seems to me the plugins are their own packages and that the metadata file is part of that packaging scheme, so I don't really see the overlap. Like you could publish the module for a Koha plugin on CPAN, but you'd have to download it and package it into a KPZ and at that point you could provide the META.yml file. That said, maybe META.yml is just too close for comfort. Maybe KMETA.yml or KPMETA.yml? Just to make it a bit more distinctive? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #11 from Martin Renvoize <martin.renvoize@ptfs-europe.com> --- I'm happy with whatever name.. META.yml probably came out of a discussion Kyle and I had about using cpan's packaging system for inspiration.. meta.yml, plugin.yml. koha.yml.. I'm not too worried about a name so long as we document it. As for requiring module load prior to reading the yml.. hmm, does feel like we should be able to get that path info from the plugin base code koha side somehow.. A chunk of this is actually about how we then expose the new meta file next to releases on github (and eventually gitlab) so we can grab that data via api's and display/use it prior to install entirely. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #13 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #11)

I'm happy with whatever name.. META.yml probably came out of a discussion Kyle and I had about using cpan's packaging system for inspiration.. meta.yml, plugin.yml. koha.yml.. I'm not too worried about a name so long as we document it.

As for requiring module load prior to reading the yml.. hmm, does feel like we should be able to get that path info from the plugin base code koha side somehow.. A chunk of this is actually about how we then expose the new meta file next to releases on github (and eventually gitlab) so we can grab that data via api's and display/use it prior to install entirely.

I'm doing my best to keep Koha backwards compatible with plugins that have no META.yml file. I definitely think we should consider ignoring plugin releases without a META.yml file for searching purposes. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #15 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Martin Renvoize from comment #11)

I'm happy with whatever name.. META.yml probably came out of a discussion Kyle and I had about using cpan's packaging system for inspiration.. meta.yml, plugin.yml. koha.yml.. I'm not too worried about a name so long as we document it.

As for requiring module load prior to reading the yml.. hmm, does feel like we should be able to get that path info from the plugin base code koha side somehow.. A chunk of this is actually about how we then expose the new meta file next to releases on github (and eventually gitlab) so we can grab that data via api's and display/use it prior to install entirely.

I will submit a followup to rename the file to PLUGIN.yml. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #17 from David Cook <dcook@prosentient.com.au> --- (In reply to Martin Renvoize from comment #11)

As for requiring module load prior to reading the yml.. hmm, does feel like we should be able to get that path info from the plugin base code koha side somehow.. A chunk of this is actually about how we then expose the new meta file next to releases on github (and eventually gitlab) so we can grab that data via api's and display/use it prior to install entirely.

The proposed process sounds like how Apt works (ie download and use the metadata before you even see the packaged software), but you're trying to shoehorn it into Github/Gitlab infrastructure. Looking at https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases, I see the koha-plugin-kitchen-sink-v2.1.39.kpz and PLUGIN.yml sitting next to each other. That means you could download and verify the metadata without the software, but that would make importing plugins a 2 step process, if you had to import the KPZ and YAML both. It would also be possible to load in the wrong YAML file for a KPZ. In terms of importing, it would be easier if the PLUGIN.yml were located within the KPZ file. However, that would require staging and unpacking the file to get the PLUGIN.yml metadata. Not very API friendly and also a bit fiddly at the server level. Ok I'm having a thought... If PLUGIN.yml had a MD5 hash for the KPZ file, that would bind together a PLUGIN.yml file and a KPZ file. (Note that you could cryptographically sign the PLUGIN.yml file to increase security.) To import a plugin, you'd first import the PLUGIN.yml, and then you'd import the KPZ file (this could be manual or automated). For backwards compatibility, you could still import a KPZ file without a PLUGIN.yml. Maybe we could add a system preference for this and slowly deprecate this functionality. For the Upload UI, you could noticeably prompt for the PLUGIN.yml, but have a small visible link to "skip" the metadata verification process. For the Plugin Management UI, maybe it would have to generate plugin metadata in the absence of the file. It would be less secure, but that's the world that we're living in at the moment, and we could deprecate this insecure functionality over time. Alternatively, maybe make Koha so that all new plugins for a system have to have PLUGIN.yml in order to import, but all existing plugins have metadata auto-generated for them (after all, they're already installed in the system). And maybe have a system preference to turn off the PLUGIN.yml requirement if necessary to import old plugins that don't have PLUGIN.yml? That would default to the safer option while allowing backwards compatibility optionally. How does that sound? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #19 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Retested on latest patches.

3) Browse to plugins home, note that nothing appears to have changed

Except my screen turned orange :o Is that actually expected to have the background changed to orange when the plugin is disabled?

5) Reload the plugins home page, note your changes to META.yml appear in the plugins table!

I don't see changes. When editing this file /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink/ META.yml But I see changes when editing this file /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm For some reason it's still META.yml . The container is fresh, patches where applied before starting it. KitchenSink was redownloaded now. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #21 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Victor Grousset/tuxayo from comment #19)

Retested on latest patches.

...

3) Browse to plugins home, note that nothing appears to have changed

Except my screen turned orange :o

Is that actually expected to have the background changed to orange when the plugin is disabled?

...

5) Reload the plugins home page, note your changes to META.yml appear in the plugins table!

I don't see changes. When editing this file /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink/ META.yml

But I see changes when editing this file /var/lib/koha/kohadev/plugins/Koha/Plugin/Com/ByWaterSolutions/KitchenSink.pm

For some reason it's still META.yml . The container is fresh, patches where applied before starting it. KitchenSink was redownloaded now.

If you aren't seeing the changes in META.yml, but *are* seeing changes to the metadata in KitchenSink.pm, that means Koha is failing to find the META.yml and is falling back to the original behavior. I'll double check my code and retest. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |m.de.rooy@rijksmuseum.nl --- Comment #24 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Why not do something like this in a plugin with an our $metadata definition: sub new { my ($class, $params) = @_; return $class->SUPER::new($params, $metadata); } Or when a new style plugin has a manifest file, do: sub new { my ($class, $params) = @_; return $class->SUPER::new($params, IO::File->new(FILENAME) ); } And let Plugin::Base::new handle either the hashref or the filehandle? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #26 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- Perhaps a bit out of scope, but could we have a Koha/Plugin/Example class that contains the absolute minimum code for a plugin to be usable in our codebase, controlled by the regular QA process ? I know KitchenSink but it contains a lot of examples and is not in the codebase. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103656|0 |1 is obsolete| | --- Comment #28 from Victor Grousset/tuxayo <victor@tuxayo.net> --- Created attachment 103722 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103722&action=edit Bug 24631: Plugin metadata should be outside the main class Currently, plugin metadata is stored within the main plugin class file but this means one has to load the class to read it. We should pull this data out into a manifest/metadata file in a standard format (JSON/YAML) to allow simpler external parsing. Test Plan: 1) Apply this patch 2) Install the latest Kitchen Sink plugin: https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases/downlo... 3) Browse to plugins home, note that nothing appears to have changed 4) Browse to your pluginsdir, find the PLUGIN.yml file for the Kitchen Sink plugin, edit some of the metadata 5) Reload the plugins home page, note your changes to PLUGIN.yml appear in the plugins table! Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=25285 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #30 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Victor Grousset/tuxayo from comment #29)

Works :D

I might have found an unrelated bug. When putting a minimum version of Koha superior to the current one. The warning text is wrong.

...

Warning: This report was written for a newer version of Koha. Run at your own risk.

Good catch! Can you file a bug report for that? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Marcel de Rooy <m.de.rooy@rijksmuseum.nl> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Signed Off |Failed QA --- Comment #32 from Marcel de Rooy <m.de.rooy@rijksmuseum.nl> --- At least comment24 and comment25 need feedback (although I did not add the others for ignoring them ;) Actually, duplicating the arguable code in GetPlugins to GetPluginsMetaData is no reason imo for automatically going thru QA. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #34 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Marcel de Rooy from comment #25)

This part of GetPlugins is rather debatable: load $plugin_class; my $plugin = $plugin_class->new({ enable_plugins => $self->{'enable_plugins'} # loads even if plugins are disabled # FIXME: is this for testing without bothering to mock config? }); We should not execute it at all imo if we did not set enable_plugins. And the override flag enable_plugins here could well be a candidate for security problems. When we install a plugin, we could save the metadata that the interface needs into plugin_data ? Why read it every time? When we upgrade the plugin, we replace it. Why load every module on a plain GetPlugins call ?

I was thinking about this, and the problem is the plugin metadata may change at any time. This is a more complicated problem than I had initially thought. We could A) update the metadata ever time the plugin is loaded, or B) Update the plugin metadata any time InstallPlugins is run. In any case this is outside the scope of this bug and requires a bit of thought before I implement anything. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #35 from Kyle M Hall <kyle@bywatersolutions.com> --- I think I've changed my mind about making this a followup bug. The new code for based on Marcel's thoughts is much more elegant. I'm going to post the change there for posterity, then close the bug and merge the changes in here as well. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103722|0 |1 is obsolete| | --- Comment #36 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 103982 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=103982&action=edit Bug 24631: Plugin metadata should be outside the main class Currently, plugin metadata is stored within the main plugin class file but this means one has to load the class to read it. We should pull this data out into a manifest/metadata file in a standard format (JSON/YAML) to allow simpler external parsing. Test Plan: 1) Apply this patch 2) Install the latest Kitchen Sink plugin: https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases/downlo... 3) Browse to plugins home, note that nothing appears to have changed 4) Browse to your pluginsdir, find the PLUGIN.yml file for the Kitchen Sink plugin, edit some of the metadata 5) Reload the plugins home page, note your changes to PLUGIN.yml appear in the plugins table! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #38 from Victor Grousset/tuxayo <victor@tuxayo.net> --- For some reason I'm not getting any change. Whether from PLUGIN.yml or KitchenSink.pm On a fresh koha-testing-docker after restart_all and flush_memcached The previous version of the patch (when I signed it off) works. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #40 from David Cook <dcook@prosentient.com.au> --- I'm a bit confused. So you're storing metadata in PLUGIN.yml *and* in plugin_data depending on the scenario? That seems suboptimal. Why not just use the plugin_data? You could read PLUGIN.yml into plugin_data at install/upgrade time, and then treat the database as the source of truth. I suppose plugin_data is really more of a key-value data store, so maybe it would be better to have a `plugins` table that stores 1 row per plugin for standard plugin metadata. Then you use that for all plugin management purposes. Doing it that way you would have 1 source of truth and you wouldn't have any arbitrary code execution. For new plugins, you use PLUGIN.yml at install/upgrade time (after which it becomes irrelevant). For old plugins, you use the insecure method of extracting metadata from the plugin code. Then you eventually deprecate the insecure method. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #42 from Victor Grousset/tuxayo <victor@tuxayo.net> --- It works :) @Kyle: Is there another way to test than modifying the archive? (so the test plan can be fixed) It's easy on Linux with file-roller, the GUI for archives that most people have. Also, bad news, without the patch I can install and uninstall a plugin. But with the patch, I can install but not uninstall.

Can't locate .pm in @INC

92: require $plugin_path; # Require the plugin to have it's path listed in INC

So $plugin_path is undef? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #44 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to David Cook from comment #40)

I'm a bit confused.

So you're storing metadata in PLUGIN.yml *and* in plugin_data depending on the scenario? That seems suboptimal.

Why not just use the plugin_data? You could read PLUGIN.yml into plugin_data at install/upgrade time, and then treat the database as the source of truth.

I suppose plugin_data is really more of a key-value data store, so maybe it would be better to have a `plugins` table that stores 1 row per plugin for standard plugin metadata. Then you use that for all plugin management purposes.

Doing it that way you would have 1 source of truth and you wouldn't have any arbitrary code execution.

For new plugins, you use PLUGIN.yml at install/upgrade time (after which it becomes irrelevant). For old plugins, you use the insecure method of extracting metadata from the plugin code. Then you eventually deprecate the insecure method.

In a nutshell, GetPluginsMetadata will look for plugin metadata in the database, and if it's not there, get it from the plugin via get_metadata and store it in the db. get_metadata looks in various places based on speed of access. First it looks in itself at the metadata key, then the metadata package var, then the database, then the PLUGIN.yml file. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #103983|0 |1 is obsolete| | --- Comment #45 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 104023 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104023&action=edit Bug 24631: Plugin metadata should be outside the main class Currently, plugin metadata is stored within the main plugin class file but this means one has to load the class to read it. We should pull this data out into a manifest/metadata file in a standard format (JSON/YAML) to allow simpler external parsing. Test Plan: 1) Apply this patch 2) Install the latest Kitchen Sink plugin: https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases/downlo... 3) Look in plugin_data for the new "__METADATA__" key, note it has the metadata in it 4) Alter the metadata in KitchenSink.pm, delete __METADATA__ from plugin_data 5) Browse to plugins home, note that nothing appears to have changed 6) Note the metadata in plugin_data has been regenerated 7) Browse to your pluginsdir, find the KitchenSink.pm file for the Kitchen Sink plugin, edit some of the metadata 8) Delete the plugin_data metadata again 9) Reload the plugins home 10) Note the metdata has been updated 11) Remove lines 28-40, 47-49 from KitchenSink.pm, this will cause Koha to look at PLUGIN.yml instead 12) Clear the metadata from plugin_data again 13) Reload the plugins home 14) Note again the metadata is regenerated in plugin_data 15) Browse to your pluginsdir, find the PLUGIN.yml file for the Kitchen Sink plugin, edit some of the metadata 16) Clear the metadata from plugin_data again 17) Reload the plugins home 18) Note again the metadata is regenerated in plugin_data 19) Reload the plugins home page, note your changes to PLUGIN.yml appear in the plugins table! Signed-off-by: Victor Grousset/tuxayo <victor@tuxayo.net> -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104023|0 |1 is obsolete| | --- Comment #47 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 104024 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104024&action=edit Bug 24631: Plugin metadata should be outside the main class Currently, plugin metadata is stored within the main plugin class file but this means one has to load the class to read it. We should pull this data out into a manifest/metadata file in a standard format (JSON/YAML) to allow simpler external parsing. Test Plan: 1) Apply this patch 2) Install the latest Kitchen Sink plugin: https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases/downlo... 3) Look in plugin_data for the new "__METADATA__" key, note it has the metadata in it 4) Alter the metadata in KitchenSink.pm, delete __METADATA__ from plugin_data 5) Browse to plugins home, note that nothing appears to have changed 6) Note the metadata in plugin_data has been regenerated 7) Browse to your pluginsdir, find the KitchenSink.pm file for the Kitchen Sink plugin, edit some of the metadata 8) Delete the plugin_data metadata again 9) Reload the plugins home 10) Note the metdata has been updated 11) Remove lines 28-40, 47-49 from KitchenSink.pm, this will cause Koha to look at PLUGIN.yml instead 12) Clear the metadata from plugin_data again 13) Reload the plugins home 14) Note again the metadata is regenerated in plugin_data 15) Browse to your pluginsdir, find the PLUGIN.yml file for the Kitchen Sink plugin, edit some of the metadata 16) Clear the metadata from plugin_data again 17) Reload the plugins home 18) Note again the metadata is regenerated in plugin_data 19) Reload the plugins home page, note your changes to PLUGIN.yml appear in the plugins table! 20) Test that you can enable/disable the plugin 21) Test that all the plugin methods ( configure, report, tool ) still work 22) Test that you can uninstall the plugin -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Victor Grousset/tuxayo <victor@tuxayo.net> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Needs Signoff |Failed QA --- Comment #49 from Victor Grousset/tuxayo <victor@tuxayo.net> ---

3) Look in plugin_data for the new "__METADATA__" key, note it has the metadata in it

Test note: select plugin_value from plugin_data where plugin_key = '__METADATA__' \G

4) Alter the metadata in KitchenSink.pm, delete __METADATA__ from plugin_data

That's a valid way to do it right? : delete from plugin_data where plugin_key = '__METADATA__';

5) Browse to plugins home, note that nothing appears to have changed

I can see my edits to KitchenSink.pm, that's not expected right? -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #51 from Victor Grousset/tuxayo <victor@tuxayo.net> ---

5) Browse to plugins home, note that nothing appears to have changed

I still the data from the .pm edited on step 4

11) Remove lines 28-40, 47-49 from KitchenSink.pm, this will cause Koha to look at PLUGIN.yml instead

«this will cause» IIUC that only a hint about what should happen next. Not an actual check to perform now. Seems clear, moving on.

13) Reload the plugins home

error!

Can't call method "isa" on an undefined value at /kohadevbox/koha/Koha/Plugins/Base.pm line 337

note: I restart_all after the KitchenSink.pm -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Status|Failed QA |Needs Signoff -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #54 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Victor Grousset/tuxayo from comment #52)

I did restart_all again and now I'm getting endless of these errors in the logs.

...

Use of uninitialized value $bundle_path in concatenation (.) or string at /kohadevbox/koha/Koha/Plugins/Base.pm line 193. Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load application from file "/kohadevbox/koha/api/v1/app.pl": Can't call method "isa" on an undefined value at /kohadevbox/koha/Koha/Plugins/Base.pm line 337. Compilation failed in require at (eval 402) line 1. Use of uninitialized value $bundle_path in concatenation (.) or string at /kohadevbox/koha/Koha/Plugins/Base.pm line 193. Error while loading /etc/koha/sites/kohadev/plack.psgi: Can't load application from file "/kohadevbox/koha/api/v1/app.pl": Can't call method "isa" on an undefined value at /kohadevbox/koha/Koha/Plugins/Base.pm line 337. Compilation failed in require at (eval 402) line 1.

I found an issue with my patch and my test plan was incomplete. Plack needs to be restarted after clearing the plugins data table because Plack keeps the plugin modules in memory. Deleting the plugin data and restarting Plack at the same time forces Koha to fetch the metadata from the original source, be it the plugin module or a PLUGIN.yml file. Please give it another try! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Martin Renvoize <martin.renvoize@ptfs-europe.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Blocks| |22832 Referenced Bugs: https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=22832 [Bug 22832] Require plugins to specify the methods they implement -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Kyle M Hall <kyle@bywatersolutions.com> changed: What |Removed |Added ---------------------------------------------------------------------------- Attachment #104256|0 |1 is obsolete| | --- Comment #56 from Kyle M Hall <kyle@bywatersolutions.com> --- Created attachment 104266 --> https://bugs.koha-community.org/bugzilla3/attachment.cgi?id=104266&action=edit Bug 24631: Plugin metadata should be outside the main class Currently, plugin metadata is stored within the main plugin class file but this means one has to load the class to read it. We should pull this data out into a manifest/metadata file in a standard format (JSON/YAML) to allow simpler external parsing. Test Plan: 1) Apply this patch 2) Install the latest Kitchen Sink plugin: https://github.com/bywatersolutions/koha-plugin-kitchen-sink/releases/downlo... 3) Look in plugin_data for the new "__METADATA__" key, note it has the metadata in it 4) Alter the metadata in KitchenSink.pm, delete __METADATA__ from plugin_data Restart Plack 5) Browse to your pluginsdir, find the KitchenSink.pm file for the Kitchen Sink plugin, edit some of the metadata 6) Delete the plugin_data metadata again Restart Plack 7) Reload the plugins home 8) Note the metdata has been updated 9) Remove lines 28-40, 47-49 from KitchenSink.pm, this will cause Koha to look at PLUGIN.yml instead 10) Clear the metadata from plugin_data again Restart Plack 11) Reload the plugins home 12) Note again the metadata is regenerated in plugin_data 13) Browse to your pluginsdir, find the PLUGIN.yml file for the Kitchen Sink plugin, edit some of the metadata 14) Clear the metadata from plugin_data again Restart Plack 15) Reload the plugins home 16) Note again the metadata is regenerated in plugin_data 17) Reload the plugins home page, note your changes to PLUGIN.yml appear in the plugins table! 18) Test that you can enable/disable the plugin 19) Test that all the plugin methods ( configure, report, tool ) still work 20) Test that you can uninstall the plugin -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #58 from Victor Grousset/tuxayo <victor@tuxayo.net> ---

Heh, I have no idea what I was thinking when I wrote that part of the test plan! If you delete the metadata in the db, and restart plack, *of course* the metadata will get updated from your altered plugin!

I expected it would be from PLUGIN.yml Ok, so retesting! -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #60 from Victor Grousset/tuxayo <victor@tuxayo.net> ---

19) Test that all the plugin methods ( configure, report, tool ) still work

Report doesn't work or maybe I don't know how to use it. Whatever, it has surely nothing to do with this. In conclusion: 🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉🎉 -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #61 from Kyle M Hall <kyle@bywatersolutions.com> --- (In reply to Victor Grousset/tuxayo from comment #58)

...

Heh, I have no idea what I was thinking when I wrote that part of the test plan! If you delete the metadata in the db, and restart plack, *of course* the metadata will get updated from your altered plugin!

I expected it would be from PLUGIN.yml Ok, so retesting!

As an FYI, the idea here is to check the database for the metadata to avoid loading the plugin. If it's not in the database, we load the plugin and ask it for its' metadata. When we as a plugin for it's metadata it checks A) For metadata in it's own Perl module ( fastest ) B) Checks the database ( second fastest, for convenience when the plugin is already loaded, is skipped when upgrading plugins ) C) The PLUGIN.yml file if it exists ( slowest, but these speeds are all relative to each other ;) -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #63 from David Cook <dcook@prosentient.com.au> --- (In reply to Julian Maurice from comment #8)

Won't that be a problem if people want to publish their plugins on CPAN ?

I've been thinking about this more, and while I was skeptical at first... I actually kind of like the idea now. For vendors, it could be easier to provide plugins via the system rather than at an instance basis. Coupled with a whitelist or some other "enable/disable" system per instance, it could make plugin management easier. -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Jonathan Druart <jonathan.druart@bugs.koha-community.org> changed: What |Removed |Added ---------------------------------------------------------------------------- Keywords|rel_20_11_target | CC| |jonathan.druart@bugs.koha-c | |ommunity.org -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 --- Comment #64 from Julian Maurice <julian.maurice@biblibre.com> --- (In reply to Kyle M Hall from comment #61)

As an FYI, the idea here is to check the database for the metadata to avoid loading the plugin. If it's not in the database, we load the plugin and ask it for its' metadata. When we as a plugin for it's metadata it checks A) For metadata in it's own Perl module ( fastest ) B) Checks the database ( second fastest, for convenience when the plugin is already loaded, is skipped when upgrading plugins ) C) The PLUGIN.yml file if it exists ( slowest, but these speeds are all relative to each other ;)

I am not sure if I understood that part correctly. You are saying that plugin's metadata can be in 3 different places (Perl module, database, and YAML file) now ? And in order to read the YAML file we need to load the module first ? What are the benefits ? You are talking about performance of different methods. Have you measured it ? Do we really gain that much by storing metadata in MySQL ? Have you considered caching metadata in memcache ? I thought that the yaml file would become the only source of truth and that it would allow us to avoid loading the Perl module when not needed (especially for disabled modules). I'm sorry but I really don't see what we gain here. If this is for easing parsing by external tools, then you can just dump the Perl module's metadata into a YAML file when you create the .kpz -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Fridolin SOMERS <fridolin.somers@biblibre.com> changed: What |Removed |Added ---------------------------------------------------------------------------- CC| |fridolin.somers@biblibre.co | |m -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Bug 24631 depends on bug 23975, which changed state. Bug 23975 Summary: Add ability to search and install plugins from GitHub https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23975 What |Removed |Added ---------------------------------------------------------------------------- Status|Pushed to master |RESOLVED Resolution|--- |FIXED -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Bug 24631 depends on bug 23975, which changed state. Bug 23975 Summary: Add ability to search and install plugins from GitHub https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=23975 What |Removed |Added ---------------------------------------------------------------------------- Status|RESOLVED |REOPENED Resolution|FIXED |--- -- You are receiving this mail because: You are watching all bug changes.

https://bugs.koha-community.org/bugzilla3/show_bug.cgi?id=24631 Martin Renvoize (ashimema) <martin.renvoize@openfifth.co.uk> changed: What |Removed |Added ---------------------------------------------------------------------------- See Also| |https://bugs.koha-community | |.org/bugzilla3/show_bug.cgi | |?id=41651 -- You are receiving this mail because: You are watching all bug changes.