search

silverstripe / silverstripe-framework

Silverstripe Framework, the MVC framework that powers Silverstripe CMS
https://www.silverstripe.org
BSD 3-Clause "New" or "Revised" License
719 stars 823 forks source link

Implement recipe pattern for installer #6860

Closed tractorcow closed 7 years ago

tractorcow commented 7 years ago

See https://github.com/silverstripe/silverstripe-framework/issues/5992

As a developer, I can start a SilverStripe project with sensible defaults and tailor them to my project's needs, while still being able to pull in upgrades.

Acceptance Criteria

Nice to have / technical notes:

Tasks

Peer review tasks:

sminnee commented 7 years ago

A few comments on these ACs:

I can use any recipe as a base via composer create-project and it will setup any required project files.

Will this mean that any content stored within a recipe's package will be copied into a new project? I would want to avoid this clutter.

A recipe exists that will install all necessary testing tools (e.g. behat, phpunit) and .travis.yml uses this

Shouldn't we just use require-dev of the modules for this?

A release (via silverstripe/cow) will automatically release all necessary recipes and create a tag for each.

I think that cow should take a recipe name as an argument and not attempt to contain within it a canonical list of recipes.

Also I think this an important use case (it's possibly covered by your 2nd "nice-to-have" but I want to make it explicit:

A user can create a project based on a recipe, remove one of the recipe-provided packages, and still make upgrades to future releases of the recipe without automatically installing that package.

I don't think that's a nice-to-have to be honest — without this requirement I'd see the recipe improvements of being of much reduced value.

tractorcow commented 7 years ago

I can use any recipe as a base via composer create-project and it will setup any required project files.

If using create-project, the files in root pretty much just stay there. that's all this means by "setup". :) Exactly how installer currently behaves.

I think that cow should take a recipe name as an argument and not attempt to contain within it a canonical list of recipes.

If we group recipes we should be able to do a combined release if needed correct? case in point, we don't want a separate release plan for cwp-recipe-basic, cwp-recipe-blog, and cwp-installer. We release them as one "main" recipe with separate components we tag as a single action.

I would see the parent recipe as being what's released, but it can trigger release of sub-recipes or dependency modules, as we do with cow releases of cwp at the moment.

Shouldn't we just use require-dev of the modules for this?

Ah, maybe. We can remove this A/C then.

A user can create a project based on a recipe, remove one of the recipe-provided packages, and still make upgrades to future releases of the recipe without automatically installing that package.

I'm in favour of adding this to the A/C too.

sminnee commented 7 years ago

I can use any recipe as a base via composer create-project and it will setup any required project files. If using create-project, the files in root pretty much just stay there. that's all this means by "setup". :) Exactly how installer currently behaves.

Yeah — this will mean that the content of recipe repos has to be limited to those files that would make sense as part of a new project — the dev/test resources for the recipe itself.

We've hit a number of points of conflict in relation to this; if this is the status-quo then I would suggest that we advise against using recipes directly with create-project and instead recommend something else, such as:

 mkdir project 
 cd project
 composer init
 composer require some/recipe

Alternatively, we could have a create-project base that is separate from the recipe.

sminnee commented 7 years ago

Case in point, we don't want a separate release plan for cwp-recipe-basic, cwp-recipe-blog, and cwp-installer

A couple of points:

tractorcow commented 7 years ago

Alternatively, we could have a create-project base that is separate from the recipe.

This is probably the other way that we could go; Split projects (root) and recipes (dependencies) entirely, or only require one or the other.

If you run cow on a parent recipe, having it run operations on all child recipes seems logical

Right. :)

I think that having 3 separate CWP recipes like this is a bit of an anti-pattern and a side-effect of us not having the recipe tooling discussed this project. I'd expect that instead we could just have 1 recipe and you could remove the pieces that you don't like.

If we could make recipes composable and able to be inlined, we could merge recipe-basic and blog, as we no longer have the issue of removable modules. If that's the case, do you want to make inlinable recipes an A/C rather than a nice to have?

tractorcow commented 7 years ago

We've hit a number of points of conflict in relation to this; if this is the status-quo then I would suggest that we advise against using recipes directly with create-project and instead recommend something else, such as:

mkdir project cd project composer init composer require some/recipe

Thinking more about this, getting rid of "create-project" might be the way to go, and we instead get users to pull in recipes as dependencies: No more installer?

sminnee commented 7 years ago

Thinking more about this, getting rid of "create-project" might be the way to go, and we instead get users to pull in recipes as dependencies: No more installer?

Let's get the recipe working without create-project first, and then review.

chillu commented 7 years ago

@tractorcow Sounds like a great idea, although I'm still working through it (lots to take in). For now, can you please make a case for why this needs to be in beta1, as opposed to later in the 4.0 release cycle? From my initial review, it doesn't look like it introduces API changes, just an alternative way of assembling project dependencies, correct? If we stop supporting the installer project for 4.0 final, that just affects new projects. Existing projects upgrading to 4.0 final will just need clear instructions for how their composer file should look like.

tractorcow commented 7 years ago

For now, can you please make a case for why this needs to be in beta1, as opposed to later in the 4.0 release cycle?

My expectation is that the initial version of the recipe system may have problems that come out of testing, which for many users won't be until a tag is made. We should want at least one level of separation (beta1 -> beta2) to iron out these issues before we stabilise the recipe API.

tractorcow commented 7 years ago

@chillu I've developed a proof of concept at https://github.com/tractorcow/recipe-cms (it's setup on packagist, so you can do a real-live install) if you want to see something tangible.

chillu commented 7 years ago

As discussed in our backlog planning, we can create a small beta3 release if you're concerned about beta2 being the first release where this approach is included - that doesn't make it beta1 material :)

tractorcow commented 7 years ago

That sounds fine :)

tractorcow commented 7 years ago

Tasked!

chillu commented 7 years ago

HVe you seen symfony flex? http://fabien.potencier.org/symfony4-compose-applications.html

tractorcow commented 7 years ago

Wow looks like a really hot idea!

sminnee commented 7 years ago

It'll be worth keeping an eye on what they're doing for inspiration, although the specific implementation looks like it'll be highly symfony-specific.

Another data point is Drupal installation profiles such as Acquia Lightning: was well as bundles of modules, these have setup wizards.

tractorcow commented 7 years ago

I've got a minimal composer require-recipe working. At the moment it only allows you to install a new recipe, but we'll also get update working later. image

image

tractorcow commented 7 years ago

Next week I'll work on update-recipe

tractorcow commented 7 years ago

ok, so these are the commands we'll probably have:

require-recipe add a new recipe and inline it. Fails if already installed. update-recipe update an inlined recipe. If the dependency isn't inlined, it'll automatically inline it.

Removing a recipe would pretty much be pulling out the inlined depedencies you don't want.... it's a bit hard to know which inlined-dependency is a part of a recipe and part of a customised project, isn't it?

How does this sound @sminnee ?

sminnee commented 7 years ago

Seems fine to me but I'd want a confirmation from @chillu :)

tractorcow commented 7 years ago

Please see https://github.com/tractorcow/recipe-plugin/issues/1 for peer review holder for plugin

tractorcow commented 7 years ago

Proof of concept at https://github.com/silverstripe/silverstripe-installer/pull/193

This is a big change! Please make sure to review.

flamerohr commented 7 years ago

have created https://github.com/silverstripe/silverstripe-framework/issues/7204 for the last task that couldn't be done until after all the merge happens.