Dependencies should be 'unpacked' to the root composer.json and merged/resolved

I have created a WIP composer plugin for this functionality and available under https://gitlab.ewdev.ca/yonas.legesse/drupal-recipe-unpack. I'll be adding documentation and further features soon.

Steps:
1. Add the git repos under the projects repositories list as follows:

        {
            "type": "vcs",
            "url": "https://gitlab.ewdev.ca/yonas.legesse/drupal-recipe-unpack.git"
        }
    

2. Run the install of the package and the recipe.
composer require ewcomposer/unpack:dev-master
In my case, for the purpose of testing, i created a custom recipe and used it.
composer require drupal_recipe/startup

Then you just need to run the unpack command to update the projects package list:
composer unpack drupal_recipe/startup

It will update the composer and lock file with the dependencies available in the recipe.

That is interesting. I will have to give that a try.

Once unpacked, does it complain about version constraints between the main and recipe composer.jsons?

Not yet, but will work towards that. It will be a matter of adding some functions to check the constraints and maybe (if needed) provide a interactive session. Right now, the feature available is the ability to unpack the recipes packages into the projects package list and allow you to update them individually. Once the process is done, it removes the package dependency of the recipe from the composer and lock file.

This was the result of unpack:

From this recipe:

   "require": {
        "drupal/config_ignore": "^3.0",
        "drupal/csp": "^1.0",
        "drupal/field_group": "^3.0",
        "drupal/gin": "^3.0",
        "drupal/gin_login": "^2.0",
        "drupal/gin_toolbar": "^1.0",
        "drupal/pathauto": "^1.0",
        "drupal/memcache": "^2.0",
        "drupal/menu_block": "^1.0",
        "drupal/metatag": "^1.0",
        "drupal/redirect": "^1.0",
        "drupal/robotstxt": "^1.0"
    }

to:

 "drupal/config_ignore": "[>= 3.0.0.0-dev < 4.0.0.0-dev]",
        "drupal/context": "^5.0",
        "drupal/core-composer-scaffold": "^10",
        "drupal/core-project-message": "^10",
        "drupal/core-recommended": "^10",
        "drupal/csp": "[>= 1.0.0.0-dev < 2.0.0.0-dev]",
        "drupal/field_group": "[>= 3.0.0.0-dev < 4.0.0.0-dev]",
        "drupal/gin": "[>= 3.0.0.0-dev < 4.0.0.0-dev]",
        "drupal/gin_login": "[>= 2.0.0.0-dev < 3.0.0.0-dev]",
        "drupal/gin_toolbar": "[>= 1.0.0.0-dev < 2.0.0.0-dev]",
        "drupal/layout_builder_browser": "dev-1.x",
        "drupal/layout_builder_iframe_modal": "^1.0",
        "drupal/layout_builder_styles": "^2.0",
        "drupal/memcache": "[>= 2.0.0.0-dev < 3.0.0.0-dev]",
        "drupal/menu_block": "[>= 1.0.0.0-dev < 2.0.0.0-dev]",
        "drupal/metatag": "[>= 1.0.0.0-dev < 2.0.0.0-dev]",
        "drupal/pathauto": "[>= 1.0.0.0-dev < 2.0.0.0-dev]",
        "drupal/redirect": "[>= 1.0.0.0-dev < 2.0.0.0-dev]",
        "drupal/robotstxt": "[>= 1.0.0.0-dev < 2.0.0.0-dev]",

I wasn't sure what to do here necessarily - for example, Pathauto has no 2.0 release. That was just a theoretical I posed in the original question. Most of these do not have a major release above the ones listed in the recipe.

A few questions:

• Can it just copy out the same list to the root composer.json?
• Why does it assume this format as well as dev release [>= 1.0.0.0-dev < 2.0.0.0-dev]? Couldn't that be ^1.0||^2.0?

For the purpose of testing, i had hardcoded the version to *, which might be why you're seeing these issue. As i'm currently working on a startup recipe, i've noticed it as well. Let me update the plugin to account for the provided versions.

I've added pretty constraint reference and should extract the correct version. I've used the the sample recipe composer you provided and getting the following:

        "drupal/csp": "^1.0",
        "drupal/field_group": "^3.0",
        "drupal/gin": "^3.0",
        "drupal/gin_login": "^2.0",
        "drupal/gin_toolbar": "^1.0",
        "drupal/memcache": "^2.0",
        "drupal/menu_block": "^1.0",
        "drupal/metatag": "^1.0",
        "drupal/pathauto": "^1.0",
        "drupal/redirect": "^1.0",
        "drupal/robotstxt": "^1.0",
 

Adding the DrupalCon tag as discussing today.

