Re: [PATCH] date/timelib: use system timezone database

On Wed, 9 Jan 2008, Joe Orton wrote: > It's a bit of a maintenance headache for distributions when > packages include their own copy of the timezone database, since this > needs to be updated frequently. There is a PECL extension to provide those updates: http://pecl.php.net/package/timezonedb It is a drop-in replacement to update the rules. > I've prepared a patch to allow the timelib code to use the system > timezone database directly; would something like this be acceptable, any > thoughts? Quite a few actually. > Notes: > > 1) I've not implemented timelib_timezone_builtin_identifiers_list() here > since it doesn't seem to be used, is it there for third-party > extensions? It could be implemented by iterating through the directory. It's not used (anymore). However, there is a PHP function timezone_identifiers_list that uses the index information to enumerate all entries. Your patch prevents this function from working as both struct elements index_size and index are set to 0/null. > 2) there's no general way that I can find to obtain the database > version, so I just invented a string here. Not having the version creates another problem, as the PECL timezonedb extension will then *always* override the builtin database. The extension uses the version number to see if the extension provides a later rules set. With your patch, this extra check will not work correctly anymore if PHP's built-in version is newer (which would happen if PHP got upgraded). Some other issues: - This patchs allows accessing any file on the filesystem (because the path is not sanitized). - Opening a file is less quick than having the data in memory. - The system's format of tzfiles does not necessarily have to be the same as the one that PHP reads. On my (Debian) system the tz provided data is already 64bit read. Once I update the extension to use this extra data, reading the system tz files won't work anymore. - It would be possible to use a "wrong set" of tzdata, resulting in bugs like http://bugs.php.net/bug.php?id=35958 The reason to bundle the database is to have a operating system independent source of tzdata, which is something that this patch destroys. I am because of this, and all the other issues, quite against adding this patch - especially because there is already a solution for this. regards, Derick -- Derick Rethans http://derickrethans.nl | http://ezcomponents.org | http://xdebug.org