composer install gives error (could not delete [symlinked directory])

[tkdb]

I'm running into an issue which prevents composer install to work properly in my setup when a dependency changed.

The dependency is FireGento/Logger and I needed to fork it in order to get my changes in (from 1.3.0 to some dev-version).

After updating composer.json + .lock reflecting the new dependencies, running composer install on linux yieds RuntimeExceptions with message

Could not delete /path/to/magento/app/code/community/FireGento/Logger/:

(nothing follows the colon). This is for directories that are symlinks.

command was (exemplary with dry-run):

$ composer install --dry-run --no-dev
 Loading composer repositories with package information
 Installing dependencies from lock file
   - Updating firegento/logger (1.3.0) to firegento/logger (dev-magento-ce1.5 4bade2d)

composer stops after the first exception. Then I can delete that directory manually like so:

$ unlink ./magento/app/code/community/FireGento/Logger

Afterwards it won't start at this file but continues to throw the next exception just at the next symlinked directory.

This is only for the package changed. So the problem is, I can't just unlink all symlinks. I wish either composer would realize what to do on these (if that is sane, needs feedback upstream I think) or - far better - the magento composer install would realize what is going on there and takes care then (the symlinks were created by magento composer installer).

Any ideas?

[tkdb]

I now updated from only little changes with my forked extension, and I didn't see this error again. Perhaps because no directory has changed.

However composer thinks I would have 20 modified files which just is not the case. Those are marked as D which I assume is deleted

$ composer update firegento/logger
Loading composer repositories with package information
Updating dependencies (including require-dev)
  - Updating firegento/logger dev-magento-ce1.5 (4bade2d => 578b676)
    The package has modified files:
    D src/app/code/community/FireGento/Logger/Block/Adminhtml/LiveView.php
    D src/app/code/community/FireGento/Logger/Block/Adminhtml/Logger.php
    D src/app/code/community/FireGento/Logger/Block/Adminhtml/Logger/Grid.php
    D src/app/code/community/FireGento/Logger/Block/Adminhtml/System/Config/Renderer/Select.php
    D src/app/code/community/FireGento/Logger/Block/Adminhtml/System/Config/Targetmap.php
    D src/app/code/community/FireGento/Logger/Formatter/Advanced.php
    D src/app/code/community/FireGento/Logger/Helper/Data.php
    D src/app/code/community/FireGento/Logger/Model/Chromelogger.php
    D src/app/code/community/FireGento/Logger/Model/Chromelogger/Library/ChromePhp.php
    D src/app/code/community/FireGento/Logger/Model/Db.php
-10 more files modified, choose "v" to view the full list
    Discard changes [y,n,v,s,?]? y
    Checking out 578b676e5810ae610cd4ecedaa68031e350b4364

Writing lock file
Generating autoload files

This is perhaps that composer doesn't understand linking here as well? Just thinking loud.

[tkdb]

Well that was on Windows, on Linux this fails again with both composer install or update.

On my end I now work-around by fully re-installing:

$  find -L magento -xtype l -print -prune -exec unlink {} \; && rm -rf vendor && composer install --no-dev

magento is the magento root dir here.

[Flyingmana]

which deploy method do you use? symlink or link?

[tkdb]

Now you ask me something. I don't have the magento-deploystrategy entry in the extra section. So I guess it is "none" but magento composer install creates symlinks.

Additionally it's good you ask becasue I didn't know there is a difference between symlink or link.

Anything I put in there for a test?

[tkdb]

This is my composer.json for completeness:

$ cat composer.json
{
    "require": {
        ...
        "magento-hackathon/magento-composer-installer": "*",
        ...
        "firegento/logger": "dev-magento-ce1.5"
    },
    "repositories": [
        {
            "type": "composer",
            "url": "http://packages.firegento.com"
        },
        {
            "type": "vcs",
            "url": "https://github.com/tkdb/firegento-logger"
        },
        ...
    ],
    "extra": {
        "magento-root-dir": "magento/",
        "auto-append-gitignore": true
    }
}

[Flyingmana]

default is symlink, so it should really be symlink. the difference is mostly on fs level, as symlinks and links are kind of different(I never use link)

And thanks for the composer.json, I will try if I can reproduce this this week

[tkdb]

if link is hardlink then for sure I'm using symlinks. Those are on disk (I double-checked that via find).

I've seen in review of #121 that recursive directory deletion is using realpath somewhere which does resolve symlinks. It's perhaps not what we want.

There is also a flag for SPL's FilesystemIterator to follow or not follow Symlinks. If you like, we could review #121 first as this is perhaps related and if I saw your commit there right, there actually was an error with it, so perhaps there are more errors ;).

[Flyingmana]

I would suggest to open a new Issue for the reviewing, so we dont mix issues up and because I expect to have this specific reported problem fixed, but refactoring and reviewing things is always ok :)

[tkdb]

Sure! That was just a suggestion.

[hakre]

The underlying issue is with composer not handling pathnames properly if those are with trailing slashes. I've taken an extensive look into this (and also found another flaw in composers filesystem handling on the route) and filed two PRs with Composer (I've used the data and code from here to do the report):

• https://github.com/composer/composer/pull/3159
• https://github.com/composer/composer/pull/3158

If you don't want to / can't wait for upstream changes you could try to remove trailing slashes in the modman file (untested).

[Flyingmana]

thank you very much for you research and your work here :)

[tkdb]

Wow :+1: - I removed the trailing slashes on those four paths in the modman file and it looks like that it did work, too. It's late, but if I'm not totally off, it seems that with the problem in composer, composer also removes the directories within the vendor dir (instead of unlinking) and therefore the package was marked as changed.

[Flyingmana]

Is it possible to reproduce this with a test? Would like to add a workaround for now

[tkdb]

Because of b8ccfdfd81439f41396ec8d0b28d7f43e7d6a25e there is no need for a test as you've removed the flaw to test for already. So the workaround is already in - isn't it?

[tkdb]

btw, I'm just seeing that removing the slashes in the modman file has influence on the condition in

https://github.com/magento-hackathon/magento-composer-installer/blob/master/src/MagentoHackathon/Composer/Magento/Deploystrategy/DeploystrategyAbstract.php#L318

[tkdb]

I started to put the unrelated changes into https://github.com/tkdb/magento-composer-installer/commits/develop - There is a tons of whatever TODO in that codebase it seems. Just seeing even whitespace is messed on empty lines. You want this cleaned up?

Also the testcase(s) are broken. I have made one fix but I'm unsure.

It's perhaps worth you cherry-pick into master as a stable branch from a develop branch or some other sort of workflow. Just let me know what you would prefer. In the current state I find it rahter cumbersome to find the place where to test and while trying out I'm running over diverse code issues. So if you can give a general direction would be helpful. I don't want to patch around with not knowing what's accepted in this project and what not.

Still trying for a test, thought ;)

[tkdb]

And from my end it seems okay to hotfix the error condition as you did with rtrim there, as it should do the job. I perhaps would leave some reference in the code and also make the methods more speaking but that's perhaps overhead in your eyes, so just an example - https://github.com/tkdb/magento-composer-installer/commit/c41f888aa469b39432b1d98dc752d3fd8fccc8da

[tkdb]

This has been fixed upstream in composer. I filed a new pull-request to take back the now superfluous changes from July. Happy merging.

As this was not an issue with magento-composer-installer I close it as "Not A Bug" ;)