I've been testing this and I've seen some success. One high-level thing that I'm wondering: should we postpone removing the recipe as a composer dependency and just focus on the first part, pulling the recipe's dependencies into the root composer.json, at least for now?

Following up from #11 - it feels odd to composer install a recipe, then upack it. You end up with the recipe in your codebase, installed by composer, but it's no longer managed by composer.

Currently working on this.

I understand the issue and completely agree. As one of the aims with the recipe initiative is to allow "application of a recipe at any point within the project cycle" as well as "allowing for updates", i like your suggestion. In that case, we can just update the plugins code to avoid removing the recipe once it unpacks it.

I see the issue and completely agree. As one of the aims with the recipe initiative is to allow "application of a recipe at any point within the project cycle" as well as "allowing for updates", i like your suggestion. In that case, we can just update the plugins code to avoid removing the recipe once it unpacks it.

@yonas.legesse - as I played with this more, a couple things I've been thinking about are:

1. I think some folks will want their recipe to stay in composer, while others will not. I wonder if we can create a configuration option to determine "remove unpacked item from composer.json", that way the user can decide whether the thing their unpacking stays in composer or not. (While it's not supported by the initiative, folks are talking about wanting to re-apply the same recipe again later to get its updates. I could imagine something like that happening in contrib and I think they'd need the recipe to stay managed by composer in order to do that.)
2. If I "unpack" a recipe, it must recursively unpack any recipes required by the recipe I'm unpacking. In particular, one way folks want to use recipes is to replace install profiles / distributions, I might have one recipe that I apply for that, but that recipe will almost certainly contain dozens of sub-recipes (individual recipes for different media types, content types, paragraphs, blocks, etc), all of which depend on different contrib modules, etc. If I unpack the parent recipe, any recipe that is required by that recipe must also be unpacked.
3. I wonder if we need a "repack" option, though I'm not even sure how that would work.

Also worth noting, I'm in the middle of some work building upon your initial share above, e.g. the recursing piece. I wonder if we can create a project on d.o so that we can collaborate more easily and review changes, etc.

@sonfd, i really appreciate your suggestions. I was especially thinking about the recursive unpacking so would love to clone the project to d.o and collaborate.

@yonas.legesse -

Agreed. I think this is a packagist library, ultimately, and is entirely decoupled from Drupal. At the end of the day, we're really just targeting a package type, which happens to be drupal-recipe, but it should work for any other package type too.

To me, MVP looks something like this:

