Increasing priority as it is broken functionality and can affect multiple modules.

https://github.com/rajeshtaneja/moodle/blob/6efbcd9257b34c1cc51c5856fd7ba43f806f590f/lib/moodlelib.php#L2074
getdate array should match with one produced by $datestring = gmstrftime('%B_%A_%j_%Y_%m_%w_%d_%H_%M_%S', $time);
%m returns two digit representation of the month, whereas getdate returns numeric representation of a month.

In theory it should not break anything, but still think it should go in master only and then back-ported later to stable branches.

Hi Raj,

I agree about the change because usergetdate should not return different formatted results if called without a timezone. (Which it does by calling getdate which returns month without the leading 0).

A really trivial comment about thing about the commit message ( http://docs.moodle.org/dev/Commit_cheat_sheet ) - could you shortern the first line and include mention of the function name. It helps in viewing the logs to have that shortened and including the function name just helps when viewing history to know exactly where it affected

Thanks Dan,
Commit message is shortened now. Pushing it for integration review.

Hi Raj - you didn't need to shortern the commit message, just first line

I really encourage you to write detailed messages in the next lines!

Hi Raj,
this looks good, it seems other calls won't be affected or will cast into integer.
i was wondering why not strip for %d too ( Two-digit day of the month (with leading zeros) ) as the same index error could be avoided elsewhere not to mention the format is kept same as getdate().

$datestring = gmstrftime('%B_%A_%j_%Y_%-m_%w_%-d_%H_%M_%S', $time);

?

Thanks Aparup,

You are right this should have been checked for all the values. Following is fixed now:

1. All int values are stripped with leading zero.
2. Values are typecasted properly, before they return
3. %j returns 001 through 366, whereas getdate() returns 0 through 365. This has been taken care off.

I have added 21 and 22 branches as well, and it should go on stable as well.

All looks good here, but this is slightly more scary change now.

Perhaps a deep look of uses is required.

reopening for more work (going on at the moment too)

I think this is good to go.
Dan can you please review this again for me.

Thanks

Looks good to me

Thanks Dan,
Pushing it for integration.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Branches re-based.

Hi Rajesh,

I was moving your changes in (not existing anymore) lib/simpletest/testmoodlelib.php to the new lib/tests/moodlelib_test.php and executing it I'm getting one failure:

phpunit lib/tests/moodlelib_test.php

Time: 3 seconds, Memory: 71.00Mb

There was 1 failure:

1) moodlelib_testcase::test_usergetdate
Failed asserting that 7 matches expected 0.

integration/lib/tests/moodlelib_test.php:1016
integration/lib/phpunit/lib.php:1162

So it seems that the code is introducing some problem with the seconds... so I'm reopening this.

Ciao

Hi Rajesh, it seems that we cannot rely in the "-" modifier at all. I asked @ HQ for help running this:

php -r 'error_reporting(0); date_default_timezone_set("UTC"); foreach (str_sp
lit("BAjYmwdHMS" ) as $m) { echo "$m: " . gmstrftime("%$m", 7) . "#" . gmstrfti
me("%-$m", 7) . "#" .  strftime("%$m", 7) . "#" .  strftime("%-$m", 7) . "#" .
PHP_EOL; }'

And got all sort of results, in summary:

