Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:35449
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: unknown (pb1.pair.com: domain php.net does not designate 85.214.94.56 as permitted sender)
Date: Wed, 13 Feb 2008 16:54:42 +0100
Reply-To: Marcus Boerger <helly@php.net>
Message-ID: <1624493738.20080213165442@marcus-boerger.de>
To: Gregory Beaver <greg@chiaraquartet.net>
CC: internals Mailing List <internals@lists.php.net>
In-Reply-To: <47AFE3B5.9070104@chiaraquartet.net>
References: <47AFE273.7030103@chiaraquartet.net> <47AFE3B5.9070104@chiaraquartet.net>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Transfer-Encoding: 7bit
Subject: Re: [PHP-DEV] Re: BC break in DirectoryIterator by fix of #44018
From: helly@php.net (Marcus Boerger)

Hello Gregory,

  It is fixed now.

marcus

Monday, February 11, 2008, 6:57:09 AM, you wrote:

> Gregory Beaver wrote:
>> Hi Marcus,
>> 
>> FYI, this change:
>> 
>> http://cvs.php.net/viewvc.cgi/php-src/ext/spl/spl_directory.c?view=diff&r1=1.146&r2=1.147
>> 
>> breaks about 20 tests in phar, it's a potentially serious BC break.  I
>> understand the reasoning behind it, but you may find other users up in
>> arms.  The main problem is that this line:
>> 
>> flags = SPL_FILE_DIR_KEY_AS_PATHNAME|SPL_FILE_DIR_CURRENT_AS_SELF;

> Important correction: the offending line is actually:

> flags = 0;

> in the previous if() {} block.

>> 
>> returns the DirectoryIterator object as current instead of an
>> SplFileInfo as it used to return.  If you instead use:
>> 
>> flags = SPL_FILE_DIR_KEY_AS_PATHNAME|SPL_FILE_DIR_CURRENT_AS_FILEINFO;
>> 
>> this will at least keep the current behavior of a SplFileInfo class
>> being returned.
>> 
>> Perhaps the better course of action would be to correct the
>> documentation rather than the behavior?  People upgrading from 5.2.5 to
>> 5.2.6 will have a nasty shock if they relied upon the default
>> constructor parameters, and even if it is reverted in 5.2.x and kept in
>> 5.3.x, the same problem holds.
>> 
>> In other words, the best fix for this is to change the default value of
>> $flags in the documentation of the constructor, not to change the
>> behavior to match faulty docs.  This is especially true since the value
>> of the constant CURRENT_AS_FILEINFO was (incorrectly) set to 0 in the
>> SPL module startup instead of the expected value - technically, the
>> documentation (which says $flags = 0) is really saying $flags =
>> CURRENT_AS_FILEINFO|KEY_AS_PATHNAME because it wasn't until this commit
>> (http://cvs.php.net/viewvc.cgi/php-src/ext/spl/spl_directory.c?r1=1.146&r2=1.147)
>> that you changed those values to non-zero.  The fact that the constant
>> was changed to the actual expected/documented value does not change the
>> fact that it's not a good reason to break BC.
>> 
>> Greg




Best regards,
 Marcus