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

Add Before/After Trigger on install/update command #218

Closed
francoispluchino opened this issue Jan 18, 2012 · 25 comments
Closed

Add Before/After Trigger on install/update command #218

francoispluchino opened this issue Jan 18, 2012 · 25 comments

Comments

@francoispluchino
Copy link
Contributor

To execute a command or script before and/or after the vendors installation.
This command would be in the composer.json:

...

"trigger": {
    "after": [
        "php ./my_folder/my_custom_script.php"
    ]
}

...

We could also define a command for each type of OS:

...

"trigger": {
    "after": [
        "php ./my_folder/my_custom_script.php myarg1 myarg2" : ["windows"],
        "./my_folder/my_custom_script.php myarg1 myarg2" : ["linux", "mac"],
    ]
}

...

In this way, it would be possible to initialize a complete project

@francoispluchino
Copy link
Contributor Author

@Seldaek Are you interested to have this behavior?
I can code immediately.

@Seldaek
Copy link
Member

Seldaek commented Jan 18, 2012

In general terms, yes. I'm not sure about details yet, naming and stuff mostly, but that should be fairly easy to change afterwards.

I would say off the top of my head this looks a bit easier or more natural:

"scripts_before": []
"scripts_after": []

Then as you suggested with the OS + a "*" that sounds fine to me.

/cc @naderman?

@naderman
Copy link
Member

I'm not sure I like this at all. I guess as long as it is limited to the main composer.json I can live with it. In general I think we should keep in mind that it should be possible to use composer as a library from a web process without any shell access at all. But can't this be done using custom installers?

@Seldaek
Copy link
Member

Seldaek commented Jan 18, 2012

Maybe yes. I'm not really sure what the potential use cases are. Maybe we should wait and think about it. There are enough issues to look at if you're bored @francoispluchino :)

@francoispluchino
Copy link
Contributor Author

I'm not bored, far from this, but I developed for my company a symfony2 project generator with an automatic registering vendors in the Autoload, Kernel (bundle only) and Routing (bundle only).

I found your project after and I wanted to add some of my work with your project.

@naderman I am OK with you for keeping the composer as library. I already use composer as library for my Symfony2 Bundles. I create a container service for install/uninstall/update library and bundle.

I saw that there was a setting "bin" in the composer.json, we could have the same logic, and use the Triger before / after composer.json only for the file type "project".

Or if you prefer, create a "init" command (or other name) run the command registered in composer.json.
This way we can initiate a project with the composer.phar and composer.json.

The second solution may be better. What do you think?

@Seldaek
Copy link
Member

Seldaek commented Jan 18, 2012

That sounds better, it would potentially allow a symfony distribution (or a cms or whatever) to be just a composer.json I guess, the creation of the app skeleton would be delegated to one of the installed packages then.

@francoispluchino
Copy link
Contributor Author

That's exactly right.

I begin the development.

@Seldaek
Copy link
Member

Seldaek commented Jan 18, 2012

I think you don't really need a custom command. You can just have a custom installer for your project + your project package that has a special type and requires the installer. Then the project package can have stuff in its "extra" key that the intsaller reads and execute.

Technically it's not an issue there are many ways to do it, I think what we need first is to define if we want this in core or not, and if yes how we do it so it's nice and can't be abused too much.

Maybe we could ship that custom installer in composer, that way packages of type "application" or something get a chance to run some init commands, but not the others, and we could warn the user (either on install or at least on packagist) that those packages will execute stuff.

@francoispluchino
Copy link
Contributor Author

Ok, so if you're interested, I can try to develop it.

@Seldaek
Copy link
Member

Seldaek commented Jan 18, 2012

So we talked about it with @naderman and I guess the only thing we agree on is that it is useful for the main package, but not the installed ones. So much like repositories are already only enabled for the main/root package, those script_before, after_scripts, or whatever name, should be only supported there, and they should be executed after every install/update I guess.

