Why should it be checked if a folder is empty before deleting it?

If you have put your own files in the folder then they would be removed on update

@wilsonge I think it needs a decision here if we want to keep things like they are, i.e. folders are deleted even if they contain files which we haven't removed before - but in this case I ask myself why we remove the files before instead of just removing the folder - or if we shall extend the folders deletion in script.php by a check (with use of glob) if the folder is empty and not delete it if not.

If you have put your own files in the folder then they would be removed on update

Are there valid reasons to put custom files in core folders?

Similiar issue would come up once we create a file with the same name that was used as custom filename right?

Are there valid reasons to put custom files in core folders?

If you haven't said that you can't add a file (or subdirectory) and you havent prevented someone from doing that then you should assume that they will.

Of course its unlikely that someone would have added something to a specific folder such as /components/com_users/views/registration/tmpl' but why wouldnt they have added something to a generic folder such as /libraries/cms or /libraries/joomla

Yes, I think it depends on the folder and what we allowed people do add to it in past.

E.g. if we allow site admins to add their own CLI scripts to the cli folder and with a later Joomla major version we decide not to use that folder anymore for CLI scripts but another one, and we delete that folder on update, the own scripts will be lost (if no backup).

That means we might add some kind of an exclude list to the build/deleted_file_check.php being developed with PR #25559 for this kind of folders, and someone should decide which folders these are.

No. Just check that a folder is empty before deleting it

we should never delete user files

That would mean we keep any folder if not empty, and I am not sure if that's what we want. I'd really like to know @wilsonge 's opinion on this.

I'd much rather keep a folder than face the consequences of deleting a users files. Especially as we have not specifically marked or prevented someone from adding files to a folder. It is just as reasonable for you to have added a folder libraries\mystuff as a folder libraries\joomla\mystuff. Imagine how you would feel if you updated to joomla 4 and discovered that libraries\joomla\mystuff had been deleted with no warning or notice. Even if a changed folder structure might mean the files dont work they still would not have been deleted

@brianteeman I slowly tend to agree with you, I only don't want to decide that alone. Other question is if we should fix that in J4 only or also in 3.10.

The other thing is that if we don't delete the folder, we will try it again and again with every update as long as the user files haven't been removed or moved to another place. For this we should at least leave some information like Warning: Could not delete folder "libraries/vendor/bin" because it was not empty.", or maybe not "Warning" but "Information", or so in the joomla_update.php log file so the admin has the chance to check and decide what to do with the remaining files (if reading that log, what most people don't do, I would guess).

Not sure - I am shocked to be honest that no one (including me) noticed this before

For this we should at least leave some information like Warning: Could not delete folder "libraries/vendor/bin" because it was not empty.", or maybe not "Warning" but "Information", or so in the joomla_update.php log` file so the admin has the chance to check and decide what to do with the remaining files (if reading that log, what most people don't do, I would guess).

agreed - at least that way we have given a notice in the correct place

Not sure - I am shocked to be honest that no one (including me) noticed this before

@brianteeman I've noticed that and thought because it is since such a long time like that, it might have been a deliberate decision.

Are there valid reasons to put custom files in core folders?

er... YES!

The /bin folder ! This contains files in Joomla 3, which will be deleted by Joomla on upgrade, but that folder could possibly have (and does have) other scripts in it from 3PD and so blindly deleting that folder would be wrong.

but why wouldnt they have added something to a generic folder such as /libraries/cms or /libraries/joomla

3PD do this all the time, there are MANY 3pd frameworks for Joomla 3 that are in the libraries folder, and deleting those folders would crash the site.

