[Filesystem] improved errors for invalid dump targets #24134
Conversation
xabbuh
commented
Sep 8, 2017
| Q | A |
|---|---|
| Branch? | 2.7 |
| Bug fix? | yes |
| New feature? | no |
| BC breaks? | no |
| Deprecations? | no |
| Tests pass? | yes |
| Fixed tickets | #24097 |
| License | MIT |
| Doc PR |
| } | ||
|
|
||
| if (is_link($dir)) { | ||
| throw new IOException(sprintf('The parent path of "%s" is a symlink to a not existing directory.', $dir)); |
There was a problem hiding this comment.
Would nonexistent (dir) sound better than not existing (dir)?
0337f1f to
c6ccc3f
Compare
|
@xabbuh This pull request works great, and without error when applied to |
| */ | ||
| public function testDumpFailsIfTargetIsSymlinkAndSymlinkTargetDoesNotExist() | ||
| { | ||
| $this->markAsSkippedIfSymlinkIsMissing(); |
There was a problem hiding this comment.
We need to check why this does not apply on Appveyor or if is_link() works differently on Windows than on Unix. If someone using Windows can help here, that would be really appreciated.
| @@ -525,13 +525,21 @@ public function isAbsolutePath($file) | |||
| * @param null|int $mode The file mode (octal). If null, file permissions are not modified | |||
| * Deprecated since version 2.3.12, to be removed in 3.0. | |||
| * | |||
| * @throws IOException If the file cannot be written to. | |||
| * @throws IOException if the file cannot be written to | |||
There was a problem hiding this comment.
I think we disabled the fabbot rule triggering this, can you check?
There was a problem hiding this comment.
No, I've fixed everything in the codebase... using php-cs-fixer.
|
Failure on Windows looks related. Status: needs work |
|
@xabbuh Can you have a look at the tests? |
|
@xabbuh Can you try to finish this one? |
|
soft ping @xabbuh |
|
I have no idea how to make the test pass on Windows and I don't have access to a Windows machine to debug this properly. I am closing here for now. Anyone wanting to dig into the issue can reuse the changes here (#24134 (comment) is the part that still needs to be fixed). I will gladly help with any questions regard the current state of the changes. |