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 Needs review
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