Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[4.2] Extension installer script enhancements #37091

Merged
merged 31 commits into from Mar 24, 2022

Conversation

laoneo
Copy link
Member

@laoneo laoneo commented Feb 18, 2022

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:

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.

laoneo and others added 2 commits February 18, 2022 20:11
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
Co-authored-by: Brian Teeman <brian@teeman.net>
@laoneo
Copy link
Member Author

laoneo commented Feb 18, 2022

Thanks @brianteeman

Co-authored-by: Brian Teeman <brian@teeman.net>
@brianteeman
Copy link
Contributor

sorry drone errors are my fault

@laoneo laoneo changed the title [4.1] Extension installer script enhancements [4.2] Extension installer script enhancements Feb 21, 2022
@laoneo
Copy link
Member Author

laoneo commented Mar 20, 2022

@regularlabs can you test that pr also?

@regularlabs
Copy link
Contributor

I'll try to do that this week...

How does this PR align with the TUF they are currently working on at the hackathon?

@laoneo
Copy link
Member Author

laoneo commented Mar 20, 2022

Don't know what a TUF is 🤷‍♂️

@regularlabs
Copy link
Contributor

@regularlabs
Copy link
Contributor

"The Update Framework"

@SniperSister
Copy link
Contributor

TUF would kick in even before the extension package is extracted, so it’s unrelated to the extension installer script.

@dgrammatiko
Copy link
Contributor

I have tested this item ✅ successfully on 3e26ed8


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.

1 similar comment
@regularlabs
Copy link
Contributor

I have tested this item ✅ successfully on 3e26ed8


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.

@Quy
Copy link
Contributor

Quy commented Mar 23, 2022

RTC


This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/37091.

@Quy Quy added the RTC This Pull Request is Ready To Commit label Mar 23, 2022
@roland-d
Copy link
Contributor

@laoneo Could you please update with the base branch? Thank you.

@roland-d roland-d merged commit b46a392 into joomla:4.2-dev Mar 24, 2022
@joomla-cms-bot joomla-cms-bot removed the RTC This Pull Request is Ready To Commit label Mar 24, 2022
@roland-d
Copy link
Contributor

Thank you everybody

@roland-d roland-d added this to the Joomla 4.2.0 milestone Mar 24, 2022
@laoneo laoneo deleted the j4/installer/service branch March 24, 2022 18:58
@joeforjoomla
Copy link
Contributor

image

@joeforjoomla
Copy link
Contributor

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?

@joeforjoomla
Copy link
Contributor

Try to install for example Acymailing or whatever other extension:
image

@joeforjoomla
Copy link
Contributor

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.

@drmenzelit
Copy link
Contributor

I can confirm this: fresh installed Joomla 4.2, no able to install extensions

@laoneo
Copy link
Member Author

laoneo commented Mar 25, 2022

I'm on it, will come up with a pr soon.

@laoneo
Copy link
Member Author

laoneo commented Mar 25, 2022

Can you guys test #37374?

@nikosdion
Copy link
Contributor

nikosdion commented Oct 11, 2022 via email

@regularlabs
Copy link
Contributor

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...

@regularlabs
Copy link
Contributor

I have no issues installing my packages on the current nightly build: 4.2.4-dev

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet