"Invalid installation files detected" message may be confusing

It was intentionally implemented like this, admins must learn to upgrade sites properly, that is during each major upgrade to move away all existing files and copy back config.php + new versions of contrib plugins, or better use git. This was a source of many problems before...

Your patch is wrong because the list is NOT complete, you can not delete only those listed files! You need to upgrade properly, sorry. More explanation in the linked issue.

Petr

> Your patch is wrong because ...

No, my patch is perfectly correct. It works exactly as stated. It reports any missing stale files that were detected. I know for sure because I tested it.

As it turned out, the message was caused by my logging in as Moodle admin when the CVS update was only half complete on the server. I was upgrading the HEAD branch. How was I to know it was going from 2.3 to 2.4? Once the CVS upgrade was complete, the message went away.

The message without a list of stale files is not helpful. It is mysterious and frustrating. I'm going to keep my friendly and informative patch on my site, and you can keep on explaining to people why it is better for the software to be mysterious and frustrating.

Gordon

No, your patch is incorrect, it completely misses the purpose of this check.

My patch is correct, because it fixes the issue that I had, which is that I wanted to know what files were missing.

I understand that you have your reasons why you don't want to accept this suggestion. Too bad.

Hmm, you still do not understand what the $someexamplesofremovedfiles means, it is a tiny subset of files that were present in previous installs but were removed in later versions. You solve absolutely nothing if you remove the listed files from borked installs, you only hide existing problems - that is definitely not a fix.

FYI, I understand perfectly what $someexamplesofremovedfiles means, and what the purpose of the check is.

What you have failed to understand is the effect of the message on the unfortunate Moodle admin who sees it.

The message says:

Some old PHP scripts have been detected ... Please fix the installation directory by removing all old scripts

This message is mysterious and confusing. It tells the admin to remove some files but it doesn't tell me what the files are. Don't tell me again that the admin doesn't need to know the list of files that Moodle found. I understand that point. I am talking here about the effect of the message on the admin who reads it for the first time.

Through my interaction with you on this issue I have learned that this message actually means, "Always install a new Moodle release into a new directory". Removing files should not be mentioned in the message, or if we do mention removing files, we should explain what files were found and what that means.

I can see you adamantly believe that "you solve absolutely nothing if you remove the listed files", in which case I am sure you would agree with me that the message should be changed to emphasize installing into a clean directory rather than "removing old scripts".

For reference, here is the complete heading and message regarding stale scripts:

Invalid installation files detected, upgrade cannot continue

Some old PHP scripts have been detected which may indicate that you installed this version over an older one. Please fix the installation directory by removing all old scripts (except config.php) before installing the new version and then try the upgrade again. You can find more information in upgrade documentation at

Hello Gordon,
You are not reading the warning correctly you say "it tells the admin to remove some files" this is not true, the exact words are "removing ALL (note the word all please !) old scripts (except config.php)". Even if english is not my native language, for me (and for nearly all other Moodle users I guess !) it means exactly "Always install a new Moodle release into a new directory" with just a config.php script in it !
Don't you feel like wasting Petr's time on this issue ?

Let me give you a few examples why we have this ugly warning (again this list is not complete):

1/ the admin and course reports were moved to new location in 2.2, if admin just unzips the directory over old 2.1 install they get the same files twice which creates fatal errors in some areas, the $someexamplesofremovedfiles does NOT contain all of these directory, just one of each of them

2/ CVS creates empty dirs during update if I remember it correctly, this breaks enumeration of plugins in some places - again we have detection of 3 dirs on in $someexamplesofremovedfiles

3/ global search was deleted completely - if you keep the files you get fatal errors

As I said in the other issue the list of all files that should not be there any more is really huge, but the problem there is that we do not know what is supposed to be there. Solution would be to whitelist instead of blacklist all files that should be there, but to do that we would need to manage the installed plugins through moodle and most probably keep a copy (or at least a file list) of everything that was installed. That is a huge task, but hopefully somebody will work on that soon.

