Make Drupal VM into a Composer plugin that scaffolds a Vagrantfile #1256

oxyc · 2017-03-24T18:01:44Z

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...

geerlingguy · 2017-03-24T18:07:27Z

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

oxyc · 2017-03-24T19:01:24Z

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.

geerlingguy · 2017-03-24T19:28:46Z

break all existing composer dependency setups

😩

geerlingguy · 2017-03-24T19:30:12Z

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.

oxyc · 2017-03-24T19:35:46Z

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 · 2017-03-25T01:28:46Z

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 · 2017-03-25T01:39:01Z

Vagrantfile

+  if Dir.exist?("#{host_drupalvm_dir}/vendor/geerlingguy/drupal-vm")
+    host_drupalvm_dir = "#{host_project_dir}/vendor/geerlingguy/drupal-vm"
+    guest_drupalvm_dir = "#{guest_project_dir}/vendor/geerlingguy/drupal-vm"
+  end


One annoying thing here is that this wouldn't support forks... any ideas?

thom8 · 2017-03-26T01:17:07Z

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 - beetboxvm/beetbox#391 (comment)

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

thom8 · 2017-03-26T01:24:13Z

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

oxyc · 2017-04-11T04:51:54Z

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

oxyc · 2017-04-23T05:56:42Z

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?

#1332

geerlingguy · 2017-04-23T21:29:33Z

@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?

oxyc · 2017-04-23T21:35:33Z

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.

geerlingguy · 2017-04-23T21:43:58Z

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.

oxyc · 2017-04-24T00:13:28Z

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?

oxyc · 2017-04-24T17:25:19Z

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 · 2017-04-24T17:47:57Z

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'

geerlingguy · 2017-04-24T18:00:57Z

@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...

oxyc · 2017-04-24T18:01:02Z

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.

geerlingguy · 2017-04-24T18:02:15Z

@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.

oxyc · 2017-04-24T18:04:32Z

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 · 2017-04-24T20:42:06Z

docs/deployment/composer.md

@@ -1,5 +1,7 @@
 Drupal VM is configured to use `composer create-project` to build a Drupal 8 codebase by default but supports building Drupal from a custom `composer.json` file as well.

+_Note that if you already have a project built using a `composer.json` file you should instead add [Drupal VM as a dependency](composer-dependency.md) of your project. The method described below will make it somewhat difficult to manage your `composer.json` together with Drupal VM as the file will be copied from `drupal_composer_path` into `drupal_composer_install_dir` as a secondary `composer.json`._


Are you okay with this, or maybe too scary? Honestly I think we should default drupal_composer_path to no in favor of Drupal VM as a dependency. I think we've seen a few issues due to this particular build setup being a bit non intuitive and I dont see much use case for it other than a faster version of drupal_build_composer_project.

Or well that's a bit odd too. Maybe it should be renamed drupal_composer_copy_path so it makes more sense as to why you need to set it to false for the composer dependency setup.

oxyc · 2017-04-26T01:39:00Z

Vagrantfile

+  cconfig = load_composer_config("#{host_project_dir}/composer.json")
+
+  # If Drupal VM is a Composer dependency set the correct path.
+  drupalvm_path = "#{vendor_dir}/geerlingguy/drupal-vm"


vendor_dir being a function seems very unclear to me coming from PHP. Rubocop complains if I add unnecessary parenthesis though. Maybe I should drop that lint?

oxyc · 2017-04-26T01:41:02Z

Vagrantfile

@@ -3,17 +3,28 @@

 require './lib/drupalvm/vagrant'


Derp this fails now.

oxyc · 2017-04-26T02:13:54Z

Vagrantfile

+
+  # If Drupal VM is a Composer dependency set the correct path.
+  vendor_dir = composer_conf.fetch('extra', {}).fetch('vendor-dir', 'vendor')
+  drupalvm_path = "#{vendor_dir}/geerlingguy/drupal-vm"


One problem here. There are some Windows developers who might prefer to code entirely within the VM right? This is why I'm avoiding calls to composer without the which function which is now in the lib we dont have access to before figuring out host_drupalvm_path (d'oh). As I'm avoiding calling composer config to retrieve the vendor-dir path, this could theoretically be wrong if the user somehow set a custom vendor path using a different method than the vendor-dir extra-field.

Also this setup requires the user to run composer install to download Drupal VM. That's probably alright, I mean all window devs using this setup would have composer installed, right?

Could also allow the user to specify the path to Drupal VM in the composer.json. So that it doesnt have to reside within the vendor-dir. But I doubt anyone would need to use that.

drupalvm_path = cconfig['path'] || "#{vendor_dir}/geerlingguy/drupal-vm"

I like this better than depending on composer being available in the current session.

Also need to consider env vars -- https://getcomposer.org/doc/03-cli.md#composer-vendor-dir

There are some Windows developers who might prefer to code entirely within the VM right?

Yes. (And the occasional Mac or Linux user who likes the isolation.)

Also this setup requires the user to run composer install to download Drupal VM. That's probably alright, I mean all window devs using this setup would have composer installed, right?

Preferably, yes. But right now there are a lot of Windows devs who neither want nor are allowed to set up PHP and/or Composer on Windows... but it could be possible we could make that a requirement for 5.0.0+...

But right now there are a lot of Windows devs who neither want nor are allowed to set up PHP and/or Composer on Windows.

Would these devs still be using Drupal VM as a composer dependency?

There's also the possibility that someone commits all dependencies and therefore not all Devs would need composer..

So for example a tech lead might use composer on behalf of the rest of the team and track all dependencies - https://www.codeenigma.com/build/blog/do-you-really-need-composer-production

I personally prefer an isolated build process but some might use DVM as composer dep without composer

… [ci skip]

…[ci skip]

oxyc · 2017-06-06T04:26:27Z

docs/deployment/composer-dependency.md

-If you're not using `composer` in your project you can still download  Drupal VM (or add it as a git submodule) to any subdirectory in your project. As an example let's name that directory `box/`.
+    # Set the location of the composer.json file and Drupal core.
+    drupal_composer_install_dir: "/var/www/drupalvm"
+    drupal_core_path: "{{ drupal_composer_install_dir }}/web"


Would it be too much to print this in the console if Drupal VM is added and no prior Vagrantfile exists? It would just make getting started so much easier if you wouldn't have to visit the docs... Or scaffold the vm/config.yml file in case a Vagrantfile is missing.

