Proposal: introduce composer_manager_build_complete(), which fires after a composer_manager build is complete.

Use case:

  • User enables module A, which has a library dependency managed by composer_manager
  • Module A needs to fire off a few functions in hook_install(), but it cannot because dependencies are not yet build when that hook in invoked
  • Instead, Module A implements composer_manager_build_complete(). which is fired after dependencies are built. Necessary install functions are then called within that implementation.
CommentFileSizeAuthor
#7 composer_manager-invoke-post-install-hook-2272547-7.patch1.49 KBAnonymous (not verified)
#3 composer_manager-invoke-post-install-hook-2272547-4.patch1.48 KBAnonymous (not verified)
#2 composer_manager-invoke-post-install-hook-2272547-3.patch1.38 KBboztek

Comments

cpliakas’s picture

cpliakas commented

I'm all for adding hooks, thanks for the idea and I think the use case makes a lot of sense.

The one comment I have is to consider changing the name to reflect nomenclature used in the Composer project and be consistent with Drupal things like hook_node_load and hook_node_insert where the format is "hook" _ "noun" _ "verb that just happened". I would propose naming this hook "composer_manager_dependencies_install".

Thoughts? Other suggestions?

Thanks!
Chris

boztek’s picture

boztek commented
Status: Active » Needs review
StatusFileSize
new composer_manager-invoke-post-install-hook-2272547-3.patch1.38 KB

This patch invokes hook_composer_manager_install after every drush invocation of composer install.

Anonymous’s picture

Anonymous (not verified) commented
StatusFileSize
new composer_manager-invoke-post-install-hook-2272547-4.patch1.48 KB

I needed to re-roll this patch to make a small modification to it. This now works with the use case we had with the code currently checked in here: http://drupal.org/project/behatrunner

cpliakas’s picture

cpliakas commented
Status: Needs review » Needs work

Patch function looks great, but I would like to adhere to the hook - noun - verb convention. I know that hook_composer_manager_dependencies_install is verbose, but it is explicit whereas hook_composer_manager_install implies it is invoked after the module is installed. Let me know if you feel strongly otherwise.

Thanks for rolling the patch!

cpliakas’s picture

cpliakas commented

In order to make it more terse, we could also do hook_composer_dependencies_install(). This is consistent with hook_composer_json_alter() and makes it a little more human readable as well.

Anonymous’s picture

Anonymous (not verified) commented

No problem at all, I completely understand. I'll re-roll and post momentarily.

Anonymous’s picture

Anonymous (not verified) commented
Status: Needs work » Needs review
StatusFileSize
new composer_manager-invoke-post-install-hook-2272547-7.patch1.49 KB

Patch re-rolled with updated name.

cpliakas’s picture

cpliakas commented
Status: Needs review » Reviewed & tested by the community

Looks good, works as advertised. Thanks for the contribution @boztek and @nateswart!

cpliakas’s picture

cpliakas commented
Status: Reviewed & tested by the community » Fixed

Committed to 7.x and 6.x branches, rolled a 6.x-1.5 and 7.x-1.5 release so that the hook is available in a stable release for modules like BehatRunner to use without requiring the dev version or a patch.

Anonymous’s picture

Anonymous (not verified) commented

Awesome - thanks a ton, Chris!

  • Commit 7b39b66 on 7.x-1.x authored by nateswart, committed by cpliakas: 
    Issue #2272547 by boztek, nateswart: Invoke hook after composer...

  • Commit c30aab7 on 7.x-2.x authored by nateswart, committed by cpliakas: 
    Issue #2272547 by boztek, nateswart: Invoke hook after composer...

  • Commit 7ed74b6 on 6.x-1.x authored by nateswart, committed by cpliakas: 
    Issue #2272547 by boztek, nateswart: Invoke hook after composer...

  • Commit 5f1d873 on 6.x-2.x authored by nateswart, committed by cpliakas: 
    Issue #2272547 by boztek, nateswart: Invoke hook after composer...

Status: Fixed » Closed (fixed)

Automatically closed - issue fixed for 2 weeks with no activity.