Newsgroups: php.internals
Path: news.php.net
Xref: news.php.net php.internals:35352
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: <47AFE273.7030103@chiaraquartet.net>
Date: Sun, 10 Feb 2008 23:51:47 -0600
User-Agent: Thunderbird 2.0.0.6 (X11/20071022)
MIME-Version: 1.0
To: Marcus Boerger <mail@marcus-boerger.de>, 
 internals Mailing List <internals@lists.php.net>
Content-Type: text/plain; charset=ISO-8859-1
Content-Transfer-Encoding: 7bit
Subject: BC break in DirectoryIterator by fix of #44018
From: greg@chiaraquartet.net (Gregory Beaver)

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;

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