geerlingguy · 2018-04-10T03:30:06Z

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 · 2019-04-04T19:22:14Z

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 :(

oxyc commented Mar 25, 2017

View reviewed changes

oxyc force-pushed the composer-plugin branch 2 times, most recently from 8474fae to e37be08 Compare March 25, 2017 01:45

oxyc changed the title ~~[WIP] Add Composer plugin which scaffolds a Vagtranfile~~ [WIP] Add Composer plugin which scaffolds a Vagrantfile Mar 25, 2017

oxyc force-pushed the composer-plugin branch from e37be08 to e738adf Compare April 24, 2017 00:12

oxyc force-pushed the composer-plugin branch 3 times, most recently from c88d57e to df17e38 Compare April 24, 2017 16:04

oxyc force-pushed the composer-plugin branch from 2878514 to 52fa95a Compare April 24, 2017 20:37

oxyc commented Apr 24, 2017

View reviewed changes

oxyc commented Apr 26, 2017

View reviewed changes

Vagrantfile Outdated

@@ -3,17 +3,28 @@

require './lib/drupalvm/vagrant'

Copy link

Collaborator Author

oxyc Apr 26, 2017

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Derp this fails now.

oxyc commented Apr 26, 2017

View reviewed changes

geerlingguy added this to the 5.0.0 milestone Apr 26, 2017

thom8 mentioned this pull request Apr 26, 2017

Remove duplicated vagrant default config beetboxvm/beetbox#410

Merged

oxyc mentioned this pull request Apr 28, 2017

Fixes Error Message "Message: LoadError: cannot load such file -- ./l… #1347

Merged

geerlingguy mentioned this pull request Apr 29, 2017

Add instructions for running Drupal VM inside Docker #1206

Closed

oxyc added 10 commits June 2, 2017 00:10

Add Composer plugin which scaffolds a Vagtranfile

ed6d032

Write a Composer warning message when upgrading from < 5.0.0

20db9b5

Update docs [ci skip]

558b82e

Update scripts docs to apply to both methods of integrating Drupal VM…

2aa0620

… [ci skip]

Add a docs note about drupal_build_composer not always being optimal …

d2b6c9a

…[ci skip]

Add docs on how to patch Drupal VM [ci skip]

1c53c9b

Drop host composer dependency

0da90e2

Revert previous commit and fix the loading of Drupal VM ruby lib

9a8b183

Take into account COMPOSER_VENDOR_DIR env variable

5df9430

fix vendor-dir location in composer.json

25a7650

oxyc force-pushed the composer-plugin branch from 2011166 to 25a7650 Compare June 2, 2017 05:14

oxyc added 3 commits June 2, 2017 00:15

fix rebase miss

e27c427

typo

5592ed1

typo [ci skip]

fba5045

oxyc commented Jun 6, 2017

View reviewed changes

oxyc mentioned this pull request Jul 8, 2017

Composer plugin? #1247

Closed

geerlingguy added the planned label Oct 7, 2017

geerlingguy modified the milestones: 5.0.0, 6.0.0 Apr 4, 2019

geerlingguy modified the milestones: 6.0.0, 7.0.0 Jul 14, 2020

		@@ -1,5 +1,7 @@
		Drupal VM is configured to use `composer create-project` to build a Drupal 8 codebase by default but supports building Drupal from a custom `composer.json` file as well.

		_Note that if you already have a project built using a `composer.json` file you should instead add [Drupal VM as a dependency](composer-dependency.md) of your project. The method described below will make it somewhat difficult to manage your `composer.json` together with Drupal VM as the file will be copied from `drupal_composer_path` into `drupal_composer_install_dir` as a secondary `composer.json`._

Make Drupal VM into a Composer plugin that scaffolds a Vagrantfile #1256

Are you sure you want to change the base?

Make Drupal VM into a Composer plugin that scaffolds a Vagrantfile #1256

Conversation

oxyc commented Mar 24, 2017

geerlingguy commented Mar 24, 2017

oxyc commented Mar 24, 2017

geerlingguy commented Mar 24, 2017

geerlingguy commented Mar 24, 2017

oxyc commented Mar 24, 2017 • edited

oxyc commented Mar 25, 2017 • edited

oxyc Mar 25, 2017 • edited

Choose a reason for hiding this comment

thom8 commented Mar 26, 2017

thom8 commented Mar 26, 2017

oxyc commented Apr 11, 2017 • edited

oxyc commented Apr 23, 2017

geerlingguy commented Apr 23, 2017

oxyc commented Apr 23, 2017 • edited

geerlingguy commented Apr 23, 2017

oxyc commented Apr 24, 2017 • edited

oxyc commented Apr 24, 2017 • edited

oxyc commented Apr 24, 2017 • edited

geerlingguy commented Apr 24, 2017

oxyc commented Apr 24, 2017

geerlingguy commented Apr 24, 2017

oxyc commented Apr 24, 2017 • edited

oxyc Apr 24, 2017 • edited

Choose a reason for hiding this comment

oxyc Apr 24, 2017 • edited

Choose a reason for hiding this comment

oxyc Apr 26, 2017 • edited

Choose a reason for hiding this comment

oxyc Apr 26, 2017

Choose a reason for hiding this comment

oxyc Apr 26, 2017 • edited

Choose a reason for hiding this comment

oxyc Apr 26, 2017 • edited

Choose a reason for hiding this comment

thom8 Apr 26, 2017

Choose a reason for hiding this comment

geerlingguy Apr 26, 2017

Choose a reason for hiding this comment

oxyc Apr 26, 2017

Choose a reason for hiding this comment

thom8 Apr 26, 2017

Choose a reason for hiding this comment

thom8 Apr 26, 2017

Choose a reason for hiding this comment

oxyc Jun 6, 2017 • edited

Choose a reason for hiding this comment

geerlingguy commented Apr 10, 2018

geerlingguy commented Apr 4, 2019

oxyc commented Mar 24, 2017 •

edited

oxyc commented Mar 25, 2017 •

edited

oxyc Mar 25, 2017 •

edited

oxyc commented Apr 11, 2017 •

edited

oxyc commented Apr 23, 2017 •

edited

oxyc commented Apr 24, 2017 •

edited

oxyc commented Apr 24, 2017 •

edited

oxyc commented Apr 24, 2017 •

edited

oxyc commented Apr 24, 2017 •

edited

oxyc Apr 24, 2017 •

edited

oxyc Apr 24, 2017 •

edited

oxyc Apr 26, 2017 •

edited

oxyc Apr 26, 2017 •

edited

oxyc Apr 26, 2017 •

edited

oxyc Jun 6, 2017 •

edited