Support for composer PluginEvents::INIT event. #117
Conversation
LionsAd changed the title 
composer.json Outdated
| }, | ||
| "require-dev": { | ||
| "composer/composer": "1.0.*@dev", | ||
| "composer/composer": "dev-master#defb8778787fbaabb464bb6946fe2ffb42f31b1d", |
There was a problem hiding this comment.
Please require ^1.1@dev, that's the correct way.
LionsAd force-pushed the use-composer-init branch from e7d037a to 3eaf4e5 Compare LionsAd changed the title 
| @bd808 Could we get this PR in please? Composer 1.1 is out and this makes installers and other early plugins work with the merge plugin. |
| Can this be reworked to not require Composer 1.1.x but instead opportunistically use it's features? The reason that I'm hesitant to require Composer 1.1.x is that Wikimedia has a few blockers to upgrading for our internal usage. |
| @bd808 Fixed, thanks! |
LionsAd force-pushed the use-composer-init branch from 575624b to 8bca63d Compare | @bd808 Now really fixed (tests pass now). Please consider taking another look. |
LionsAd force-pushed the use-composer-init branch from 8bca63d to 67b306a Compare 
src/MergePlugin.php Outdated
| { | ||
| $this->state->loadSettings(); | ||
| // @todo devMode cannot be properly detected at this stage, but | ||
| // merging it should not hurt. |
There was a problem hiding this comment.
I'm not sure that this assumption is really safe. Unconditionally enabling dev mode merging will make it impossible for anyone using Composer >=v1.1.0 to omit merged dev mode packages using the --no-dev CLI argument. This feels like a "but why would anyone do ..." rationalization that is easy to make during initial coding and difficult to fix later when someone presents a real world use case. The problem with not setting it to true is that currently mergeFile only merges each file one time to avoid certain side effects of double loading. This introduces the inverse problem of never being able to manage dev mode requirements if the flag is not set initially.
A possible fix would be to split merging in some way so that differentiates between processing in the two different dev mode states. Done properly this could allow the INIT event to process everything with setDevMode(false) (which is always safe and wanted) and then depending on the command invoked possibly trigger another pass with setDevMode(true) based on the known state when an install, update, or dump event is processed.
There was a problem hiding this comment.
I think the way to fix this issue would be to split the dev mode processing in ExtraPackage::mergeInto into a separate function. Then MergePlugin::mergeFile could be adjusted to call or not call the new function as appropriate. The guard against double processing a file there would then need to be adjusted to keep track of both types of merges. This would a bit of new complexity, but it shouldn't be too horrible.
LionsAd force-pushed the use-composer-init branch from ea80da4 to 29ce69a Compare | @bd808 All your concerns have been fixed. |
3 participants
Composer just added a very handy INIT event for 1.1 (and 1.1 is out now)
(composer/composer#5232 (comment)).
Notes
This makes custom installers work correctly with the merge plugin - without having to use hacks (douggreen/drupal-composer-installer#9).
PS: Overall this could simplify the merge plugin a lot - as now in theory just merging files once after plugin initialization could be enough - except for when it is installed in the same moment.