-
-
Notifications
You must be signed in to change notification settings - Fork 3.8k
[5.4] Fix removing the installation folder on Windows #46584
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: 5.4-dev
Are you sure you want to change the base?
Conversation
|
I have tested this item 🔴 unsuccessfully on 18f2fc5 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46584. |
|
@brianteeman I didn't expected this because I tested it... which setup? with xampp or another windows server setup? |
|
maybe it's the developer version which doesn't delete the folder and does something different, could you try to use only the patch on a 5.4 version? |
|
using laragon 8.3 with php 8.5 I wrote as expected because the explanation @richard67 wrote earlier is not changed by this pr |
|
Just tested and it worked OK for me. I downloaded Joomla 5.4.1, then patched the file manually before running installation process. The installation folder was completely deleted. I'm using Xampp |
|
tested again with a download of the released j.5.4.1 where i replaced the changed file with the file in this PR exactly the same |
|
thanks, I will test it with laragon |
|
j4i with xampp it works also with this pre-built package and php 8.2 by pressing the delete installation button. |
|
PS from my perspective it's a very low level issue |
|
While looking at the code I found the comment where it actually says it's expected behavior and I don't know way the issue wasn't closed... but since I had a working solution for me I continued with the pr... |
|
Don't know whats different in your setup, but I installed Laragon 8.4.0 on a new windows 11 with php 8.3, extracted joomla from this pr and was able to remove the installation folder as expected... |
|
I have not tested this item. This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46584. |
|
no idea why it doesnt work for me - but if it works for others then great |
|
I have tested this item ✅ successfully on 18f2fc5 This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46584. |
|
No idea. I did not expect that it works, because in former installations I had another error message tha @brianteeman and thought, it is laragon stuff+´ |
Better safe than sorry.
|
I decided to add back the old workaround, better safe than sorry. @brianteeman can you test again that at least the old workaround works for you. |
|
@joomdonation @chmst Have you also tested that it fails on your environment without the PR applied? Fails means there is an empty "installation" folder after the installation. And you should test with a stable version so the "installation" folder would be deleted automatically in any case, not having the choice with the button like you have it in a development environment or a development version (nightly build, PR package or any pre-release). |
|
A fresh check using wampserver 64 with joomla 5.4.1 stable didn´t meet the BEFORE issue. The /installation folder is actually deleted here in this setup without aplying any fix. |
|
In my case, before this PR, I still have an empty installation folder left. After this PR, the installation completely deleted, so it worked for me. I'm using xampp, but PHP 8.4 downloaded and configured separately, not coming from xampp. |
|
@joomdonation I just see you haven't marked a test result in the issue tracker? @brianteeman @chmst Could you test again with the latest changes. Thanks in advance. |
This is a very low priority issue and the current status is acceptable (with empty installation folder). So I will wait for Brian test to see if he still see error with the latest change. If there is no error anymore, I will report my test result. If there is still error, maybe there is still some conditions cause errors which we don't know and in that case, better we leave it as how it is. |
|
retested with the latest changes using the full download from this PR with laragon 8.4 and php 8,.4.8 and the behaviour is the same - i still have the empty installation folder (I deleted a previous comment as that was my error) |
|
Re-tested successfully. On laragon, win11, php 8.3.16 in a completely new directory used the download package Joomla_5.4.1-Stable-Full_Package.zip Then tested with Joomla_5.4.2-dev+pr.46584-Development-Full_Package.zip |
|
Since the old workaround is still in place and it works a bit better for some people then before I would say it's acceptable to merge this till someone want's to invest more time to solve windows problems. |
@HLeithner It might come too late for 5.4.2 and 6.0.2 as we are already preparing the stable packages today. So it might be 5.4.3 and 6.0.3 (and 6.1.0-beta1) in February where we can get it in. I will see if I can reactivate my Windows environment where I could reproduce the issue and if so I can also test it. |
|
It's not urgent at all, so 5.4.3 is of course fine. |
|
I have tested this item ✅ successfully on c668b71 Without the fix, an empty installation folder is remaining after an installation of a 5.4 stable and using one of the buttons to go to site or administrator. With the fix the installation folder is completely removed. In both cases the installation has been made in a subfolder of the webserver's document root, i.e. site URL is Environment:
This comment was created with the J!Tracker Application at issues.joomla.org/tracker/joomla-cms/46584. |
|
I tried to test this PR on Windows 11, WAMP server 3,3,7, PHP 8.3.19, Apache 2.4.62 |

Pull Request for Issue #37909 .
Summary of Changes
Instead of checking if a file has not been deleted form the installation folder, move the expected file to the
tmpfolder and delete it after the move. This allows to delete the complete installation folder. because the lock is no longer on a file in the installation folder. Instead the lock is in thetmpfolder which we don't delete. The lock on the moved file will be freed after the the php process finishes and windows finally deleted the move file.Testing Instructions
Install joomla on windows with xampp or any other windows php server.
Actual result BEFORE applying this Pull Request
Expected result AFTER applying this Pull Request
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