Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:35353
Mailing-List: contact internals-help@lists.php.net; run by ezmlm
Received-SPF: error (pb1.pair.com: domain chiaraquartet.net from 38.99.98.18 cause and error)
Message-ID: <47AFE3B5.9070104@chiaraquartet.net>
Date: Sun, 10 Feb 2008 23:57:09 -0600
User-Agent: Thunderbird 2.0.0.6 (X11/20071022)
MIME-Version: 1.0
CC: Marcus Boerger <mail@marcus-boerger.de>, 
 internals Mailing List <internals@lists.php.net>
References: <47AFE273.7030103@chiaraquartet.net>
In-Reply-To: <47AFE273.7030103@chiaraquartet.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: Re: BC break in DirectoryIterator by fix of #44018
From: greg@chiaraquartet.net (Gregory Beaver)

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