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 2.3.2 crash on vendor-expose or any recipe-plugin commands (e.g. update-recipe) #18

Closed
tim-mediasuite opened this issue Mar 31, 2022 · 4 comments · Fixed by #19

Comments

@tim-mediasuite
Copy link

Composer 2.3.2 was released yesterday. Ever since composer vendor-expose fails:

> composer vendor-expose
PHP Fatal error:  Declaration of Composer\Command\BaseCommand::getComposer(bool $required = true, ?bool $disablePlugins = NULL, ?bool $disableScripts = NULL) must be compatible with SilverStripe\RecipePlugin\RecipeCommandBehaviour::getComposer($required = true, $disablePlugins = NULL) in /home/tim/Sites/scratch-silverstripe/vendor/silverstripe/recipe-plugin/src/RequireRecipeCommand.php on line 73

Fatal error: Declaration of Composer\Command\BaseCommand::getComposer(bool $required = true, ?bool $disablePlugins = NULL, ?bool $disableScripts = NULL) must be compatible with SilverStripe\RecipePlugin\RecipeCommandBehaviour::getComposer($required = true, $disablePlugins = NULL) in /home/tim/Sites/scratch-silverstripe/vendor/silverstripe/recipe-plugin/src/RequireRecipeCommand.php on line 73

Downgrading to Composer 2.2.10 works. This is with vendor-plugin 1.5.2

This is on a scratch SS site with a small composer.json:


{
    "name": "silverstripe/installer",
    "type": "silverstripe-recipe",
    "description": "The SilverStripe Framework Installer",
    "require": {
        "php": "^7.3 || ^8.0",
        "silverstripe/recipe-plugin": "^1.2",
        "silverstripe/recipe-cms": "~4.10.0@stable",
        "silverstripe-themes/simple": "~3.2.0",
        "silverstripe/login-forms": "~4.6.0@stable"
    },
    "require-dev": {
        "phpunit/phpunit": "^9.5",
        "silverstripe/behat-extension": "^4.8"
    },
    "extra": {
        "resources-dir": "_resources",
        "project-files-installed": [
            "app/.htaccess",
            "app/_config.php",
            "app/_config/mimevalidator.yml",
            "app/_config/mysite.yml",
            "app/src/Page.php",
            "app/src/PageController.php"
        ],
        "public-files-installed": [
            ".htaccess",
            "index.php",
            "web.config"
        ]
    },
    "config": {
        "process-timeout": 600,
        "allow-plugins": {
            "composer/installers": true,
            "silverstripe/recipe-plugin": true,
            "silverstripe/vendor-plugin": true
        }
    },
    "prefer-stable": true,
    "minimum-stability": "dev"
}

@tim-mediasuite
Copy link
Author

So it appears composer tightened the method signature for getComposer to add types

From the composer release notes here: https://github.com/composer/composer/releases/tag/2.3.0

BC Break: added native parameter & return types to many internal APIs, we explicitly left the most extended/implemented symbols untouched but if this causes problems nonetheless please report it ASAP (https://github.com/composer/composer/pull/10547, https://github.com/composer/composer/pull/10561)

@GuySartorelli
Copy link
Member

Thanks for raising this @tim-mediasuite.
After some investigation the problem is actually in silverstripe/recipe-plugin, so I'm moving the issue there. The reason you're seeing it when running composer vendor-expose is because the autoloader is also resulting in recipe-plugin being parsed.

This was specifically caused by the addition of typehints to the $required and $disabledPlugins parameters in Composer\Command\BaseCommand::getComposer() in composer/composer#10561. I've raised that in composer/composer#10679 since their release notes say to report any breakages caused by those changes.

SilverStripe\RecipePlugin\RecipeCommandBehaviour uses the SilverStripe\RecipePlugin\RecipeCommandBehaviour trait, which declares getComposer() as an abstract method.

public abstract function getComposer($required = true, $disablePlugins = null);

It would be interesting to know the intention of this, and whether that abstract method declaration is still required - we can have discussions and investigation into that pending composer's response to composer/composer#10679.

@GuySartorelli GuySartorelli transferred this issue from silverstripe/vendor-plugin Mar 31, 2022
@GuySartorelli GuySartorelli changed the title Composer 2.3.2 crash on vendor-expose Composer 2.3.2 crash on vendor-expose or any recipe-plugin commands (e.g. update-recipe) Mar 31, 2022
@Seldaek
Copy link
Contributor

Seldaek commented Mar 31, 2022

IMO the fix would be #19 - these methods are not needed as far as I can tell as they are always present on Composer\Command\BaseCommand which the trait-using classes are extending from.

@Seldaek
Copy link
Contributor

Seldaek commented Mar 31, 2022

Please try it in a real project though, I did not.

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

Successfully merging a pull request may close this issue.

3 participants