We are moving to Git Issues for bug tracking in future releases. During transition, content will be in both tools. If you'd like to file a new bug, please create an issue.

Bug 5943 - Plugin installed via PluginManagementHandler does not install schema file.
Plugin installed via PluginManagementHandler does not install schema file.
Status: REOPENED
Product: OJS
Classification: Unclassified
Component: Plug-ins
2.4.x
PC Linux
: P3 normal
Assigned To: PKP Support
: 5517 (view as bug list)
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2010-09-23 17:04 PDT by jerico
Modified: 2012-09-25 15:39 PDT (History)
3 users (show)

See Also:
Version Reported In: 2.3.2
Also Affects: OCS 2.3.4, OHS 2.3.2, OMP 1.0


Attachments
Dummy Plug-In (without schema) to check the file structure in the tar. (1.45 KB, application/octet-stream)
2010-09-23 17:31 PDT, jerico
Details
Updated dummy plugin with install.xml (2.45 KB, application/x-gzip)
2010-09-24 12:10 PDT, Matthew Crider
Details

Note You need to log in before you can comment on or make changes to this bug.
Description jerico 2010-09-23 17:04:10 PDT
Hi Matt,

one user in Germany has reported on the German forum that installing his plug-in via the plug-in management handler didn't install the schema file. I'll ask him to upload his plug-in here in this bug report. Is it ok if I assign this bug to you as the original author of the handler?

It seems the user is using 2.3.2.0, so I'm not sure whether this problem is fixed in 2.3.3 but as 2.3.2 still is our official release we should probably have a look at it...

What do you think?
Comment 1 Matthew Crider 2010-09-23 17:06:35 PDT
Yes, i'll take a look, I'll just have to see his plugin.  Its possible the plugin was just packaged incorrectly--There's a fairly strict procedure to packaging the plugins.
Comment 2 jerico 2010-09-23 17:30:07 PDT
Yes, I've explained that procedure. He sent me a dummy plug-in (without the db schema) just to test the structure which installed ok on 2.3.2, see attachment.

BTW: Originally (before 2.3.2) the plug-ins were expected to be packaged without a root folder. On the gallery most plug-ins come with a root folder (which I also find practical if you want to unzip the plug-in locally for debugging). After confirming with Alec I changed the handler so that plug-ins will be expected to contain a root folder. IIRC this change was first released in 2.3.2.

The original forum entry is here: http://www.carpet-project.net/forum/beitraege/ojs_ocs_omp_harvester/erzeugen_einer_tabelle_fuer_ein_ojs_plugin/seite/1/#pid303 

Even without reading German you should be able to check the file structure I recommended there.
Comment 3 jerico 2010-09-23 17:31:09 PDT
Created attachment 3234 [details]
Dummy Plug-In (without schema) to check the file structure in the tar.
Comment 4 Matthew Crider 2010-09-24 12:10:34 PDT
Created attachment 3244 [details]
Updated dummy plugin with install.xml

Florian, it looks like this was a packaging problem (and perhaps a documentation problem--This is rather confusing).  The plugin installer expects an install.xml file (or upgrade.xml file), and just a schema.xml will not work.  I've uploaded a new version of the plugin with an install.xml file and a change to the base class that references the new install descriptor.  If that is sufficient, could you close the bug?
Comment 5 jerico 2010-09-24 12:32:49 PDT
I'll make sure I report that back to the German forum. Thanks a lot, Matt, for looking into this!
Comment 6 jerico 2010-09-24 14:46:13 PDT
Just one more thing, Matt. It seems we have some unnecessary duplication there. The schema file is already present via the getInstallDataFile() method and should be installed via the post-installation hook by the installer independent of the installation method (manual or via upload). So why would I have to include it /again/ in the install.xml?

I think it's a bug if the plug-in upload does not call the post-installation hook (or doesn't register the plug-in so that the hooks get executed...). What do you think?
Comment 7 Matthew Crider 2010-09-24 15:05:51 PDT
Florian, I've just tested the dummy plugin with removing the getInstallSchemaFile() (which references install.xml) and including getInstallDataFile() (which returns the path to a schema.xml).  It works fine that way.  Is that what you meant?
Comment 8 jerico 2010-09-24 15:13:10 PDT
Yes, that's what I meant. Our user reported that this had not worked in his case. He said that having no install.xml and returning the schema file from getInstallDataFile() didn't actually install the table.
Comment 9 jerico 2010-09-24 15:16:10 PDT
The user also rightly asks why we have to duplicate the version number in install.xml and version.xml. Do you know why install.xml has been introduced to plug-ins in the first place?
Comment 10 Matthew Crider 2010-09-24 15:45:24 PDT
Actually, he's right Florian--I hadn't deleted the install.xml file (just the function that returns it), so when I did and just used the getInstallDataFile() function, the table doesn't get installed.  I guess if the install file is present and called install.xml, it will be loaded automatically (regardless if you implement getInstallSchemaFile()).

This was modeled after OJS itself, which requires an install.xml file to install the app and has the version in the install.xml file and version.xml file.  I'd propose, since the tool is working as its supposed to be, we postpone to 2.3.3 any enhancements to make the plugin install tool more plugin-centric (i.e. allow just schema files to be loaded without install files).  It would also be good to document how to package and install plugins in the wiki (which I can take on).

Just thinking out loud here...  It might also be cool to have a plugin gallery page built into OJS which can load a list of available plugins from one of our servers, and download and install them automatically.  With the installation tool already in place, its fairly trivial to automate this and we wouldn't have to package so many plugins with OJS (though there is a question of whether we want to provide the bandwidth for that).
Comment 11 jerico 2010-09-25 08:10:56 PDT
(In reply to comment #10)
> I guess if the install file is
> present and called install.xml, it will be loaded automatically (regardless if
> you implement getInstallSchemaFile()).

Hm, that seems to be another inconsistency (in addition to the one with the schema file) between the manual installation and installation via upload which makes both installation processes potentially incompatible.

The problem with this is: IMO it is very likely that plug-in authors will test only with one of the two installation processes. The current implementation means that even trivial plug-ins developed and tested for one installation process will potentially not work for the other (see the trouble our user had). This constrains the re-usability of plug-ins in the community which is the very reason of their existence. So I would disagree that the upload process currently "works as supposed".

> This was modeled after OJS itself, which requires an install.xml file to install
> the app and has the version in the install.xml file and version.xml file.  I'd
> propose, since the tool is working as its supposed to be,

Did plug-ins prior to 2.3 require an install.xml to be present? I thought that the install.xml requirement was only introduced with the creation of the plug-in uploader or was at least optional prior to 2.3. If this is true then this is a bug because it breaks backwards compatibility (as can be seen in the case reported by our user). I may be mistaken, though.

If my assumption is correct then I don't understand why we should replace a working installation procedure with another one especially if it breaks compatibility. I don't have a problem at all with an inconsistency between the OJS application and plug-in installation processes, especially if it makes plug-ins easier to install. I actually like the upload page very much, I just would like to make it work with pre-2.3 plug-ins and keep manual install and upload-install compatible at least for a long intermediate period. AFAIK manual install has not even be deprecated so far.

> we postpone to 2.3.3
> any enhancements to make the plugin install tool more plugin-centric

I guess you mean postpone to 2.3.4? See above, I currently think this is a bug not an enhancement request. But I'm unsure about that. That's why I've included Alec as he knows the pre-2.3 plug-ins out there much better than I do.

> It would also be good to
> document how to package and install plugins in the wiki (which I can take on).

Yeah, that would be very helpful. Apart from a Wiki entry we should also update all existing plug-in documentation to make sure that we don't have outdated information there. It seems our user was initially relying on some documentation describing the manual installation process that led him nowhere.

> It might also be cool to have a plugin gallery
> page built into OJS which can load a list of available plugins from one of our
> servers, and download and install them automatically.

Yes, I like that idea. As you say: It's worth asking whether PKP wants to host such a service. I propose you open a separate bugzilla entry for that one and post the question to pkp-support. I seem to remember that Alec once mentioned something similar but I don't remember well. Alec, do you remember?
Comment 12 Matthew Crider 2010-09-27 10:56:39 PDT
The general consensus is that this should be postponed for now, so reassigning to pkp-support and to 2.3.4.  

Things to consider:
-Making sure the plugin installer is as backwards-compatible as possible (e.g. to 2.2.3-era plugins which first introduced the require to require_once change).  This means that the plugin manager should accept schema.xml files without install.xml files.
-Direct installation from PKP plugin gallery (Though have to consider the impact of the requirement that the web server be allowed to write to the plugin directory, and probably require CURL)
-Better documentation.

Anything else?
Comment 13 jerico 2010-09-27 15:19:59 PDT
(In reply to comment #12)
> -Making sure the plugin installer is as backwards-compatible as possible (e.g.
> to 2.2.3-era plugins which first introduced the require to require_once
> change).  This means that the plugin manager should accept schema.xml files
> without install.xml files.

It also means that installation without a version.xml should work if possible (see how the upgrade process does this).
Comment 14 jerico 2010-09-30 17:39:18 PDT
*** Bug 5517 has been marked as a duplicate of this bug. ***
Comment 15 jerico 2010-09-30 17:42:40 PDT
I just discovered that it's ok to leave out the version attribute in the install.xml (the version from version.xml will be used instead). When we write a documentation for plug-in packaging then we should mention that!

That resolves /one/ of the potential inconsistencies but not all of them (see earlier comments).
Comment 16 jerico 2010-09-30 17:55:38 PDT
Just to document another requirement mentioned by Alec: We should get rid of the index.php file in the plug-in directory.

My proposition: Keep the index.php only for backwards compatibility and use it when it's provided. Otherwise use the following nomenclature:

$pluginClassName = ucfirst($pluginName).ucfirst($pluginCategory).'Plugin';
$pluginClassFile = $pluginClassName.'inc.php';

This gives us a "convention over configuration" approach while still keeping the ability to do something more complex via index.php if we really need to (don't think that that'll ever be necessary).
Comment 18 jerico 2010-12-06 12:48:00 PST
Reopening because there are still subtle differences between the web installation process and the script based installation process. The only way to configure an installation file should be via the PKPPlugin::getInstallDataFile(). Otherwise it is possible that an installation file is run twice.
Comment 19 Matthew Crider 2012-09-25 15:39:42 PDT
Also, plugins should be able to be installed from e.g. the JM's plugin list.  Legacy plugins or plugins that aren't packaged correctly by plugin authors can only be run by manually creating an entry in the versions table (or repackaging the plugin).  It should be easier to drop a plugin into the OJS filesystem and get it running.