[#37091] - [4.2] Extension installer script enhancements
- Closed
- 24 Mar 2022
- Medium
- Build: 4.2-dev
- # 37091
- Diff
- Digital-Peak:j4/installer/service
User tests: Successful: Unsuccessful:
Summary of Changes
This pr adds an interface for the installer script to have a common way for installer actions for extensions. Additionally the extension can use the container for to get dependencies during install operation.
With this new approach the extension developer doesn't have to create a class with a special class name like mod_installer_legacyInstallerScript anymore and has more freedom and flexibility during the install routine. Using the old way is getting deprecated. It is one step further to use injected dependencies.
Examples can be found here:
- Container based script mod_installer_container.zip
- Script based script mod_installer_script.zip
- Legacy script mod_installer_legacy.zip
A script can look like:
<?php
use Joomla\CMS\Application\AdministratorApplication;
use Joomla\CMS\Installer\InstallerAdapter;
use Joomla\CMS\Installer\InstallerScriptInterface;
use Joomla\DI\Container;
use Joomla\DI\ServiceProviderInterface;
return new class () implements ServiceProviderInterface {
public function register(Container $container)
{
$container->set(InstallerScriptInterface::class, new class ($container->get(AdministratorApplication::class)) implements InstallerScriptInterface {
private $app;
public function __construct(AdministratorApplication $app)
{
$this->app = $app;
}
public function install(InstallerAdapter $parent): bool
{
$this->app->enqueueMessage('Installed by container!!');
return true;
}
public function update(InstallerAdapter $parent): bool
{
$this->app->enqueueMessage('Update by container!!');
return true;
}
public function uninstall(InstallerAdapter $parent): bool
{
return true;
}
public function preflight(string $type, InstallerAdapter $parent): bool
{
return true;
}
public function postflight(string $type, InstallerAdapter $parent): bool
{
return true;
}
});
}
};
Or return directly an instance of the installer script interface:
<?php
use Joomla\CMS\Factory;
use Joomla\CMS\Installer\InstallerAdapter;
use Joomla\CMS\Installer\InstallerScriptInterface;
return new class () implements InstallerScriptInterface {
public function install(InstallerAdapter $parent): bool
{
Factory::getApplication()->enqueueMessage('Installed by instance!!');
return true;
}
public function update(InstallerAdapter $parent): bool
{
Factory::getApplication()->enqueueMessage('Update by instance!!');
return true;
}
public function uninstall(InstallerAdapter $parent): bool
{
return true;
}
public function preflight(string $type, InstallerAdapter $parent): bool
{
return true;
}
public function postflight(string $type, InstallerAdapter $parent): bool
{
return true;
}
};
The legacy way is still supported for backwards compatibility.
Testing Instructions
- Install the container module from the link above
- Install the script module from the link above
- Install the legacy module from the link above
Actual result BEFORE applying this Pull Request
- The container module shows no text "Installed by container!!"
- The container module shows no text "Installed by script!!"
- The container module shows text "Installed by legacy!!"
Expected result AFTER applying this Pull Request
- The container module shows text "Installed by container!!"
- The container module shows text "Installed by script!!"
- The container module shows text "Installed by legacy!!"
Documentation Changes Required
Needs to be documented in the docs.
| Status | New | ⇒ | Pending |
joomla-cms-bot
- | Category | ⇒ | Libraries |
| Labels |
Added:
?
|
||
sorry drone errors are my fault
| Title |
|
||||||
I have tested this item
While your test extensions install, I cannot say the same for any real world extensions. Tested on PHP 8.0.15. They all end up displaying an empty red error box during installation. I used all the free of charge, no user account required for download, Joomla 4 compatible extensions from the first page of JED's Top Rated extensions. I tested them on Joomla 4.1 and 4.2-dev+your PR on PHP 8.1. Here are the extensions which failed to install with your PR but installs just fine on 4.1:
- Akeeba Backup 9.2.0
- JCE 2.9.18
- AcyMailing Starter 7.7.5
- JEvents 3.6.30
- Phoca Gallery 4.5.0Beta2
- SP PageBuilder Lite 3.8.3
- Advanced Module Manager 9.0.7
- ARK Editor 2.8.6
Both 4.1 and 4.2-dev+pr sites are on the same local server setup, the same I use as my primary development and testing environment.
As long as existing, real world, top rated extensions compatible with Joomla 4.1 are not installable without any modification I consider this PR fundamentally broken.
My suspicion without having actually debugged the issue is that you only tested on PHP 7 or didn't actually test with real world extensions on PHP 8. I see that you have introduced static typing in the signatures for the installation scripts' superclasses. However, this is incompatible with PHP 8.0 because the implementation (in our software's installation scripts) has a different signature than the superclass. We can not change our software because it would become impossible to install on Joomla 4.0 and 4.1 for the same reason!!! This means we could never publish software which is compatible with both Joomla 4.1 and 4.2, making updating Joomla! sites impossible. The simple solution is to NOT MESS with the legacy script superclasses. Do NOT add static typing to the method parameters and method results. Leave them as-is.
Let me know when you fix that so I can retest.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.
And keep in mind:
Any breaking changes (no matter how small the break is or how small the group is of people/extensions it impacts), means a MAJOR version release.
I am not against breaking changes (if there are ways to deal with it). But it simply means it means that release should be Joomla 5.0.0.
Seeing Joomla says it uses semantic versioning, but still uses major versions as a marketing tool, a breaking change will take ages to find its way into the stable release.
@regularlabs Even worse, if such a breaking change was included in Joomla it could not even go into Joomla 5 because then we could never make extensions compatible with Joomla 4.LAST and 5.0, meaning nobody could update their sites! This is why the extensions installer can never have breaking changes. It needs to have continuity.
Once the problems with this PR are fixed it can be included to Joomla 4.2. Here's how it would look for us:
- For as long as we support Joomla 4.0 and 4.1 we stick with the legacy script
- When we stop supporting Joomla 4.0/4.1 we move to the 4.2+ container approach
- Come Joomla 5 our software is already compatible (at least as far as installation goes), nothing to do.
- Come Joomla 6 the legacy scripts are dropped. Nothing breaks unless it's something which hadn't been updated since before 5.0 which, okay, it's too old to support new PHP versions anyway.
So, @laoneo 's approach is thoughtful — as long as the legacy scripts don't break. I have to say ‘cheated’ a bit in my testing. I did check the code first and saw he changed the method signature. I had already hit a problem with PHP 8 when I had added static typing to my installation script methods but Joomla 4.0 didn't have matching signatures in the superclass. I saw that and thought “oh, shoot, do you think this means all extension installation is broken with this PR?”. I checked and yup, it's broken. His test extensions use the new signatures which is why he never noticed. So, really, my experience breaking things for myself came in handy here :D
Yup, been there.
Ok, had to remove the interface from the default InstallerScript class as I want to keep the type hinting in the interface. Additionally the Legacy installer ensures now to return always a boolean.
I have tested this item
Thank you for the fixes @laoneo :) I can now confirm that it works with third party extensions. My stuff (including all of my other extensions I had not previously tested), JCE etc install. There are some extensions in the previous list not installing but they seem to deliberately fail the installation by doing a maximum Joomla version check, so not our monkey and not our problem.
@regularlabs You may want to check what's going on with Advanced Modules Manager on the 4.2-dev, I got an error which seems to come from your library installation (“Container not set in RLInstallerAdapterLibrary”).
As far as I am concerned this PR is now complete, functional and has a good backwards compatibility strategy. Thank you!
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.
@nikosdion can you test again the module manager? Should also work now.
Give me a sec, waiting for the build to complete on Drone so I can update the same site I'm using for testing. I'll post back when I have new results :)
Installing Advanced Module Manager throws an Internal Server Error. Everything else I tried works the same as before.
If you want to troubleshoot you can download the same free version of Advanced Module manager (9.0.7) I am using in my tests from the link on its JED page: https://extensions.joomla.org/extension/advanced-module-manager/
Module manager should work now as well.
I have tested this item
I can confirm that with all the latest changes I could install all extensions I had tried before, plus all of my own Joomla 4–only extensions.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.
Thanks @nikosdion, testing other extensions revealed a couple of issues which are solved now. Hopefully other extension devs will test their extensions as well. The @regularlabs extension used the installer very extensively, so I have a good feeling now.
It uses the API which is absolutely fine. Was only wondering what brought you that far to use your own Installer classes. But this can be discussed somewhere else...
@regularlabs can you test that pr also?
I'll try to do that this week...
How does this PR align with the TUF they are currently working on at the hackathon?
Don't know what a TUF is
"The Update Framework"
TUF would kick in even before the extension package is extracted, so it’s unrelated to the extension installer script.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.
I have tested this item
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.
| Status | Pending | ⇒ | Ready to Commit |
RTC
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.
| Labels |
Added:
?
|
||
| Status | Ready to Commit | ⇒ | Closed |
| Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2022-03-24 15:56:01 |
| Closed_By | ⇒ | roland-d |
It's no longer possible to install any extension after this PR using the legacy script currently working on 4.1
Whats going on there?
It seems that there is a serious b/c break here. I've found that all functions of the legacy installer script must have an explicit 'return true' otherwise this error is the result.
I can confirm this: fresh installed Joomla 4.2, no able to install extensions
My best guess is that would affect core updates, not extension updates. Of course I’ve already talked about that in checks notes ah right 2017. Why does it take Joomla 5 years to act on something basic is beyond me but then again it’s Joomla so I’m not even surprised.
Anyway, can you check this PR Peter so maybe just maybe we can improve on the crappy, outdates extensions upgrade code since the JED doesn’t actually allow us to use our own update frameworks?
I have changed the way my installer works (now using more of Joomla's own manifest capability). So that could hopefully also deal with some issues the new installer ran into.
I will try to test this out next week and see what I find...
I have no issues installing my packages on the current nightly build: 4.2.4-dev
Thanks @brianteeman