Another way could be to simply have Class::method or such as a "script", so we would just call that method. The benefit would be that we can pass it some php parameters easily (packages just installed for example). Thoughts?

@francoispluchino
Copy link
Contributor Author

I was thinking like your first thought, but the second one, I didn't catch all you've said. Did you think about a way to declare the class and the method to execute?

@Seldaek
Copy link
Member

Seldaek commented Jan 19, 2012

Yeah I meant that instead of using scripts we could say:

"before_scripts": [
    "Foo\\Bar::method",
    "My_Class::foo",
]

Then those would be executed, and they could do their own php/os/whatever checks and then run scripts if necessary and so on.

@francoispluchino
Copy link
Contributor Author

I like it just fine.

Would you separate the install and update methods?
Or do you prefer to send a custom event or an array to the static method.
The idea is to have a generic enough one to later add an uninstall command for example.

I send to the method the action type ('install', 'update)', the event type ('before', 'after'), and the Package class for get most information.

On the other hand, to load these classes, we should perhaps indicate to the composer.json the folder where they are.

I think adding a section to define the 'root' of the folders containing the classes, the rest of the path being made by FQCN.

"dir_scripts": [
    "my/custom/dir/in/my/project",
    "folder/of/my/package",
]

What do you think?

@Seldaek
Copy link
Member

Seldaek commented Jan 19, 2012

IMO it's ok to just force them to be autoloadable with PSR-0 for now.

@francoispluchino
Copy link
Contributor Author

@Seldaek When you use this class Composer\Installer\InstallerInstaller?.

@Seldaek
Copy link
Member

Seldaek commented Jan 19, 2012

I'm not sure what you are trying to say.

@francoispluchino
Copy link
Contributor Author

I am looking where to place the event dispatcher, but I think it will be in the install method of the InstallCommand command.

For the PSR-0, Do I instantiate a new ClassLoader or is it possible to get one?

@Seldaek
Copy link
Member

Seldaek commented Jan 19, 2012

You can just require the one that was dumped in the installcommand I think, and then the class should be available. And yes you can start the whole process from InstallCommand. BTW I think #207 should be done before you can require the generated autoload. If you do it please do it as a separate PR though, otherwise I try to look at it myself later.

@francoispluchino
Copy link
Contributor Author

About the PR #207, I don't think I'm enough gifted to do it myself. So, I'll wait for it to be resolved to progress on this issue.

@Seldaek
Copy link
Member

Seldaek commented Jan 22, 2012

207 has been fixed.

@francoispluchino
Copy link
Contributor Author

I'm starting to look more closely how, and instead of re-developing a event system, to add a dependency Symfony\EventDispatcher, it is very light and well done. In addition, it is part of the Symfony Framework component, that is already using Composer.

For the classes declarations, I suggest to specify the classes of the package used by the system event in the composer.json package. The methods are run before or after all of the action, but in order to install/update the packages. Classes declared in the root composer.json will be executed after the others (for the before and the after separately).

We could add an option --no-event to update and install commands to avoid starting the implementation of these classes.

As you point out above, it would be interesting that the site packagist.org indicates that the package uses W and X classes for events before and Y and Z for the events after.

What do you think?

@Seldaek
Copy link
Member

Seldaek commented Jan 23, 2012

I'm not sure there is much point in having a full blown event dispatcher here. I mean we just have a list of callbacks we have to call.

@francoispluchino
Copy link
Contributor Author

Ok, I'll make a classes loader of events by trying to use the AutoloadGenerator. This one will be added in the Install and Update command.

However, does the idea to indicate these classes directly both in the package's composer.json and in the composer root suit you?

@Seldaek
Copy link
Member

Seldaek commented Jan 23, 2012

I'm not sure I got your last sentence. As for the autoloadgenerator as I said earlier, I think you can just require the vendor/.composer/autoload.php file that was generated after installation. It should enable you to autoload all classes of all packages (granted they are PSR-0)

@Seldaek
Copy link
Member

Seldaek commented Feb 5, 2012

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

No branches or pull requests

3 participants