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

Pass CommandEvent instead of Event #3382

Closed
trivago-makbarof opened this issue Oct 30, 2014 · 5 comments · Fixed by #3544
Closed

Pass CommandEvent instead of Event #3382

trivago-makbarof opened this issue Oct 30, 2014 · 5 comments · Fixed by #3544
Labels
Milestone

Comments

@trivago-makbarof
Copy link

Hi
We want to define a script in our composer.json called "prepare-deploy"

We wanted to call buildBootstrap inside of the script but as this script is not in set of Composer\Command\RunScriptCommand::$commandEvents you're passing Event class to that command

I propose to add a new option called as-command in RunScriptCommand so the user can force composer to pass CommandEvent to the command

https://gist.github.com/trivago-makbarof/4cc2229ef9384c33ce8b

Currently we have fixed the problem with calling our own script handler which converts Event to CommandEvent the calls the sensio script handler

Thanks

@galphanet
Copy link

I support this issue. I don't understand why it was designed this way though.

@Seldaek
Copy link
Member

Seldaek commented Dec 7, 2014

Why is probably historical reasons, but indeed it's kinda broken the way it is, and as CommandEvent extends from the base Event without adding anything they are fully compatible.

The best fix to avoid breaking BC issues with existing packages is to detect what the callback expects with reflection and if it's a CommandEvent we copy the event info to a new CommandEvent in case we don't have one yet.

@Seldaek Seldaek added the Feature label Dec 7, 2014
@Seldaek Seldaek added this to the Urgent milestone Dec 7, 2014
@galphanet
Copy link

While waiting for the fix, here is the code we use to convert the object, if this can be useful for someone :

use Composer\Script\CommandEvent;
use Composer\Script\Event;



    /**
     * Convert Event in EventCommand.
     * @param $event Event A instance
     * @return EventCommand
     */
public static function convertEventToCommandEvent(Event $event) {
        return new CommandEvent($event->getName(), $event->getComposer(), $event->getIO(), $event->isDevMode(), $event->getArguments());
    }

@alcohol
Copy link
Member

alcohol commented Dec 12, 2014

# ./composer.json
{
  "require": {
    "sensio/distribution-bundle": "@stable"
  },
  "scripts": {
    "prepare-deploy": [
      "Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::buildBootstrap"
    ]
  }
}
$ composer prepare-deploy

This is what you're trying to run, if I understand correctly?

@galphanet
Copy link

Exact, but I don't call Sensio\\Bundle\\DistributionBundle\\Composer\\ScriptHandler::buildBootstrap but a custom PHP class.

alcohol added a commit to alcohol/composer that referenced this issue Dec 12, 2014
alcohol added a commit to alcohol/composer that referenced this issue Dec 12, 2014
alcohol added a commit to alcohol/composer that referenced this issue Dec 12, 2014
alcohol added a commit to alcohol/composer that referenced this issue Dec 12, 2014
alcohol added a commit to alcohol/composer that referenced this issue Dec 12, 2014
ChadSikorra pushed a commit to ChadSikorra/composer that referenced this issue Mar 28, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging a pull request may close this issue.

5 participants
@alcohol @Seldaek @galphanet @trivago-makbarof and others