search

wikimedia / composer-merge-plugin

Merge one or more additional composer.json files at Composer runtime
MIT License
923 stars 160 forks source link

Composer option `--no-scripts` is being ignored #234

Open tklie opened 1 year ago

tklie commented 1 year ago

Right now, if a project root does not already contain a vendor folder (e.g. on initial project setup), a composer install --no-scripts does not respect the --no-scripts option and instead executes scripts (e.g. post-autoload-dump scripts) after the install.

This is causing problems for us when trying to containerize Laravel applications in a CI context, where basically every build is an initial install. We use a post-autoload-dump script which triggers Laravel's package discovery. This then fails because it is trying to register services that for example require secrets only passed in at runtime.

After a quick source code dive, this seems to be due to the MergePlugin running composer update to apply merge settings without passing the disableScripts parameter (which also apparently was not available yet in composer 2.0) to the newly instantiated composer instance (i.e. factory method) when the original composer command was executed as such.

Example files

composer.json

{
    "require": {
        "php": "^8.1",
        "wikimedia/composer-merge-plugin": "^2.0"
    },
    "config": {
        "allow-plugins": {
            "wikimedia/composer-merge-plugin": true
        }
    },
    "extra": {
        "merge-plugin": {
            "require": [
                "composer-shared.json"
            ],
            "ignore-duplicates": true,
            "merge-scripts": true,
            "replace": false
        }
    },
    "scripts": {
        "post-autoload-dump": [
            "Illuminate\\Foundation\\ComposerScripts::postAutoloadDump",
            "@php artisan package:discover --ansi"
        ]
    }
}

composer-shared.json

{
    "require": {
        "guzzlehttp/guzzle": "^7.2",
        "laravel/framework": "^9.19",
        "laravel/sanctum": "^2.14.1",
        "laravel/tinker": "^2.7"
    },
    "require-dev": {
        "fakerphp/faker": "^1.9.1",
        "laravel/pint": "^1.0",
        "laravel/sail": "^1.0.1",
        "mockery/mockery": "^1.4.4",
        "nunomaduro/collision": "^6.1",
        "phpunit/phpunit": "^9.5.10",
        "spatie/laravel-ignition": "^1.0"
    }
}
bd808 commented 1 year ago

If I'm following the upstream Composer source correctly, the --no-scripts cli flag eventually turns into an EventDispatcher->setRunScripts(false) call which sets a protected $this->runScripts member variable.

There are getEventDispatcher() and setEventDispatcher() methods in PartialComposer, so we might be able to grab the EventDispatcher from $this->composer inside of our onPostInstallOrUpdate callback and pass it on to the fresh Composer instance passed to the $installer in that same method.

bd808 commented 1 year ago

I haven't done the same tracing through the Composer 1.x code base. If the same things are not implemented in 1.x this should probably be considered blocked on #243.

Mohammad-Alavi commented 6 months ago

@tklie Hey Tim, have you solved this problem in the end?

tklie commented 6 months ago

@Mohammad-Alavi I did - not to my complete satisfaction, tbh, but I needed a quick fix. I ended up forking the plugin, getting the ConsoleIO and Input options (no-plugins and no-scripts) from the event using reflection, and then passing them to the factory method that creates the new composer instance.