Skip to content

Conversation

@othercorey
Copy link
Member

@othercorey othercorey commented Dec 29, 2025

closes #19142

@othercorey othercorey added this to the 5.2.11 milestone Dec 29, 2025
Copy link
Member

@markstory markstory left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good, but the test may need adjusting for windows.

@othercorey othercorey force-pushed the remove-symlink branch 3 times, most recently from cffcf10 to b770ddc Compare December 29, 2025 12:02
@othercorey
Copy link
Member Author

it seems file_exists returns true for a symlink on windows when the target doesn't exist even though the documentation says otherwise. We'd need a more elaborate check.

@LordSimal
Copy link
Contributor

Thats a windows specific issue as a broken symlink is still a "file" for windows.
You'd need to read that file and make sure, it actually exist like so:

if (is_link($path)) {
    $target = readlink($path);

    if ($target === false || !file_exists($target)) {
        echo "Broken symlink\n";
    }
}

@othercorey
Copy link
Member Author

Are you sure readlink returns false on windows?

@LordSimal
Copy link
Contributor

Its not primarily about readlink returning false, but it returns the path the the actually symlinked file. So in your case !file_exists($target) would then be true.

$target === false is just a precaution to make sure a path is actually returned.

@othercorey
Copy link
Member Author

I plan to add a more complete check that the symlink points to the right target.

@othercorey
Copy link
Member Author

@markstory @LordSimal Did a more general check and logic.

rmdir($fakeTarget);

$this->assertFileDoesNotExist($fakeTarget);
$this->assertTrue(is_link($path));
Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Only check the link still exists as file_exists behaves differently on linux and windows for a disconnected link.

@markstory markstory merged commit 36cae30 into 5.x Jan 9, 2026
14 of 15 checks passed
@markstory markstory deleted the remove-symlink branch January 9, 2026 04:11
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

PluginAssetsTrait::_remove() does not remove symlink if it points to a non-existing path

4 participants