drupal-vm icon indicating copy to clipboard operation
drupal-vm copied to clipboard

Make Drupal VM into a Composer plugin that scaffolds a Vagrantfile

Open oxyc opened this issue 7 years ago • 25 comments

Proof of concept for #1247 which supports all earlier setups with the addition of reading config_dir from composer.json.

Might want to drop support for some scenarios at some point tbh...

oxyc avatar Mar 24 '17 18:03 oxyc

Like basically say "Add Drupal VM as a composer dependency, or use it standalone, and that's it"?

geerlingguy avatar Mar 24 '17 18:03 geerlingguy

Yeah, that would probably make it easier.

Just realized that making this change would break all existing composer dependency setups as the old Vagrantfile will be overridden and the DRUPALVM_CONFIG_DIR definition will disappear.

oxyc avatar Mar 24 '17 19:03 oxyc

break all existing composer dependency setups

😩

geerlingguy avatar Mar 24 '17 19:03 geerlingguy

Also, with the Vagrantfile ever-expanding in size, I wonder if it might be good to have a 'ruby-src subfolder somewhere (something like that... whatever the Ruby convention would be) and break out business logic in there?

Or if there's some other way of building up a complicated Vagrantfile that's a better pattern. It's just getting unwieldy at this point.

geerlingguy avatar Mar 24 '17 19:03 geerlingguy

Only example I've seen that breaks out the logic is homestead.

https://github.com/laravel/homestead/blob/master/Vagrantfile https://github.com/laravel/homestead/blob/master/scripts/homestead.rb

oxyc avatar Mar 24 '17 19:03 oxyc

This is what it would look like if we dropped support for a delegating Vagrantfile.

Edit: Added a commit so it's easier to read. I can always revert it.

If we decide to go with this I guess the PR will have to wait until Drupal VM 5.0.0 as this will truly break all the things.

oxyc avatar Mar 25 '17 01:03 oxyc

break all existing composer dependency setups

This is actually the reason we are dropping all other installation methods so it's easier/safer to make BC breaks - https://github.com/beetboxvm/beetbox/issues/391#issue-209679900

But yeah these changes are only coming in the next minor version bump.

thom8 avatar Mar 26 '17 01:03 thom8

@oxyc there's also a test Class which could be useful -- https://github.com/beetboxvm/beetbox/blob/master/tests/PluginTest.php

thom8 avatar Mar 26 '17 01:03 thom8

  • [x] approval
  • [x] update docs
  • [ ] should we include tests for the composer plugin?
  • [x] way more readable logic

oxyc avatar Apr 11 '17 04:04 oxyc

Also, with the Vagrantfile ever-expanding in size, I wonder if it might be good to have a 'ruby-src subfolder somewhere (something like that... whatever the Ruby convention would be) and break out business logic in there?

https://github.com/geerlingguy/drupal-vm/pull/1332

oxyc avatar Apr 23 '17 05:04 oxyc

@oxyc - Also, note that I think I'd like to include #1332 in a 4.x release, but I'm considering saving this change (along with more of a "this is how you do it the Drupal VM way" philosophy) for a 5.0 release. What do you think?

geerlingguy avatar Apr 23 '17 21:04 geerlingguy

Yeah I agree this should be saved for 5.0.

Btw, is there a way to detect which version you're updating from in a composer plugin? If it is, we could issue a warning stating that the Vagrantfile has been overridden and that config_dir now needs to be set in composer.json.

oxyc avatar Apr 23 '17 21:04 oxyc

Is there a way to detect which version you're updating from in a composer plugin?

That... I'm not sure of. I'm pretty new to Composer plugins myself.

geerlingguy avatar Apr 23 '17 21:04 geerlingguy

I came pretty far and managed to detect updates from <5.0.0 using Composer\Installer\PackageEvents::POST_PACKAGE_UPDATE but then I noticed that it won't run when the package is first updated, it will run the old code until the next update.

So I went a different, hackier, way. Thoughts?

screen shot 2017-04-23 at 19 10 32

oxyc avatar Apr 24 '17 00:04 oxyc

Also, note that I think I'd like to include #1332 in a 4.x release, but I'm considering saving this change (along with more of a "this is how you do it the Drupal VM way" philosophy) for a 5.0 release. What do you think?

