Support custom SplFileInfo objects in Phar::buildFromIterator() #14143
Conversation
|
Any progress planned for this here actually? :-) |
|
This needs a strong rebase. But this should work. |
ndossche
force-pushed
the
phar-fileinfo
branch
from
112ba55 to
61e8342
Compare
|
This is now rebased, with extra tests, and with some cleanups+fixes. This is only a part of the reproducibility journey. It allows to override the builtin classes' behaviour, which you could argue is a bugfix. |
ndossche
force-pushed
the
phar-fileinfo
branch
2 times, most recently
from
8752a73 to
ac8f782
Compare
While it is possible to return a custom SplFileInfo object in the iterator used by buildFromIterator(), the data is not actually used from that object, instead the data from the underlying internal structure is used. This makes it impossible to override some metadata such as the path name and modification time. The main motivation comes from two reasons: - Consistency. We expect our custom methods to be called when having a custom object. - Support reproducibility. This is the original use case as requested in [1]. Add support for this by calling the getMTime() and getPathname() methods if they're overriden by a user class. [1] theseer/Autoload#114.
ndossche
force-pushed
the
phar-fileinfo
branch
from
ac8f782 to
06fe327
Compare
|
I believe this is ready for review. After this is merged I'll work on additional methods on ext/phar for setting timestamps etc directly. |
| zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0, "Could not resolve file path"); | ||
| return ZEND_HASH_APPLY_STOP; | ||
| if (UNEXPECTED(Z_TYPE(rv) != IS_STRING)) { | ||
| zend_throw_exception_ex(spl_ce_UnexpectedValueException, 0, "getPathname() must return a string"); |
There was a problem hiding this comment.
This probably should be a type error, but I think there are other such problems in ext/phar so I'm happy to defer this to a latter PR.
|
|
||
| fname_len = strlen(fname); | ||
| save = fname; | ||
| goto phar_spl_fileinfo; |
There was a problem hiding this comment.
I guess a follow-up refactoring is to get rid of this goto and assign the fname var in the corresponding switch case statement. As the GitHub UI didn't show me the label iniitally...
3 participants
While it is possible to return a custom SplFileInfo object in the
iterator used by buildFromIterator(), the data is not actually used from
that object, instead the data from the underlying internal structure is
used. This makes it impossible to override some metadata such as the
path name and modification time.
The main motivation comes from two reasons:
custom object.
[1].
Add support for this by calling the getMTime() and getPathname() methods
if they're overriden by a user class.
[1] theseer/Autoload#114.