Details
-
Type:
Bug
-
Status: Closed
-
Priority:
Trivial
-
Resolution: Fixed
-
Affects Version/s: 2.3.4, 2.4.1, 2.5
-
Fix Version/s: 2.5
-
Component/s: Administration
-
Labels:
-
Testing Instructions:
-
Affected Branches:MOODLE_23_STABLE, MOODLE_24_STABLE, MOODLE_25_STABLE
-
Fixed Branches:MOODLE_25_STABLE
-
Pull from Repository:
-
Pull Master Branch:wip-mdl-38382
-
Pull Master Diff URL:
Description
Whilst testing MDL-38378 I cherry picked version version 2.3.4+ (Build: 20130228) (2012062504.10) to test the changes I made within '/lib/db/update.php' as the script has the condition:
if ($oldversion < 2012062504.11 && $oldversion >= 2012062504.08) {
|
So I thought a test value of 2012062504.10 would satisfy this criteria. However, the value after the decimal point appears to be truncated to 2012062504.1. And Moodle reports:
Upgrading Moodle database from version 2.3.4+ (Build: 20130228) (2012062504.1) to 2.3.4+ (Build: 20130307) (2012062504.12)
Note the disparity between the build date and reported Moodle version when compared with the original version.php file. Thus causing confusion on the part of the administrator / developer when using the value.
Gliffy Diagrams
Issue Links
- Testing discovered
-
MDL-38378
DB Upgrade script contains hard coded database prefix.
-
- Closed
-
Activity
Dear Petr Skoda,
That does not account for the condition code in MDL-38378 as it is:
$oldversion < 2012062504.11
|
and the message upon upgrade of:
Upgrading Moodle database from version 2.3.4+ (Build: 20130228) (2012062504.1) to 2.3.4+ (Build: 20130307) (2012062504.12)
As the value of $oldversion is almost certainly being truncated.
Cheers,
Gareth
I'm going to repeat the test to check.
Hi Gareth,
first of all many thanks for your promptly action with MDL-38378. I read that query so many times that cannot believe how I skipped the prefix use.
About this... I don't get the point... why do you say that $oldversion is being truncated? They are floats so, 2012062504.1 and 2012062504.10 are "equivalent", the former is not any truncation of the later.
Hence, any of them is smaller than 2012062504.11, and the condition works. Or am I missing anything?
Perhaps it would be better to show those versions with the trailing zeros, agree, but that is only cosmetic, it has no incidence in the evaluation afaik.
Ciao 
Dear Eloy Lafuente (stronk7),
No worries
I'm just double checking my test procedure in detecting this as real thing or just a cosmetic.
Cheers,
Gareth
Dear Eloy Lafuente (stronk7),
In double checking (as you do), the script is running and hence 2012062504.10 is contained in $oldversion. It's the 'echo' of the value caused confusion. Therefore downgraded this to a 'trivial' as only cosmetic but could cause mild heart palpitations for admins.
Going to reword the issue.
Cheers,
Gareth
Aha, okey, agree, thanks!
Looks like fix for this is to use the 'p()' function and not what is being used at the moment.
Thanks for reporting this Gareth,
I have put this on backlog.
In the meantime feel free to help us work on this issue. If you are able to provide a patch or links to your Git repository branch, please add a patch label so we will spot it.
+1 for he patch (master only)
Thanks for the review Petr,
Pushing it for integration.
Integrated (master only), thanks!
Test passed. Thanks!
This issue has been integrated upstream and is now available both via git and cvs (and in some hours, via mirrors and downloads).
Thanks!
PS: Yay, legacy template messages. Yes, you're ok, we don't have CVS anymore!
The version numbers are floats, you should not use "=" comparison for floats.