Deletion of individual files on uninstall is broken

[oliver-graetz]

While doing some tests on installing and uninstalling plugins I notices that plugins aren't cleanly uninstalled. Upon further investigation it is clear that commit https://github.com/magento-hackathon/magento-composer-installer/commit/e96796e62f5c0d8436d038ab785b3456c07d1fab is the cause. There is even a comment pointing out the problem.

The \Composer\Util\Filesystem->removeDirectory function will exclusively delete directories but not files. The commented out implementation does delete individual files in its else branch.

This not only leads to unclean removal of plugins, it also brings up "already exists" warnings when trying to reinstall later followed by forced manual cleanup.

[oliver-graetz]

Why is there no progress on this bug? Fixing it is a simple rollback since it was working before.

[Flyingmana]

Its not a "simple rollback", maybe it is, but before I do this I would need to test a lot, as deleting not enough is one side, deleting to much is a real problem.

You say it worked ok before, but I will be the one who gets blamed, if it does not work after the rollback.

If you want to speed this up, we still have no unittests for this. Also finding some people who test and support this change would be helpfull.

To speak for me: My priority lies in making things work without relaying on copy or symlink methods.

[oliver-graetz]

Well, it is not a simple rollback (although that would also work), but the reason for why it is currently not working is so simple that no unit test is needed to see what is wrong. The commented out code contains this:

    if (is_dir($dir) && ! is_link($dir)) {
        // ...
    } else { $result = @unlink($dir); }
    return $result;

So it contains code for removing single files. This code is needed because the code comment for the function states it Recursively removes the specified directory or file. But the code call to the composer API that replaces the commented out code starts with:

    if (!is_dir($directory)) { return true; }

So the code change is a simple regression that changes the code to explicitly not do what it is supposed to do: delete single files.

[oliver-graetz]

And to add a question: What do you mean by "without relaying on copy or symlink methods"? In the milestone discussion I read about leaving the files in the vendors directory. Is a new method that would enable this the ultimate goal regarding deployment methods?