Suppress warnings on date_default_timezone_get() #121

Closed
wants to merge 1 commit into
from

Conversation

Projects
None yet
7 participants

Unfortunately date_default_timezone_get() still triggers the classic timezone warning, and there is no other way to determine if a timezone has been set.

While one can ensure their projects always perform a date_default_timezone_set() there are still common places where monolog can be imported without direct user control over setting default timezones.

In my particular case, including monolog in a Symfony2 composer project with the stock Symfony2 post-install-cmd and post-update-cmd chain causes failures at the end of an install/update cycle. I believe it's composer in this case that translates all triggered warnings/errors into exceptions which causes whatever process that is running to fail.

Therefore I propose we just suppress the warnings from date_default_timezone_set() for now.

In the future timezone management should maybe be mildly refactored to be injectable and default to UTC, but this will eliminate

Contributor

stof commented Sep 27, 2012

I would suggest configuring PHP properly instead by putting a date.timezone in your php.ini

Btw, I'm wondering why it is not specified in the default php.ini file of distributions as PHP requires this setting

True, and I did this after trying other more reasonable options, however I worry about shared-hosts and what not.

On a side, more personal note, I'm frustrated date_default_timezone_get() throws the warning in the first place.

Owner

Seldaek commented Sep 27, 2012

@dcousineau it's normal that it throws a warning since it does not have a default timezone set, the function can not properly return anything. It can guess from your machine's environment, but it's apparently not reliable enough to make it the default.

Right, I just have a fantasy where there's at least a flag one could pass to indicate "Please don't guess, give me the default or false if it wasn't set, I know what I'm doing I promise".

I am facing this problem too.

My date.timezone is set to "Europe/Madrid" but it stills throws the warning, which makes composer to stop working...

Contributor

stof commented Oct 14, 2012

@marcosgdf are you sure that it is set in the right php.ini ? Take care that the CLI often uses a different php.ini

@stof Yep, I'm completely sure because I modified the memory limit for composer to work

Owner

Seldaek commented Nov 11, 2012