The "/bin" folder (not to be mixed up with "/libraries/vendor/bin" is in script.php of J4 in the list of folders to be deleted since J4 Beta 1 or earlier.

That doesn't mean it's good or bad, it only means it's nothing new.

Also doesn't mean it's right... and probably overlooked by 3pd until it happens...

if we dont remove sampledata images on update in case they are being used then its clear that not checking that a folder is empty is an oversight and not deliberate.

For now I have modified my PR #34289 so that folders "/bin" and "/libraries/vendor/bin" will not be deleted anymore on update. This should work for the mean time until we decide to go the way and do what @brianteeman suggest here.

This should work for the mean time

No it wont. Not sure why you are focussing on this bin folder. I am not

@brianteeman I am not sure if I should quickly hack a fix including the log which we talked about (and without the log I won't do it) so short before RC1 or if I should to it with more care and more time later and for the mean time at least not delete these bin folders.

Finally it should be done as discussed, because then we would not need any exception rules for particular folders in the tool used for creating the raw lists.

Any folder that is not empty should not be removed
it should be logged that a folder is not empty and has not been removed.
thats all.

I know what you want .. I am not stupid.

No offence intended - I was summarising

(not removing bin is a waste of time if you still try to remove other non-empty folders)

I am not even sure if a non-empty folder will be removed and if not what will happen. Been waiting until I got out of the studio to check that.

I see nothing in our filesystem library to check if a folder is empty so it would either need to add it there or to use glob direclty in script.php.

If someone can find something in our libraries,, please advise before I go the wrong way and implement something using glob.

just checked an update and confirming that non-empty folders are removed

Seems I should use scandir and not glob, see https://stackoverflow.com/questions/7497733/how-can-i-use-php-to-check-if-a-directory-is-empty .

just checked an update and confirming that non-empty folders are removed

@brianteeman Yes, I did the same yesterday.

if you think about it then the deleted files list could be a fraction of the size if its decided its ok to delete a non-empty folder

if you think about it then the deleted files list could be a fraction of the size if its decided its ok to delete a non-empty folder

@brianteeman Yes, I've mentioned that already here #34298 (comment) :

.. - but in this case I ask myself why we remove the files before instead of just removing the folder - ...

Another thing which makes it need a bit time is that in script.php we currently have no logging at all about file or folder deletion, so I would have to add that.

I can see some reasons to put files in core directories (besides media and libraires) but I also see good ways to not do it, anyway we have to face the situation and don't want to break sites because a 3rd dev hasn't the best idea possible. A check deleteOnlyIfItIsEmpty make sense.

Surely this is a release blocker

we have to face the situation and don't want to break sites because a 3rd dev hasn't the best idea possible. A check deleteOnlyIfItIsEmpty make sense.

We have the same issue with J3, too.

@wilsonge I'd like to know your opinion here.

Surely this is a release blocker

It's not a release blocker because it's been the case ever since joomla update was implemented a decade ago.

In terms of severity of the issue - I suspect if we were truly going to see something serious happen it would have been when we did the big namespace of all the core libraries in Joomla 3.8. So even though it's theoretical I suspect it's not super practical - as there's significantly less library directories removed in J4 compared to 3.8...

I'd much rather keep a folder than face the consequences of deleting a users files. Especially as we have not specifically marked or prevented someone from adding files to a folder. It is just as reasonable for you to have added a folder libraries\mystuff as a folder libraries\joomla\mystuff. Imagine how you would feel if you updated to joomla 4 and discovered that libraries\joomla\mystuff had been deleted with no warning or notice. Even if a changed folder structure might mean the files dont work they still would not have been deleted

I think it's also worth noting this case would still be totally broken even if you didn't delete that directory as libraries/cms and libraries/joomla are just outright no longer autoloaded anymore (as they don't exist). So OK we haven't deleted the files - but the end user would still find their site totally broken if they relied on this behaviour.

@wilsonge So we should not fix it at all? Or only in J4?

I've made a draft PR for J3 - see #35066 - and will soon make one for J4, too, then you can choose and decide.

Draft PR for J4 is #35067 .

I've closed my draft PR for staging because it was wrong and I don't expect anything to be changed in 3.10 anyway.

In 3.x it might be that the deleted files and folders lists in script.php contain only a parent folder but not its subfolders and files because it was a deliberate decision with 3.5 to recursively delete a folder's content before deleting a folder.

I think I will close my draft PR for 4.0-dev, too, because it contains some mistakes and is not complete yet, and to me it is questionable if we really should go that way.

Just imagine someone browses the Joomla root and then subfolders on Windows with the filesystem explorer or other tools. For every folder which contains an image, a thumbnail file might be created depending on the tool and its settings.

There might be other scenarios where files are created by tools, e.g. if someone uses tools for remote folder synchronization which create hidden files.

These files would block deletion of their parent folders, and these then for their parent folder and so on.

I don't really think that people will check their update log and notice that.

I've closed my draft PR for J4, too, for the reasons stated above.