Re: BC break in DirectoryIterator by fix of #44018

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