User tests: Successful: Unsuccessful:
Pull Request for Issue #45653 .
During extension installation, when calling Installer, Database not set in Joomla\CMS\Installer\Installer error is displayed
Install an extension that calls Installer function, like https://www.joomlack.fr/en/joomla-extensions/page-builder-ck
Installation fails with Database not set in Joomla\CMS\Installer\Installer error
Installation is OK
Please select:
Documentation link for docs.joomla.org:
No documentation changes for docs.joomla.org needed
Pull Request link for manual.joomla.org:
No documentation changes for manual.joomla.org needed
Status | New | ⇒ | Pending |
Category | ⇒ | Libraries |
Labels |
Added:
PR-6.0-dev
|
I have tested this item ✅ successfully on e57a3b4
Joomla 6.0.0-Alpha 3 PHP 8.3.14 Completed Successful
The install component fails without the patch.
I have tested this item ✅ successfully on e57a3b4
Joomla 6.0.0-alpha4-dev, PHP 8.3.24, MySQL 8.4.0
Installation of extension successfully installed which fails without the patch.
Thank you @Elfangor93.
Let's wait for @rdeutz 's review.
@conseilgouz Not a good idea to use the branch update button when a PR has successful human tests because this resets the test counter, and if that PR has 2 tests and not RTC yet, it might get lost. I will restore the test counter now. But keep it in mind for the next time.
Status | Pending | ⇒ | Ready to Commit |
RTC
Thank you @richard67
No comment about : #45670 (comment) ?
Pascal
@Hackwar Could you check that? Thanks in advance.
Labels |
Added:
RTC
|
I understand what you are trying to do here, but I don't think it is right. The goal is to inject the database object from the outside. Now however you are always fetching it from the container. This is actually a step backwards and you might as well just use Factory::getDbo() instead. If you are creating your own Installer instance, you also have to do the proper setup and inject the database object into it. I'm also not sure if this is a new issue, since the same was necessary in J5 already as well. I wouldn't merge this PR.
This PR is to fix issue #45653 : some extensions installation/update won't work in J6.0 without this PR (Joomgallery, pagebuilderCK, ... and one of my onw).
As stated in #45653 (comment) , it could be fixed by adding a note in installation documentation, but who reads it.
The code in the past which allowed this was https://github.com/joomla/joomla-cms/blob/5.3-dev/libraries/src/Adapter/Adapter.php#L92
As I wrote, this is actually expected behavior. We want injection and if we want to actually make this a reality, we have to break this at some point. The commit introducing this into the Installer was here: #37625
Whats the difference between injectibg the DB object by myself or take it automatically from the Container if its not yet set?
As far as I see, I could still inject my own instance..
Note : in J5.3, setDatabase was performed in Adapter construct function.
That's missing in the new Installer construct function.