[FileSystem] Windows fix #16987
[FileSystem] Windows fix #16987
Conversation
41a16e2 to
413294e
Compare
| @@ -100,6 +100,10 @@ public function mkdir($dirs, $mode = 0777) | |||
| public function exists($files) | |||
| { | |||
| foreach ($this->toIterator($files) as $file) { | |||
| if ('\\' === DIRECTORY_SEPARATOR AND strlen((string) $file) > 258) { | |||
There was a problem hiding this comment.
YOu should use && instead of AND (same below)
There was a problem hiding this comment.
Also, why is the cast to string required?
|
Status: Needs Work |
ed1ae0d to
4d78bbc
Compare
* Fixes edge case on windows where PHP does not generate a PHP Warning but instead returns a wrong result https://bugs.php.net/bug.php?id=71103 * Improved error reporting on `unlink` used in `remove()`
| @@ -157,7 +161,8 @@ public function remove($files) | |||
| } | |||
| } else { | |||
| if (true !== @unlink($file)) { | |||
| throw new IOException(sprintf('Failed to remove file %s', $file)); | |||
| $error = error_get_last(); | |||
| throw new IOException(sprintf('Failed to remove file "%s": %s.', $file, $error['message'])); | |||
There was a problem hiding this comment.
Usually, this kind of changes are considered as new features, to me moved to a PR on the master branch IMO
There was a problem hiding this comment.
not really, we do improve exception messages in patch releases.
|
These changes are not correct as some functions work with length 259 while others don't. See https://bugs.php.net/bug.php?id=70943 |
|
@Tobion this PR only addresses the |
|
What's the status of this PR? |
|
ready to merge |
|
@flip111 |
|
Thank you @flip111. |
This PR was merged into the 2.3 branch. Discussion ---------- [FileSystem] Windows fix | Q | A | ------------- | --- | Bug fix? | yes | New feature? | no | BC breaks? | no | Deprecations? | no | Tests pass? | yes | Fixed tickets | #16783 | License | MIT | Doc PR | * Fixes edge case on windows where PHP does not generate a PHP Warning but instead returns a wrong result https://bugs.php.net/bug.php?id=71103 * Improved error reporting on `unlink` used in `remove()` Commits ------- 0d5f7e2 Update FileSystem
unlinkused inremove()