I did not write this message alone, I asked other developers for help because English is not my native language and sometimes what I write/say does not make much sense (I wish it always did in Czech at least). So please if you think the message is confusing (I think it may be) please propose a better message that tell admins: "Stop unzipping new PHP files over old install when upgrading to next major version (it is ok for minor releases though) - you must remove ALL previous files except config.php first. This warning may be caused by incomplete CVS update too."

Thank you Petr. I appreciate your taking the time to consider this issue with me, and for suggesting a revision to the strings which appear when mixed Moodle versions are detected.

Below is the my proposal for the revised "upgradestalefiles" and "upgradestalefilesinfo" strings in the "lang/en/admin.php" lang file.

I am not sure who to present these revised strings to for consideration, and I would be grateful for any advice you can give.

regards
Gordon

Mixed Moodle versions detected, upgrade cannot continue

The Moodle update process has been paused because PHP scripts from at least two major versions of Moodle have been detected in the Moodle directory.

This can cause significant problems later, so in order to continue you must ensure that the Moodle directory contains only files for a single version of Moodle.

The recommended way to clean your Moodle directory is as follows:

• rename the current Moodle directory to "moodle_old"
• create a new Moodle directory containing only files from either a standard Moodle package download, or from the Moodle CVS or GIT repositories
• move the original config.php file and any non-standard plugins from the "moodle_old" directory to the new Moodle directory

When you have a clean Moodle directory, refresh this page to resume the Moodle update process.

This warning is often caused by unzipping a standard Moodle package over a previous version of Moodle. While this is OK for minor upgrades, it is strongly discouraged for major Moodle upgrades.

This warning can also be caused by an incomplete checkout or update operation from a CVS or GIT repository, in which case you may just have to wait for the operation complete, or perhaps run the appropriate clean up command and retry the operation.

reopening and changing title to reflect our conclusion that the text is confusing, thanks for the proposal

I am going to bring this forward on Monday during HQ meeting.

+1 for the new issue title, and thanks for including the new strings in the discussions at HQ. Hope that goes well !

Hi,

I just ran into this. It would be helpful if the installer could tell the admin what the files it detected were. I am a Moodle developer and had to do the following to work otu what was going on:

diff --git a/lib/upgradelib.php b/lib/upgradelib.php
index aa6ee8f..c12256d 100644
--- a/lib/upgradelib.php
+++ b/lib/upgradelib.php
@@ -338,6 +338,7 @@ function upgrade_stale_php_files_present() {
 
     foreach ($someexamplesofremovedfiles as $file) {
         if (file_exists($CFG->dirroot.$file)) {
+            error_log($file);
             return true;
         }
     }

Dan, again, listing of the files solves nothing, quite the opposite, you need to start from fresh checkout.

For the record, I agree with Petr that we shouldn't be displaying these files, but the message is very similar to other 'out of date plugins' type errors we've seen in the past.

Ah, right, I think you found the reason why it is confusing.

Dan, could you shed any light on how your Moodle directory came to contain files from multiple Moodle versions? Would that be covered by either (1) unzipping a Moodle zip file over a previous version of Moodle, or (2) an incomplete CVS update, or (3) something else?

In my case it was a bad git state, very visible if you do git status (hadn't run git clean -fd after cancelling a git checkout command).

> a bad git state

I see. In which case, what do you think of the modifying the last part of the suggested rewording of the message (given earlier in this thread) to the following:

This warning can also be caused by an incomplete checkout or update operation from a CVS, SVN or GIT repository, in which case you may just have to wait for the operation complete, or perhaps run the appropriate clean up command and retry the operation.

I have modified the proposed revision to the "upgradestalefilesinfo" message to include references to the GIT repository situation that Dan came across.

Yep, makes sense.

To integrators: I have used markdown to make this consistent with help strings, feel free to improve it.

GOrdon: thanks a lot for the patch and especially the proposed solution!

Great news - and thanks for making the solution a reality Petr!

Thanks Petr/Gordon. I've integrated this now.

Message is more explanatory now.

Thanks for fixing this, Petr

Congratulations!

You've made it into the weekly release!

Thanks for your contribution - here are some random drummers to keep you inspired for the next week!
http://www.youtube.com/watch?v=_QhpHUmVCmY