1. A command composer unpack [packages] that copies all requirements of a package into composer.json.
2. A configuration option, set in composer.json, that specifies which package types should be auto-unpacked, i.e. unpacked when they're required or updated.
3. A configuration option, also set in composer.json, that specifies which package types should be recursively unpacked, i.e. unpacked when encountered during an unpack action. (Perhaps this should be the same setting as auto-unpacking. The case that I think about is if someone doesn't want to auto-unpack on require, but definitely would want packages to be unpacked recursively when they call composer unpack manually. For example, if manually unpacking a recipe that requires other recipes.)

Intentionally left out of the list is removing the unpacked package from composer.json. Though, I think that's probably another configuration option in `composer.json`.

I've added the suggestion as a features roadmap within the README. I've created the project on Github https://github.com/woredeyonas/Drupal-Recipe-Unpack. Lets created the milestones as issues and continue.

@yonas.legesse - Thanks for sharing! I opened one issue, recommending that you rename to composer-unpack to keep it a generic composer plugin.

Additionally, I have my own repository where I took your original code and started working on it. I'll fork your repo and open some PRs sometime this week.

Thanks again.

Reading up a bit on the symfony unpack feature, they even made it the default (auto) behavior now to unpack composer packages when installing symfony packs (which are really similar to what recipes are).
Was wondering if we could just pull in symfony/flex and then use the symfony unpack command.
https://github.com/symfony/flex/tree/2.x/src/Unpack

@mallezie the symfony/flex package is a huge pack and done for starting up a boilerplate symfony project with minimal configuration and that might be a jargon for our use case. I've tried that and didn't seem to work out of the box and might need some workaround. So, here's what we can do:
1. Try requiring it in a default project and see if it works. If not, see what work around is required and whether it will be acceptable
2. Think about scalability and how much we want to add features to this plugin pertinent to Drupal recipe.

I used https://gitlab.ewdev.ca/yonas.legesse/drupal-recipe-unpack today.

Worked as expected, although there was no success message or anything upon completion. That would be good to add to give a user the assurance that it worked.

I believe when I had packages in my site's composer.json file with specificity, it did not update when my recipe specified drupal/package: '*'. I don't know if that is expected behavior, I just wanted to call it out.

Thanks for a cool tool!

@yonas.legesse is it okay if we add you code to the recipes fork and make it a part of the initiative?

@alexpott Absolutely. I would also volunteer to work on it to make it a complete tool.

If a recipe requires another recipe, are the dependencies from the dependent recipe unpacked also?

If a recipe requires another recipe, are the dependencies from the dependent recipe unpacked also?

The answer is no. If I require a recipe package that requires other recipe packages, when I unpack, I get dependent recipes unpacked to my site's composer.json file.

This works probably exactly as expected. However, if I only depend on the recipes, they could update/change the recipes and dependencies, and since I don't re-apply the recipe, I am tied to a stack of dependencies that could get out of sync.

New question is, should all the dependencies of recipes be unpacked?

This could also be documented to developers. When you apply recipe A, just unpack sub-recipe-B, sub recipe-C, etc..

Per the new requirements (see Slack thread):

The recipe unpack plugin should unpack a recipe upon running composer require drupal/a-recipe

composer require would only unpack a recipe's dependencies and it will up to the user whether they apply the recipe or not. What we're trying to avoid is adding a recipe as a dependency, as that's the whole premise of recipes.

About how we want to handle a recipe removal, that's another discussion. With this approach, the recipe package reference wouldn't exist in composer.json so composer remove would not find it. The remove functionality could be either added into the composer plugin, as a new drush command, or part of the drupal script.

I worked a on adding an unpack composer plugin into core.

This plugin will unpack a recipe's dependencies into the root composer.json and update the composer.lock as well. If a recipe has other recipes as dependencies, those will also be unpacked.

I created the code working around the idea proposed by @thejimbirch of adding unpacking support for other types of projects (i.e, modules or themes). This way, creating other unpackers should be easy.

Pending tasks:

1. Add support to also unpack patches
2. Add tests

I will working on the unpack patches during next week and work as feedback is provided, then start adding tests to core for this plugin.

How to test manually

1. Make your project support recipes
2. Add the plugin repository in your composer.json:
   

   {
     "type": "path",
     "version": "dev-master",
     "url": "composer/Plugin/Unpack"
   },

3. Require and enable the plugin:
   composer config allow-plugins.drupal/core-composer-unpack true
composer require drupal/core-composer-unpack
4. Install a recipe!

Upon installing it, its dependencies must be unpacked into your root composer.json. If your recipe depends on other recipes, those recipes should be unpacked as well.

Shout out to @yonas.legesse - I used part of its plugin to create this, thanks!

I tried the above, and it appears to be working as expected with multiple tests and different recipes/dependencies, though I'm not convinced removing the recipe itself from composer.json/lock is the correct approach here.

What we're trying to avoid is adding a recipe as a dependency, as that's the whole premise of recipes.

Given we've now unpacked the dependencies, what's the concern of having the recipe here?

About how we want to handle a recipe removal, that's another discussion.

If the recipe was still in composer, removing it there would keep all the unpacked dependencies, and literally just delete the recipe.

Also, because recipes allow default content, how would you deploy a recipe to a remote environment without then re-requiring the recipe on that environment's composer? Is the expectation that the recipes folder is a committed folder rather than package managed? I wouldn't be entirely opposed to that, but I am slightly scarred from npm/bower-asset vs commit libraries folder :)

If the approach is to commit the recipes folder, and then delete them when no longer needed then this likely needs some docs around DX.

Marking NW for discussion as well as "Pending tasks" above.

Also, because recipes allow default content, how would you deploy a recipe to a remote environment without then re-requiring the recipe on that environment's composer? Is the expectation that the recipes folder is a committed folder rather than package managed? I wouldn't be entirely opposed to that, but I am slightly scarred from npm/bower-asset vs commit libraries folder :)

Recipes should not become a content deployment mechanism. We need to improve default content tooling in the module and core so that this can happen.

Leaving recipes in the root composer.json then subjects them to being updated via composer update. Recipes are not intended to be updated and reapplied like this.

Recipes should not become a content deployment mechanism. We need to improve default content tooling in the module and core so that this can happen.

Agree, but recipes currently do support default content, so how is a developer expected to deal with this?

With Recipes, I'd expect you require the recipe locally (which unpacks it's dependencies but not the recipe itself), apply it, export your config, commit and you're good to go.

