Fix error when vendor dir contains broken symlinks #11670
Conversation
src/Composer/Util/Filesystem.php
Outdated
| @@ -251,6 +251,9 @@ public function ensureDirectoryExists(string $directory) | |||
| $directory.' exists and is not a directory.' | |||
| ); | |||
| } | |||
| if (is_link($directory)) { | |||
| @$this->unlinkImplementation($directory); | |||
There was a problem hiding this comment.
When the unlink gives something wrong you never see this, otherwise there is no reason to 'supress' the termination cause the link is always exists (is_link)?
There was a problem hiding this comment.
you are right, I added an error message when the symbolic link could not be deleted. But at the same time I'd like to make minimal changes to current logic, so I added supress on unlink
vtsykun
force-pushed
the
fix/broken-links
branch
2 times, most recently
from
d04c4e0
to
2c2d0f4
Compare
src/Composer/Util/Filesystem.php
Outdated
| $error = 'Could not delete symbolic link '.$directory.': '.(error_get_last()['message'] ?? ''); | ||
| } | ||
|
|
||
| $error = @mkdir($directory, 0777, true) ? null : ($error ?? $directory.' does not exist and could not be created: '.(error_get_last()['message'] ?? '')); |
There was a problem hiding this comment.
please use a if/else rather than a ternary here. Currently, your @ suppresses errors for the whole ternary, which is not what we want here as it could hide issues in the composer code.
src/Composer/Util/Filesystem.php
Outdated
|
|
||
| $error = null; | ||
| if (is_link($directory) && !@$this->unlinkImplementation($directory)) { | ||
| $error = 'Could not delete symbolic link '.$directory.': '.(error_get_last()['message'] ?? ''); |
There was a problem hiding this comment.
Why not throwing the RuntimeException here instead of attempting to create the directory but remembering the previous error ?
There was a problem hiding this comment.
I didn't want to add a new point where an exception could be thrown, (i.e if unlink == false OR mkdir == false). Unlink made fails on Windows by may reasons, like antivirus, FS locking, so I throw an exception like before only on mkdir = false. But I can change that if it's safe
vtsykun
force-pushed
the
fix/broken-links
branch
from
2c2d0f4
to
24978b0
Compare
|
Thanks |
Fixes #11668