Skip to content
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

Complete fix for #77 (timezone warning) #297

Merged
merged 1 commit into from Sep 9, 2015

Conversation

michaelhogg
Copy link
Contributor

The #77 bug involves this warning being raised when PHPMD is executed:

Warning: date(): It is not safe to rely on the system's timezone settings. You are required to use the date.timezone setting or the date_default_timezone_set() function.

In v2.0.0 and earlier, the warning was caused by calling date() without first setting the timezone, in this line in XMLRenderer::renderReport():

$writer->write('timestamp="' . date('c') . '">');

93b5835 attempted to fix the bug by adding this code to src/bin/phpmd:

if (!ini_get('date.timezone') && !date_default_timezone_get()) {
    date_default_timezone_set('UTC');
}

⚠️ Although this fixed the warning from the date() call in XMLRenderer::renderReport(), it introduced a new bug. The new call to date_default_timezone_get() raises this warning:

Warning: date_default_timezone_get(): It is not safe to rely on the system's timezone settings. You are required to use the date.timezone setting or the date_default_timezone_set() function.

See @hbandura's comment on #77.

From the docs for date_default_timezone_get():

In order of preference, this function returns the default timezone by:

  • Reading the timezone set using the date_default_timezone_set() function (if any)
  • Reading the value of the date.timezone ini option (if set)

There's no need to call date_default_timezone_get(), because:

  • We know date_default_timezone_set() hasn't been called. (Look at the top of src/bin/phpmd. Only Composer's autoload.php is executed, and date_default_timezone_set() is not called there.)
  • We've just tested the value of the date.timezone ini option by calling ini_get('date.timezone').

So the call to date_default_timezone_get() can safely be removed, and this will completely fix the issue of the timezone warning.

@Arcanemagus
Copy link
Contributor

Any chance you can look this over @ravage84?

@ravage84 ravage84 added the Bug label Sep 9, 2015
@ravage84 ravage84 added this to the 2.3.0 milestone Sep 9, 2015
ravage84 added a commit that referenced this pull request Sep 9, 2015
@ravage84 ravage84 merged commit 9e6b691 into phpmd:master Sep 9, 2015
@ravage84
Copy link
Member

ravage84 commented Sep 9, 2015

@Arcanemagus & @michaelhogg It would be great if you could verify that the dev-master of phpmd now works for you. Thanks

@manuelpichler
Copy link
Contributor

+1

@Arcanemagus
Copy link
Contributor

The changes in this PR fix the messages about the timezone, the rest of my phpmd installation isn't working at the moment though and I have yet to trace that down. (Installed globally via composer on Win 8.1 x64, and I could swear it was working at one point so I'm not sure when it broke.)

@Arcanemagus
Copy link
Contributor

Well, after a reboot it's working perfectly again, go figure.

With the changes from this PR I'm no longer getting error messages about the timezone.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Development

Successfully merging this pull request may close these issues.

None yet

4 participants