drupal-vm
drupal-vm copied to clipboard
Make Drupal VM into a Composer plugin that scaffolds a Vagrantfile
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...
Like basically say "Add Drupal VM as a composer dependency, or use it standalone, and that's it"?
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.
break all existing composer dependency setups
😩
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.
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
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.
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.
@oxyc there's also a test Class which could be useful -- https://github.com/beetboxvm/beetbox/blob/master/tests/PluginTest.php
- [x] approval
- [x] update docs
- [ ] should we include tests for the composer plugin?
- [x] way more readable logic
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 - 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?
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
.
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.
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](https://cloud.githubusercontent.com/assets/302736/25318648/d0cbcc0a-2858-11e7-8f18-46447eae90d8.png)
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...
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
Require Drupal VM as a Composer development dependency.
composer require --dev geerlingguy/drupal-vm
Create and configure a config.yml file, eg. in
vm/config.yml
.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 - 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...
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.
-
You download Drupal VM and build a quick site within Drupal VM using
vagrant up
. Point out the downsides. -
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 - 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.
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.
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 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 ;)
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?
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.
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 :(