• Linux seemed to accept it ok.
• MacOS X was working ok for Eric but not for me (I was getting "-S" on output)
• Windows directly "eat" any modifier with "-" (and requires setting error reporting to 0.
• Solaris returns "-S" too.

So I think we must do that without those "-".

Here it's a copy of the discussion @ HQ. Thanks everybody:

[15:48:36] <Cindereloy> Offtopic. Can anybody execute this under linux and windows:

php -r 'date_default_timezone_set("UTC"); foreach (str_split("BAjYmwdHMS" ) as $m) { echo "$m: " . gmstrftime("%$m", 7) . "#" . gmstrftime("%-$m", 7) . "#" .  strftime("%$m", 7) . "#" .  strftime("%-$m", 7) . "#" . PHP_EOL; }'

and paste results?
[15:48:49] <Cindereloy> (it's not a virus)
[15:49:28] <Eric Merrill> B: January#January#January#January#
A: Thursday#Thursday#Thursday#Thursday#
j: 001#1#001#1#
Y: 1970#1970#1970#1970#
m: 01#1#01#1#
w: 4#4#4#4#
d: 01#1#01#1#
H: 00#0#00#0#
M: 00#0#00#0#
S: 07#7#07#7#
[15:49:37] <Cindereloy> that is which OS?
[15:49:44] <Eric Merrill> RHL 5 I think
[15:49:52] <timhunt> repeated many thousands of times.
[15:50:15] <Eric Merrill> Identical on mac os x too
[15:50:18] <timhunt> How can I kill the warnings and notices?
[15:50:22] <Cindereloy> identical on mac?
[15:50:24] <david> 
B: January#January#January#January#
A: Thursday#Thursday#Thursday#Thursday#
j: 001#1#001#1#
Y: 1970#1970#1970#1970#
m: 01#1#01#1#
w: 4#4#4#4#
d: 01#1#01#1#
H: 00#0#00#0#
M: 00#0#00#0#
S: 07#7#07#7#

[15:50:34] <Cindereloy> grrr, here i get:

B: January#-B#January#-B#
A: Thursday#-A#Thursday#-A#
j: 001#-j#001#-j#
Y: 1970#-Y#1970#-Y#
m: 01#-m#01#-m#
w: 4#-w#4#-w#
d: 01#-d#01#-d#
H: 00#-H#00#-H#
M: 00#-M#00#-M#
S: 07#-S#07#-S#

[15:50:37] <Eric Merrill> Red Hat and Mac seem to give identical results
[15:51:08] <timhunt> $ php -r 'error_reporting(0); date_default_timezone_set("UTC"); foreach (str_sp
lit("BAjYmwdHMS" ) as $m) { echo "$m: " . gmstrftime("%$m", 7) . "#" . gmstrfti
me("%-$m", 7) . "#" .  strftime("%$m", 7) . "#" .  strftime("%-$m", 7) . "#" .
PHP_EOL; }'
B: January##January##
A: Thursday##Thursday##
j: 001##001##
Y: 1970##1970##
m: 01##01##
w: 4##4##
d: 01##01##
H: 00##00##
M: 00##00##
S: 07##07##
[15:51:25] <Cindereloy> buf, that's worse. It "eats" them.
[15:51:26] <timhunt> I added error_reporting 0
[15:51:48] <Cindereloy> oki, thanks. I think the conclusion is that we cannot rely on the "-" modifier.
[15:51:52] <Andrew Nicols> Do you want it on Solaris too?
[15:51:58] <Cindereloy> yes thanks
[15:52:03] <Andrew Nicols> 504 nicols@current10x:~> /opt/csw/php5/bin/php -r 'date_default_timezone_set("UTC"); foreach (str_split("BAjYmwdHMS" ) as $m) { echo "$m: " . gmstrftime("%$m", 7) . "#" . gmstrftime("%-$m", 7) . "#" .  strftime("%$m", 7) . "#" .  strftime("%-$m", 7) . "#" . PHP_EOL
; }'
B: January#%-B#January#%-B#
A: Thursday#%-A#Thursday#%-A#
j: 001#%-j#001#%-j#
Y: 1970#%-Y#1970#%-Y#
m: 01#%-m#01#%-m#
w: 4#%-w#4#%-w#
d: 01#%-d#01#%-d#
H: 00#%-H#00#%-H#
M: 00#%-M#00#%-M#
S: 07#%-S#07#%-S#

[15:52:09] <Cindereloy> aha
[15:52:28] <Cindereloy> oki, many thanks. 

Thanks for pointing this Eloy
I have updated all branches.

The main moodle.git repository has just been updated with latest weekly modifications. You may wish to rebase your PULL branches to simplify history and avoid any possible merge conflicts. This would also make integrator's life easier next week.

TIA and ciao

Branches re-based

Integrated, thanks! (21, 22 & master).

Note: I've added one extra commit on each branch to change current assertEqual() tests to assert[Identical|Same]() (simpletest|phpunit), so both value and type will be tested.

all good raj, thanks for the clear test instructions

UPDATE tracker_issues
   SET status = 'Closed',
      comment = 'Thanks!'
WHEN participants = 'Did a gorgeous work'

This landed upstream some hours ago (some - me - developer fell slept in the sofa yesterday before spamming this).