With default content however, if the recipe requires the content to be functional (let's use the example of creating/setting the frontpage), then unless you're porting the DB to the next env, you're back to the same chicken/egg of content blocks in config. So how does this work?

Leaving recipes in the root composer.json then subjects them to being updated via composer update. Recipes are not intended to be updated and reapplied like this.

Makes sense, and agree.

With default content however, if the recipe requires the content to be functional (let's use the example of creating/setting the frontpage), then unless you're porting the DB to the next env, you're back to the same chicken/egg of content blocks in config. So how does this work?

With the current functionality of default content, you cannot definitively create and set the home page. The node you create could be nid 14 on your local, but NID 20139 on the site the recipe is applied to.

I agree with you that functionality would be super awesome, but we haven't started to understand the default content module issues yet to be able to get core to do what we assume it should.

Added support for patches in recipes composer.json. If the project's root composer JSON has the patches plugin and the recipes has patches in its composer.json, this will be unpacked into the root composer.json, as long as you have patching unpacking enable in your project's composer.json, like this:

"drupal-unpack": {
    "drupal-recipe": {
        "unpack-dev-dependencies": false,
         "unpack-patches": true
    }
},

This config should be within the "extra" section of your composer.json.

Pending task: tests!

I tested by following the steps from #35 📌 Dependencies should be 'unpacked' to the root composer.json and merged/resolved Active
and recipe dependencies are getting added to the root composer.json

All the steps to test are already mentioned in #5 but it is unclear where to add the the following:

1. Add
   

   {
     "type": "path",
     "version": "dev-master",
     "url": "composer/Plugin/Unpack"
   },
   

   to the root composer.jsonAdd it to the "repositories": [section.

The repositories section will look something like this:

"repositories": [
        {
            "type": "path",
            "url": "core"
        },
        {
            "type": "path",
            "url": "composer/Plugin/ProjectMessage"
        },
        {
            "type": "path",
            "url": "composer/Plugin/VendorHardening"
        },
        {
            "type": "composer",
            "url": "https://packages.drupal.org/8"
        },
        {
            "type": "path",
            "version": "dev-master",
            "url": "composer/Plugin/Unpack"
        }
    ]

2. Installed the recipe https://packagist.org/packages/kanopi/gin-admin-experience with composer require kanopi/gin-admin-experience
3. All its dependencies got added to the root compose.json file
   

      "require": {
            "composer/installers": "^2.0",
   +        "drupal/admin_toolbar": "*",
            "drupal/core": "self.version",
   +        "drupal/core-composer-unpack": "11.x-dev",
            "drupal/core-project-message": "self.version",
           "drupal/core-vendor-hardening": "self.version",
   +        "drupal/gin": "*",
   +        "drupal/gin_login": "*",
   +        "drupal/gin_toolbar": "*"
   
   

However, when I tried to remove the recipe with composer composer remove kanopi/gin-admin-experience, the dependencies did not get removed from the composer.json file.

composer require drupal/core-composer-unpack

Where is this package hosted? Can't find it on packagist.org?

Ah, I see: once the MR gets merged, there will be a local instance of this plugin in composer/Plugin, which can be added to the composer repositories, allowing to require the plugin.
Ok, perspectively it might make sense to host it on packagist.org, just like the other core-xxx plugins.

A few things needed to get this moving again:

• To keep scope under control, we should remove the support for patches, for now, and deal with that in a follow-up issue that already exists: ✨ "Unpack" patches from a recipes composer.json to the root composer.json Active .
• Let's get the failing CI jobs passing!
• Can we get some functional test coverage? We can probably take inspiration from whatever tests exists for core's scaffold plugin.

Left an initial review - I think my questions largely concern the overall complexity of the MR here. It feels a like there are more moving parts than necessary; I think we might want to try to simplify and streamline it a bit.

To be honest, I felt the same way about the scaffold plugin, but I was not involved in developing that. On the other hand, the scaffold plugin is far more complicated, so it might have been designed with that in mind. We should not follow in the scaffold plugin's footsteps unless we really need to. Simplicity is better!

For #3355485-46: Dependencies should be 'unpacked' to the root composer.json and merged/resolved → : yeah, I'll try to add some tests.

#3355485-48: Dependencies should be 'unpacked' to the root composer.json and merged/resolved → :
I guess the complexity of it comes from two things:
1) I did base the code around the scaffold plugin, as I was not sure how these plugins are developed in core, so I agree that it's complex and I believe simplicity should be our approach.
2) I was thinking about extensibility when creating the plugin. The idea came from the conversation on the recipes channel, about maybe other unpackers in the future, if needed. So, that's why there's some configurability around it. If we don't think we'll need more unpackers in the future, I think we can simplify the code, but I do like the idea of it being ready in case of some other feature or requirement in the future.

Added some base tests.

This seems like a blocking issue for project browser/automatic updates to work with recipes so tagging with 'Drupal CMS stable release blocker'. I'm basing this on the understanding that if a recipe is installed, it will be impossible to safely remove it without unpacking, because doing so could also remove the dependencies of the recipe (which will usually be installed on the Drupal site but not in the root composer.json). This means that if we add a way to remove a recipe without this, sites will get broken, but also if that's not implemented, sites installed before this issue is done could end up with crufty recipes that are impossible for the site owner (using project browser) to remove.

Also concerned about the version constraints when unpacking. If a recipe has a < 1.3 constraint for any reason and we unpack that constraint to the root composer.json, it will prevent updates of the module with that constraint, even though there is no reason to prevent that (assuming the module has an upgrade path for its own config/content from 1.2 to 1.3). So feels like unpacking needs to essentially composer require ~$current_installed_version rather than copy constraints over - to allow later upgrade paths as if the site owner had just run composer require themselves in the first place (or had the module already installed etc.)..

This is neither a Drupal CMS release target nor a stable blocker. It'd be nice to have, but it doesn't block anything.

If someone spins up Drupal CMS with dependencies on recipes, that's...fine. They can continue operating their site with the recipes keeping their dependencies in place. When unpacking becomes possible, they'll be able to remove that intermediate layer of dependencies...but it's not a big deal for those to sit there while we work on this.

Discussed with @thejimbirch in slack and decided to move this to core. If there's a really good reason to keep it against the recipes and distributions project that's fine but seems more historical that it was posted there - now that recipes is in core and we've made several changes directly.

This is very much on my list to review when I'm back next week. I'd like Drupal CMS to adopt it as soon as humanly possible.

Removing Needs tests tag as tests were added.

The issue was moved to Drupal core, but the MR needs to be moved also.

b_sharpe → changed the visibility of the branch 3355485-recipes-unpack to hidden.

MR moved over to core, looks like some cspell and phpstan failures need addressing.

4 Failing PHPstan tests remain.

 Line   composer/Plugin/Unpack/UnpackCollection.php                           
 ------ ---------------------------------------------------------------------- 
  36     Method Drupal\Composer\Plugin\Unpack\UnpackCollection::getIterator()  
         return type with generic class ArrayIterator does not specify its     
         types: TKey, TValue                                                   
         🪪  missingType.generics                                              
 ------ ---------------------------------------------------------------------- 
 ------ ------------------------------------------------------------------------------------------- 
  Line   core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php                              
 ------ ------------------------------------------------------------------------------------------- 
         Ignored error pattern #^Method                                                             
         Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:cloneFixtureProjects\(\)            
         has no return type specified\.$# (missingType.return) in path                              
         /build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php                       
         was not matched in reported errors.                                                        
         Ignored error pattern #^Method                                                             
         Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:createIsolatedComposerCacheDir\(\)  
         has no return type specified\.$# (missingType.return) in path                              
         /build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php                       
         was not matched in reported errors.                                                        
         Ignored error pattern #^Method                                                             
         Drupal\\Tests\\Composer\\Plugin\\Scaffold\\Fixtures\:\:tearDown\(\)                        
         has no return type specified\.$# (missingType.return) in path                              
         /build/core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php                       
         was not matched in reported errors.                                                        
 ------ ------------------------------------------------------------------------------------------- 
 [ERROR] Found 4 errors               

Fixed PHPStan error on UnpackCollection::getIterator() .

I'm not quite sure about the errors on core/tests/Drupal/Tests/Composer/Plugin/Scaffold/Fixtures.php though. They are void functions—no need for return type.

@robert-arias These are not about actual errors, but known errors to be ignored. As they don't happen anymore they need to be removed from core/.phpstan-baseline.php

I have tested the unpacking with nested recipe (more than 4 levels) and it worked correctly. Unfortunately the patches unpacker was moved to follow-up issue, so this is still not completely covering my use case, as some recipes Do have patches, and transferring them manually is quite a big task.

I have a silly question: how to unpack already installed recipe? The recipe that was added before the core-composer-update package. Is there a command to manually trigger the unpacking? I believe all users who were waiting for unpacker in core and had there recipe in composer.json would like to get rid of them when it is possible. So it would be nice to know how to trigger the command for certain package manually, if it exists, if not - I think that would be a good addition to this feature.

I would love to move it to RTBC, but there is some non-addressed feedback in the MR.

The Needs Review Queue Bot → tested this issue. It no longer applies to Drupal core. Therefore, this issue status is now "Needs work".

This does not mean that the patch necessarily needs to be re-rolled or the MR rebased. Read the Issue Summary, the issue tags and the latest discussion here to determine what needs to be done.

Consult the Drupal Contributor Guide → to find step-by-step guides for working with issues.

I've rebased the MR on top of 11.x to make it mergeable.

I'm working through the comments on the MR.

I've added a new drupal:unpack RECIPE/PACKAGE command to help existing sites.

Still to do:

• Add test coverage of the new command
• Check existing test coverage to make sure it is good
• Write a change record
• Add plugin to core's root composer.json and the project templates.

Note that now we have the composer command linked up for a core checkout this code can easily be tested. On a core checkout on this MR branch you can do:

# Require a recipe. It will not be unpacked because we disable automatic unpacking in the core root composer.json
$ composer require drupal/events

# Unpack the recipe
$ composer drupal:unpack drupal/events

If you want to test automatic unpacking you can update the root composer.json add set on-install-and-update to true in the extras section.

@phenaproxima @thejimbirch and I spoke about unpacking in the contrib room at DrupalCon Atlanta. (Some initial discussion is in this slack thread.) Our discussion was about whether recipes should be auto-unpacked and when should they be auto-unpacked. I'm documenting here while it's fresh.

Our starting point was that recipes will be (unless the drupal-unpack plugin is disabled) auto-unpacked when the recipe is required with composer require. This creates two scenarios that we're not sure about:

1. If I `composer require` a recipe and it was the wrong one, I can't just `composer remove` it to get back to the starting point because it's unpacked all of its dependencies into my composer.json.
2. Since recipes can accept input, it's a valid use-case to have a "Content Type generator" recipe, or similar. I may want this recipe to stick around in my site. If it's not managed by composer, it may not stick around in my project and won't be easily updatable. And since recipes are normally managed by composer, they're probably not part of my git repository.

For #1, I think our consensus is that this scenario is not ideal, but also maybe not that big of a problem. In many situations, e.g. while using Project Browser to apply a recipe, users will have no awareness of their composer.json and having extra dependencies won't cause them any problems. And there's been talk of a composer plugin to remove orphaned dependencies that could further reduce the relevance of this problem.

For #2, one idea is that unpacking recipes should be a game-time decision. Maybe by default recipes should be auto-unpacked on require, but something like composer require drupal/some-recipe --no-unpack would prevent the recipe from being unpacked. (But according to some, it may not be possible to add this flag to composer require.)

@thejimbirch and I discussed this issue with @alexpott.

One thing we agreed on is that this should be for recipes only. We can remove UnpackerInterface and UnpackerFactory; they won't be needed. To make it even clearer that this plugin is only intended for use with recipes, we should rename it to drupal/core-recipe-unpack, under the Drupal\Composer\Plugin\RecipeUnpack namespace. Any configuration flags should be under extra.drupal-recipe-unpack in composer.json.

I think we also agreed that a reasonable default workflow is that the recipes are unpacked as soon as they are required into your site, and are not kept as Composer-managed packages.

Will that leave orphaned dependencies in your composer.json if you ultimately don't apply the recipe you just required? Quite possibly, yes. Having an additional plugin to remove dependencies that you're not actually using would be helpful, but is not in scope here; we could add that in a separate issue. Or maybe it could be implemented by Package Manager. Either way, it's not our problem right now.

I think that this "auto-unpack" behavior should be configurable, and enabled by default. If a site builder chooses to disable it, they can still call composer drupal:recipe-unpack foo/bar at the command line to unpack it later.

So, kicking this back for those changes.

I ended up removing the unit test here because:

1. It makes the architecture leaky; classes that should be final have to be left open so we can mock them; and methods that should be private have to be made public so we can call them.
2. I'm not sure it really gives us any more confidence in the code than the functional test does. Unit tests can have an unfortunate tendency to simply become a repeat of the main business logic, and I think that's the case here. The functional test is what really shows us that the plugin works as intended, so I think we should double down on it. I think the functional test is also now testing most of what we care about, since the scope of the plugin is reduced to just handling recipes, so I'm not sure the unit test is adding much value in light of that.

Also, one thing we have not yet handled here, as far as I can see: What if a recipe has a dependency that is already required in the top-level composer.json?

IMHO, the existing requirement and constraint should "win", and be kept as-is. This will definitely need functional test coverage.

🌱 [meta] composer require / install module discrepancy issues Active is open against project browser for a UI to be able to remove unused dependencies. We should possibly open a core issue for the detection and removal bits themselves which could maybe live in package_manager?

Agreed it's fine to ignore that here, lots of ways that sites can end up with unused composer dependencies already.

@phenaproxima - I think handling for a recipe requirement already being in composer.json is accounted for ~ line 116 of RecipeUnpacker and works as you want, though I didn't check on tests.

Ah, you're totally right! Glad it's there. I do think that will need test coverage, then.

@phenaproxima I added test coverage of an existing dependency. I've also got test coverage of moving a dep from require-dev to require and not moving a dep from require to require-dev :)

I've got unpacking of require and require-dev recipes working. We've got some decisions about behaviour to decide.

We need to decide how the following situations work...

We have recipeA and recipeB. recipeA depends on recipeB.

What should happen when recipeA is in require-dev and recipeB is in require and you call unpack recipeA?
What should happen when recipeA is in require-dev and recipeB is in require and you call unpack recipeB?
What should happen when recipeA is in require and recipeB is in require-dev and you call unpack recipeA?
What should happen when recipeA is in require and recipeB is in require-dev and you call unpack recipeB?

For more information on this problem space if you do the following on an empty directory:

$ composer init --no-interaction --name test/test --type project
$ composer require --dev symfony/yaml
$ composer require symfony/yaml

The second require will offer to move the dependency. If you answer no then it will not do the include.

@alexpott and I discussed this extensively in a Slack DM.

Here was the long thought process I had, for those interested:

My current thoughts on this:

What should happen when recipeA is in require-dev and recipeB is in require and you call unpack recipeA?
Everything goes to require because some of recipeA’s dependencies are in require, although a more fiddly way to do this would be to “switch modes” midstream such that everything at or below recipeB in the dependency tree, is unpacked to require. But that feels more complicated than we need to bother with.

What should happen when recipeA is in require-dev and recipeB is in require and you call unpack recipeB?

Everything goes to require. You are explicitly unpacking something in require, therefore everything under it goes to require.

What should happen when recipeA is in require and recipeB is in require-dev and you call unpack recipeA?

Everything goes to require. For the same reason as above. recipeB is moved to require because it’s a dependency of recipeA.

What should happen when recipeA is in require and recipeB is in require-dev and you call unpack recipeB?

Everything goes to require. recipeA depends on recipeB, and recipeA is a requirement, which implies that all of its dependencies are too. (edited) 

Another way to put my thinking on this is: require always has more “gravity” than require-dev (edited) 

For the simple reason that, semantically speaking, require means “I cannot function without this”

It is also worth mentioning that we don’t need the unpacking to be “perfect”. If it moves something into require that’s really more dev-oriented, we actually have no way to know that, because we don’t know how the site builder is using that module or theme in their specific context. It can and should be up to the site builder to sort that out. Our priority needs to be not breaking sites (edited) 

^^ Which, now I think about it, is actually a pretty compelling reason to always unpack to require, unconditionally, and never do require-dev.

To put it another way: some module/theme that originally came in as a dev dependency might actually end up being used, in practice, as a production dependency. And Composer would have no way to know that. We’d have to rely on the site builder knowing how Composer works, which we absolutely cannot rely on. (edited) 

Here’s another reason to avoid require-dev: it can introduce confusion. Someone might require a recipe into require-dev with the idea that “well, I’m only using this recipe to help build the site, and then I’ll remove it” — that’s a reasonable line of thought. But then let’s say they do exactly that, and now they have their working site, but have unwittingly unpacked all these runtime dependencies introduced by the recipe into require-dev. They are in a prime position to WSOD if they unwittingly do composer install --no-dev , which is best practice in production. (edited) 

So I dunno. require-dev for recipes at all might be really bad news.

It might have made more sense back when there was this idea that the unpack plugin could handle any kind of package. But for recipes specifically, now that we’ve narrow-scoped it? Not sure.

And here's my summation, your honor:

Recipes introduce dependencies on modules and themes that Drupal will expect to be present, or it will WSOD. If you have any of those dependencies in require-dev, you are one composer install --no-dev away from breaking your site completely. (Omitting dev dependencies from production is a best practice.)

We cannot safely assume that site builders, who may have zero understanding of or experience with Composer, will know that installed modules and themes effectively must to be in composer.json's require section.

It is also much less important for unpacking to result in "everything in its right place", than for it to result in a working site that isn't broken.

@alexpott proposed what I think is an excellent solution that sidesteps the problems outlined in #82:

I think we should only support unpacking recipes in require

If you do require --dev recipe we should not auto unpack and tell people that dev recipes are not unpacked

If you call unpack on a dev required recipe we tell you we’re going to move it to require and unpack

And ask the user if that is okay

If no don’t unpack

I think with those rules we’ll have way less complexity [and magic]

Yes. YES! A thousand times yes to this.

I should note that the solution in #83, while great, doesn't actually remove all risk to site operators. Consider:

If you do require --dev recipe we should not auto unpack and tell people that dev recipes are not unpacked

This will still bring in modules and themes, as indirect dependencies of the recipe. Therefore, you could still break your site with composer install --no-dev.

But, @alexpott and I feel that this is an acceptable trade-off, for a few reasons:

• This isn't a whole lot different than directly adding modules and themes to require-dev. It's just that the recipe adds a layer of indirection. So it's a tad worse, but not much.
• For the recipe to be a dev dependency, you would have had to specifically run composer require RECIPE --dev. It would have been an explicit choice by a developer.
• If you add a recipe to a site via Project Browser, it won't add it as a dev dependency. This is what we would expect most site builders to do (in Drupal CMS, anyway).
• But even if a site builder did add a recipe at the command line, composer require's default behavior is to add the recipe to require, not require-dev.

If, despite these mitigating factors, you do somehow manage to get a recipe into require-dev, the unpack plugin will try to nudge you in the right direction, as #83 states:

If you call unpack on a dev required recipe we tell you we’re going to move it to require and unpack

And ask the user if that is okay

If no don’t unpack [i.e., keep the dependencies in place]

If, after all this, you still have a recipe in require-dev which has introduced runtime dependencies, and you have explicitly refused to unpack it into require, then that's on you, o mighty power user. :)

I've implemented #83/#84

Next thing to do is to not do anything when --dry-run is passed into the require command. Working on that.

So --dry-run works but it reveals another issue. Here's a useful composer.json for testing unpacking.

{
    "name": "test/test",
    "type": "project",
    "require": {
        "drupal/core-recipe-unpack": "@dev"
    },
    "repositories": {
      "composer-unpack": {
        "type": "path",
        "url": "/PATH/TO/YOUR/DRUPAL/MR_CHECKOUT/composer/Plugin/RecipeUnpack",
        "options": {
          "symlink": true
        }
      },
      "drupal": {
        "type": "composer",
        "url": "https://packages.drupal.org/8"
      }
    },
    "config": {
      "allow-plugins": {
         "drupal/core-recipe-unpack": true
      }
    }
}

Replace /PATH/TO/YOUR/DRUPAL/MR_CHECKOUT with the correct value for your set up...

If you do this then run the following commands on a directory with this in:

1. git init
2. git add .
3. git commit -m "Init commit"
4. composer install
5. composer require drupal/events

You'll see it is unpacked...
Then do...

1. git co .
2. composer require drupal/events

You'll see it is not unpacked.

This is because we find recipes being added during the post-package-install event but this is not triggered the second time around because the packages are there. I think we need to do cleverer stuff in post-update-cmd as this is triggered both times. And then we might need to deal with --dry-run :)

Okay I've fixed #87 and will still work with a --dry-run ... I'm going to add test coverage for both things and then I think we're ready for another round of review and rtbc :)

Okay I think we're 99% done. We just need to add tests for two things:

• The composer lock hash is correct after unpacking a recipe.
• The recipe files are left alone after unpacking.

Oh and I think we should add a follow-up to implement the ability to unpack multiple or even all the recipes in a root composer.json. Atm the unpack command accepts a single package name.

I've addressed #89 but in doing that I've revealed the two of the tests are resulting in invalid composer files so we need to fix that.

We have a problem...

If you have a composer.json as follows

{
    "name": "test/test",
    "type": "project",
    "require": {
        "drupal/core-recipe-unpack": "@dev",
        "composer/installers": "^2.3"
    },
    "repositories": {
      "composer-unpack": {
        "type": "path",
        "url": "/PATH/TO/CORE/CHECKOUT/WITH/MR/composer/Plugin/RecipeUnpack",
        "options": {
          "symlink": true
        }
      },
      "drupal": {
        "type": "composer",
        "url": "https://packages.drupal.org/8"
      }
    },    
    "extra": {
        "installer-paths": {
            "core": ["type:drupal-core"],
            "libraries/{$name}": ["type:drupal-library"],
            "modules/contrib/{$name}": ["type:drupal-module"],
            "profiles/contrib/{$name}": ["type:drupal-profile"],
            "themes/contrib/{$name}": ["type:drupal-theme"],
            "drush/Commands/contrib/{$name}": ["type:drupal-drush"],
            "modules/custom/{$name}": ["type:drupal-custom-module"],
            "themes/custom/{$name}": ["type:drupal-custom-theme"]
        }
    },
    "minimum-stability": "dev",
    "prefer-stable": true,
    "config": {
      "allow-plugins": {
            "drupal/core-recipe-unpack": true,
            "composer/installers": true
        }
    }
}

and then do the following:
1. composer install
2. composer require drupal/events
3. composer require drupal/jwt

Step 2 will successfully unpack the recipe but when you do Step 3 it'll detect the recipe has been removed and uninstall it. Which is something we want to avoid...

Oh dear.