-
-
Notifications
You must be signed in to change notification settings - Fork 3.7k
[6.0 alpha 2] Database not set in Joomla\CMS\Installer\Installer error #45670
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
base: 6.0-dev
Are you sure you want to change the base?
Conversation
Database not set in Joomla\CMS\Installer\Installer error
Note : in J5.3, setDatabase was performed in Adapter construct function. |
PR #43792 replaces parent::__construct call by its local calls. $this->db is missing from the new code. |
In #43792, libraries/src/Console/ExtensionInstallCommande.php has 2 calls to Installer (line 151 and line 184). I think that setDatabase lines (152 and 185) are not required anymore as this will be done in Installer.php construct function (if this PR is accepted). |
I have tested this item ✅ successfully on e57a3b4 The install component fails without the patch. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45670. |
Thank you @bcordis |
I have tested this item ✅ successfully on e57a3b4 Installation of extension successfully installed which fails without the patch. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45670. |
Thank you @Elfangor93. |
@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. |
RTC This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/45670. |
Thank you @richard67 No comment about : #45670 (comment) ? Pascal |
@Hackwar Could you check that? Thanks in advance. |
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 |
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.. |
Pull Request for Issue #45653 .
Summary of Changes
During extension installation, when calling Installer, Database not set in Joomla\CMS\Installer\Installer error is displayed
Testing Instructions
Install an extension that calls Installer function, like https://www.joomlack.fr/en/joomla-extensions/page-builder-ck
Actual result BEFORE applying this Pull Request
Installation fails with Database not set in Joomla\CMS\Installer\Installer error
Expected result AFTER applying this Pull Request
Installation is OK
Link to documentations
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