Or we could just aim to release 5.0.0 at or shortly after DrupalCon? Like we did with 3.0.0 last year. Maybe too early for another major version bump though...

oxyc avatar Apr 24 '17 17:04 oxyc

Regarding docs for this I guess we could entirely delete Drupal VM as a Composer Dependency, or move it as a brief page under_Getting Started_ section somewhere.

In the Installation docs we point people to the Quick Start Guide. I guess we should either update the Quick Start Guide, or update these sections with how to set up a project using Drupal VM as a Composer dependency.

Basically the install instructions are

  1. Require Drupal VM as a Composer development dependency.

    composer require --dev geerlingguy/drupal-vm
    
  2. Create and configure a config.yml file, eg. in vm/config.yml.

  3. If you placed the config.yml file in a subdirectory tell Drupal VM about it. If not, Drupal VM will look for it in the root of your project.

     composer config extra.drupalvm.config_dir 'vm'
    

oxyc avatar Apr 24 '17 17:04 oxyc

@oxyc - I think one other thing that would be helpful to document somewhere/somehow is how you could still patch Drupal VM using something like https://github.com/cweagans/composer-patches — sometimes people need to either use a change in master but not a tagged release, or patch one specific task somewhere for their project...

geerlingguy avatar Apr 24 '17 18:04 geerlingguy

Maybe add as the first page Installation / How to integrate Drupal VM (I'm terrible at naming things)

Where we point out that there are two ways of integrating Drupal VM with your project.

  1. You download Drupal VM and build a quick site within Drupal VM using vagrant up. Point out the downsides.

  2. You require Drupal VM as a development dependency to your existing project. Show a quick start using drupal-composer/drupal-project

Then we also point to this page from the separate Install pages so people consider the options.

oxyc avatar Apr 24 '17 18:04 oxyc

@oxyc - I'm good with that. As long as you don't make people who use it standalone feel shameful, since that's probably going to still be the default/majority use case for a while until more of the community adopts Composer.

geerlingguy avatar Apr 24 '17 18:04 geerlingguy

that's probably going to still be the default/majority use case for a while

Yeah this would probably still be the first option mentioned in docs. What about the Quick Start Guide though? Should that be updated to reflect both or should we keep composer dependency as an advanced feature?

patch Drupal VM using something like https://github.com/cweagans/composer-patches

Mind blown! I hadn't even considered that.

oxyc avatar Apr 24 '17 18:04 oxyc

Do we want to add support for storing configuration values in the composer.json like beetbox?

We could either a) scaffold the file like they do or b) merge it into extra_vars as we already need to pass config_dir there anyways.

oxyc avatar Apr 25 '17 11:04 oxyc

@oxyc we decided to let config in composer.json overwrite the normal config file so it wasn't possible to manage config in both places, mainly to reduce the WTF factor...

Otherwise, debugging could get pretty difficult and confusing as to which takes precedence.

When using composer to manage config it's recommended to ignore the normal config files -- https://github.com/thom8/drupal8-vagrant/blob/master/.gitignore

Also run into a few issues with data type conversion from JSON to YAML, so it's still WIP ;)

thom8 avatar Apr 26 '17 00:04 thom8

we decided to let config in composer.json overwrite the normal config file so it wasn't possible to manage config in both places, mainly to reduce the WTF factor...

Yeah it could easily get very confusing! However I don't see much reason for the feature when the file needs to be created anyways. Drupal VM won't be providing defaults so the use case would mainly be forks or downstream projects (such as sprintbox) I guess?

oxyc avatar Apr 26 '17 00:04 oxyc

Just wanted to note that I grabbed some of the work here and dumped it into https://github.com/geerlingguy/drupal-vm-docker, which is more of a quick proof-of-concept just using the Drupal VM Docker container as a starting point.

I am still planning on eventually converting Drupal VM itself into a Composer Plugin—maybe for 5.0.0 still—but I don't think the timing is right yet; it seems the majority of people using Drupal VM still download or git clone it, and aren't ready to take the full Composer plunge.

But we can use https://github.com/geerlingguy/drupal-vm-docker as a testing ground for this, too.

geerlingguy avatar Apr 10 '18 03:04 geerlingguy

Dropping to 6.0.0 milestone. The Drupal community's adoption of Composer is just not there yet to default this thing to Composer-first :(

geerlingguy avatar Apr 04 '19 19:04 geerlingguy