Sorry but this is not gonna happen, you can either configure it in php.ini, or set it per-script with date_default_timezone_set(), so there is really no good reason for us to suppress the error, if you're seeing it most likely you'll have the wrong timezone (especially since 5.4 does not set anything by default anymore and just uses UTC if you don't set it).

@Seldaek Seldaek closed this Nov 11, 2012

It is happening. It is configured in php.ini with a correct timezone, but the warning is always showing because of relying in date_default_timezone_get system setting. The only way to solve this is by adding @ or by setting the timezone manually in the script, but that is annoying as every time we update dependencies, we have to remove our changes and apply them after the update.

Contributor

pborreli commented Nov 11, 2012

@marcosgdf can you run in cli : php -i |grep date.timezone

Owner

Seldaek commented Nov 11, 2012

@marcosgdf you must have set it wrong somehow, or you found a php bug, but I'm pretty sure that it's working for a ton of other people as it is. Maybe your apache uses another php.ini than the cli one, I can't say without knowing your setup, but I'm 99% sure you can fix it by setting it in the right php.ini.

If php.ini really is no option, then set it in the script, "in the script" doesn't mean in monolog, you can set it in your front controller for example. You shouldn't need to touch vendor files for this.

@pborreli date.timezone => Europe/Madrid => Europe/Madrid
@Seldaek but the problem is that because of Monolog, composer stops working when installing or updating as it detects a php warning in post-install-cmd and post-update-cmd as @dcousineau says. That's why I modify it directly in vendor files.

Owner

Seldaek commented Nov 11, 2012

The error in post-install-cmd is triggered by one of your scripts doing something with monolog, and your php being misconfigured at that point, I don't know why since your php -i seems to return the correct value.

@Seldaek It's actually caused by Symfony or Monolog. Here's an actual log:

Marcos: marcos$ php composer.phar update
Loading composer repositories with package information
Updating dependencies
  - Installing jdorn/sql-formatter (v1.0.1)
    Downloading: 100%         

  - Updating doctrine/doctrine-bundle (v1.0.0 => dev-master 4fa47ca)
    Checking out 4fa47caf19c426d607a636b2da73bc0e5a87ec66

  - Updating doctrine/orm 2.3.x-dev (6bad01 => c6eb04)
    Checking out c6eb04de145b986d968be1ddb0e0a43cd8110b8e

  - Updating sensio/distribution-bundle 2.1.x-dev (v2.1.1 => v2.1.3)
    Checking out v2.1.3

  - Updating sensio/generator-bundle 2.1.x-dev (3a65c9 => 56c202)
    Checking out 56c2029d1f3f41b39dd0df40009cb448df010bf1

  - Updating psliwa/php-pdf dev-master (8270f3 => ff356e)
    Checking out ff356ef951e9b641da2b82d4e109a7f243dcb811

  - Updating twig/twig dev-master (61393d => b16765)
    Checking out b16765ce8f8edd227c6c90dba489326001fd348a

  - Updating doctrine/dbal 2.3.x-dev (9395ca => 97801a)
    Checking out 97801a74ba646d069c6674439ed74e75b34e80ca

  - Updating kriswallsmith/assetic dev-master (a1b6fa => d62030)
    Checking out d6203098559cf9fc90e2890c10533d0adad33bd0

  - Updating symfony/assetic-bundle dev-master (4eea85 => 30d384)
    Checking out 30d3841dc9643d352503a72f3cdf93d193c439d2

  - Updating monolog/monolog dev-master (38bb47 => bcb51e)
    The package has modified files:
    M src/Monolog/Logger.php
    Discard changes [y,n,v,s,?]? y
    y - discard changes and apply the update
    n - abort the update and let you manually clean things up
    v - view modified files
    s - stash changes and try to reapply them after the update
    ? - print help
    Discard changes [y,n,v,s,?]? y
    Checking out bcb51e01406965ebd030d4b99eb199a9226c3fef

  - Updating doctrine/data-fixtures dev-master (181b71 => a95d78)
    Checking out a95d7839a7794c7c9b22d64e859ee70658d977fe

  - Updating sensio/framework-extra-bundle 2.1.x-dev (6a63ea => v2.1.3)
    Checking out v2.1.3

Writing lock file
Generating autoload files



  [ErrorException]                                                                                                                                                                                                                                                                                                                                                                                                                                                                  
  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. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected 'Europe/Berlin' for 'CET/1.0/no DST' instead in /xx/vendor/monolog/monolog/src/Monolog/Logger.php line 112  



Script Sensio\Bundle\DistributionBundle\Composer\ScriptHandler::clearCache handling the post-update-cmd event terminated with an exception



  [RuntimeException]                                                         
  An error occurred when executing the "'cache:clear --no-warmup'" command.  



update [--prefer-source] [--prefer-dist] [--dry-run] [--dev] [--no-custom-installers] [--no-scripts] [-v|--verbose] [-o|--optimize-autoloader] [packages1] ... [packagesN]


Marcos:x marcos$ ```
Owner

Seldaek commented Nov 11, 2012

What if you do:

php -r 'error_reporting(-1);var_dump(date_default_timezone_get());'
marcos$ php -r 'var_dump(date_default_timezone_get());'
string(13) "Europe/Madrid"
marcos$ 
Owner

Seldaek commented Nov 11, 2012

So is something else setting the timezone to an invalid value between composer & monolog?

Your doing a date_default_timezone_get() and a failover to a default of UTC() if the previous call returns a falsey value. The problem is date_default_timezone_get() does not return a falsey value without also triggering a PHP warning, which in systems like Symfony2's console component are turned into uncaught exceptions.

Frankly the timezone for Monolog should not be hidden behind a protected static set in the constructor, a fully formed DateTimeZone object should be passed into the constructor to allow for proper DI and if said object is not passed in, a default of new \DateTimeZone('UTC'); would be used. This would completely avoid relying on date_default_timezone_get() and any potential issues with any user's PHP configuration.

I will provide another pull request later, I'm fine with leaving this closed.

If I insert var_dump(date_default_timezone_get()); die; in line 112 of Monolog/Logger.php, I get:

marcos$ php composer.phar update
Loading composer repositories with package information
The contents of http://packagist.org/p/providers-latest.json do not match its signature, this is most likely due to a temporary glitch but could indicate a man-in-the-middle attack. Try running composer again and if please report it if it persists.
The contents of https://packagist.org/p/providers-latest.json do not match its signature, this is most likely due to a temporary glitch but could indicate a man-in-the-middle attack. Try running composer again and if please report it if it persists.
Updating dependencies
Generating autoload files
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    
  [ErrorException]                                                                                                                                                                                                                                                                                                                                                                                                                                                                  
  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. In case you used any of those methods and you are still getting this warning, you most likely misspelled the timezone identifier. We selected 'Europe/Berlin' for 'CET/1.0/no DST' instead in /Users/marcos/dev/rentgest/vendor/monolog/monolog/src/Monolog/Logger.php line 112  
                                                                                                                                                                                                                                                                                                                                                                                                                                                                                    
Script Sensio\Bundle\DistributionBundle\Composer\ScriptHandler::clearCache handling the post-update-cmd event terminated with an exception
                                                                             
  [RuntimeException]                                                         
  An error occurred when executing the "'cache:clear --no-warmup'" command.  
                                                                             
update [--prefer-source] [--prefer-dist] [--dry-run] [--dev] [--no-custom-installers] [--no-scripts] [-v|--verbose] [-o|--optimize-autoloader] [packages1] ... [packagesN]
marcos$ 

What I understand of this error is that date_default_timezone_get() should not be used before date_default_timezone_set() as it is not safe to rely on php.ini setting...

Also using new \DateTimeZone(date_default_timezone_get()); is somewhat redundant: the constructor for the DateTime object already utilizes the default date time zone settings and will raise a warning if you try to create or get a new DateTime object when no default timezone is set.

Owner

Seldaek commented Nov 11, 2012

I'm ok with injecting it, but if it's not injected IMO it should read from date_default_timezone_get (if only for BC's sake).

@marcosgdf it's fine to use date_default_timezone_get() if the timezone is set properly in php.ini. The thing is, it seems to be set ok in yours, but not in the symfony script handler. That handler though calls php ... - so it creates a subprocess, and maybe that one does not have a proper timezone defined for whatever reason due to environment settings?

Owner

Seldaek commented Nov 11, 2012

@dcousineau not true AFAIK, new DateTimeZone needs an argument, so we need to call date_default_timezone_get(), and we need the timezone instance because when creating a DateTime from a unix timestamp it's always set to the UTC timezone.

Apparently at some point you set a timezone, but that is not Europe/Madrid as in your php-cli output, but CET/1.0/no DST, which is not a valid definition and php falls back to Europe/Berlin.
You should check your system for more php.ini files and check if one of them contains this complex and invalid timezone definition,

Timezone is correctly set as in my comment when I did php -r 'var_dump(date_default_timezone_get());'. It runs OK in cli and in cgi mode. Also, if I directly do php app/console cache:clear --no-warmup it doesn't throw any error, but if this command is run by composer, the error is thrown.

Maybe this is a problem that should be reported in composer...

OK. I found the problem. @Seldaek was right, it was an environment settings problem... I set an alias for 'php' in my bash settings but it wasn't a global alias so...

sh-3.2# php -i |grep date.timezone
date.timezone => no value => no value
sh-3.2# 

I'm so sorry... Didn't really think about it... I apologize for any inconvenience

Owner

Seldaek commented Nov 13, 2012

Ah OK. I was busy writing the following which would have helped you debug it. Maybe the PhpExecutableFinder should try to find which php is currently executed in priority (it does it with PHP 5.4 that has a new PHP_BINARY constant, but not in 5.3 because that's a bit harder).


Could it be that you have more than one php on your machine?

  • open in vendor/sensio/distribution-bundle/Sensio/Bundle/DistributionBundle/Composer/ScriptHandler.php
  • at line 175, add var_dump($phpPath);
  • run composer install to trigger the code and see if you see anything dumped in the output
Owner

Seldaek commented Nov 13, 2012

Ok no after a quick check this seems impossible (guessing the path..). The best is really to use PATH to make sure php is found instead of aliases, that way symfony can find it too.

Contributor

pborreli commented Nov 13, 2012

@marcosgdf glad you found out !

moabidi commented Sep 15, 2013

If you run the script from Ligne comande interface (CLI) you must active the directive date.timezone in the file /etc/php5/cli/php.ini
If you run the script from navigator you must active the directive date.timezone in the file /etc/php5/apache/php.ini

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