User tests: Successful: Unsuccessful:
Pull Request for Issue #40430.
In #40430 an issue came up where updating failed because the framework class File wasn't available in /administrator/components/com_admin/script.php. This PR reverts that change from #40257. We will eventually have to switch over the whole update process from File/Folder/etc to the framework classes and update the finalisation.php as well.
Codereview.
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
Category | ⇒ | Administration com_admin |
Status | New | ⇒ | Pending |
@laoneo @MacJoom This PR here is one of the few rare cases where a review is sufficient and clear. Just check what is in file extract.php of the update component. Check the namespace used there for the mocked file system libs, and check that the change in script.php fits to that. When doing updates it can happen on the same environment that one time the real filesystem libs or the stuff in extract.php is used, and another time the real lib. So it might be hard to catch the issue with a real test.
On some point we want to make the switch to the framework classes. We will face then the same issue again. So it makes sense to invest a bit of resources as we are aware of the issue right now and do some testing on what to do to properly fix it. Because in my opinion all is needed is to change the namespace here. This basically what Nik suggested in the comment #40430 (comment). So I would prefer to see if that fix would work, but first we need to be able to reproduce it.
@laoneo No it is not sufficient to change the namespace at that one place in extract.php because this namespace contains the mocks for the file and for the folder class and the CMS has not been changed yet to move to the framework for the folder class, so we would need 2 namespaces in extract.php as long as that is not done.
Yes, @richard67, thanks for explaining this. Indeed, we will have to switch over at one point, but that would happen when we have switched all other code over. At that point we would also have to check the classes which are included in the finalisation.php and update that code where necessary. With this, we actually don't have one or two, but three different implementations of the File and Folder class and I'm pretty sure that they are all not in sync to each other. So there is more work to do than just to change the namespace of the filesystem package in the finalisation.php.
Can we then not just load composer in the finalization script and get completely rid of the fake namespaces?
For now I would say this PR here is the quick fix so people can test updating with nightlies, and the bigger clean-up we do soon after that and have it hopefully ready for 4.4-alpha1.
Can we then not just load composer in the finalization script and get completely rid of the fake namespaces?
I don‘t know, but would be cool of course if we could get rid of the fake namespace.
Labels |
Added:
PR-4.4-dev
|
Agree that we should first accept here and then have a look if we can deal something with the framework package.
Status | Pending | ⇒ | Fixed in Code Base |
Closed_Date | 0000-00-00 00:00:00 | ⇒ | 2023-05-12 08:10:05 |
Closed_By | ⇒ | laoneo |
I have tested this item✅ successfully on 91e63b8
Code review.
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/40569.