Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

composer remove should remove symlink to source #10023

Closed
kdambekalns opened this issue Jul 27, 2021 · 2 comments
Closed

composer remove should remove symlink to source #10023

kdambekalns opened this issue Jul 27, 2021 · 2 comments
Labels
Milestone

Comments

@kdambekalns
Copy link
Contributor

Reporting this after researching neos/flow-development-collection#2135

My composer.json:

{
    "name": "neos/flow-development-distribution",
    "description": "Flow Development Distribution",
    "license": "MIT",
    "config": {
        "vendor-dir": "Packages/Libraries",
        "bin-dir": "bin"
    },
    "require": {
        "neos/flow-development-collection": "dev-master"
    },
    "repositories": {
        "distributionPackages": {
            "type": "path",
            "url": "./DistributionPackages/*"
        }
    },
    "scripts": {
        "post-update-cmd": "Neos\\Flow\\Composer\\InstallerScripts::postUpdateAndInstall",
        "post-install-cmd": "Neos\\Flow\\Composer\\InstallerScripts::postUpdateAndInstall",
        "post-package-update": "Neos\\Flow\\Composer\\InstallerScripts::postPackageUpdateAndInstall",
        "post-package-install": "Neos\\Flow\\Composer\\InstallerScripts::postPackageUpdateAndInstall",
    }
}

Output of composer diagnose:

Checking composer.json: OK
Checking platform settings: OK
Checking git settings: OK
Checking http connectivity to packagist: OK
Checking https connectivity to packagist: OK
Checking github.com oauth access: OK
Checking disk free space: OK
Checking pubkeys:
Tags Public Key Fingerprint: 57815BA2 7E54DC31 7ECC7CC5 573090D0  87719BA6 8F3BB723 4E5D42D0 84A14642
Dev Public Key Fingerprint: 4AC45767 E5EC2265 2F0C1167 CBBB8A2B  0C708369 153E328C AD90147D AFE50952
OK
Checking composer version: You are not running the latest stable version, run `composer self-update` to update (2.1.3 => 2.1.5)
Composer version: 2.1.3
PHP version: 7.4.21
PHP binary path: /usr/local/Cellar/php@7.4/7.4.21_1/bin/php
OpenSSL version: OpenSSL 1.1.1k  25 Mar 2021
cURL version: 7.77.0 libz 1.2.11 ssl (SecureTransport) OpenSSL/1.1.1k
zip: extension present, unzip present

When I run the remove command on a package coming from the path repository:

$ ls -l Packages/Application
lrwxr-xr-x  1 karsten  staff   49 Jul 27 12:03 Flownative.DoubleOptIn -> ../../DistributionPackages/Flownative.DoubleOptIn
drwxr-xr-x+ 9 karsten  staff  288 Jul 27 11:51 Neos.Behat

$ composer2 remove flownative/flow-doubleoptin

I get the following output:

…
Package operations: 0 installs, 0 updates, 1 removal
  - Removing flownative/flow-doubleoptin (dev-master), source is still present in Packages/Application/Flownative.DoubleOptIn
…

$ ls -l Packages/Application
lrwxr-xr-x  1 karsten  staff   49 Jul 27 12:03 Flownative.DoubleOptIn -> ../../DistributionPackages/Flownative.DoubleOptIn
drwxr-xr-x+ 9 karsten  staff  288 Jul 27 11:51 Neos.Behat

And I expected this to happen:

The symlink is removed, the code in ../DistributionPackages/Flownative.DoubleOptIn is left in place.

The reason for this behaviour is

if (realpath($path) === realpath($package->getDistUrl())) {
if ($output) {
$this->io->writeError(" - " . UninstallOperation::format($package).", source is still present in $path");
}
return \React\Promise\resolve();
}
– but I don't understand the reasoning behind it, TBH.

@Seldaek Seldaek added the Bug label Jul 28, 2021
@Seldaek Seldaek added this to the 2.1 milestone Jul 28, 2021
@Seldaek
Copy link
Member

Seldaek commented Aug 17, 2021

Yeah it doesn't seem really sensible. It was a fix for another issue done in #9116 - and it makes sense in that case perhaps, but we need to tweak this for sure to support both cases gracefully. I'll investigate some more.

@Seldaek
Copy link
Member

Seldaek commented Aug 18, 2021

Ok I think the fix works for all use cases now. /cc @ryanaslett I tried your repro case from #9116 but if you can also have a look that'd be good :)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants