[Filesystem] silence fileperms() issues on remote filesystems #23340
Conversation
xabbuh
commented
Jun 30, 2017
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #23312 |
| License | MIT |
| Doc PR |
| @@ -69,7 +69,9 @@ public function copy($originFile, $targetFile, $overwriteNewerFiles = false) | |||
| } | |||
|
|
|||
| // Like `cp`, preserve executable permission bits | |||
| @chmod($targetFile, fileperms($targetFile) | (fileperms($originFile) & 0111)); | |||
| if (stream_is_local($originFile)) { | |||
There was a problem hiding this comment.
I just wonder if there are other resource types we need to consider too
There was a problem hiding this comment.
I was reading about the stream_is_local() function and I saw this user comment: http://php.net/manual/en/function.stream-is-local.php#121126 Should we take that into account?
There was a problem hiding this comment.
TL;DR @javiereguiluz's point: stream_is_local() return false for file:// paths, but chmod() works on them. So yes, we should have a case for them.
| @@ -69,7 +69,7 @@ public function copy($originFile, $targetFile, $overwriteNewerFiles = false) | |||
| } | |||
|
|
|||
| // Like `cp`, preserve executable permission bits | |||
| @chmod($targetFile, fileperms($targetFile) | (fileperms($originFile) & 0111)); | |||
| @chmod($targetFile, fileperms($targetFile) | (@fileperms($originFile) & 0111)); | |||
There was a problem hiding this comment.
shouldn't the first "fileperms" also be silenced?
would that work?
@(chmod($targetFile, fileperms($targetFile) | (fileperms($originFile) & 0111)));
There was a problem hiding this comment.
Shouldn't we always be able to get the perms of the newly written file?
There was a problem hiding this comment.
can't the new file be remote, thus have the same issue?
|
wait, the original issue is buggy! things are already silenced, that's a "SilencedErrorContext" |