geerlingguy / drupal-vm

A VM for Drupal development
https://www.drupalvm.com/
MIT License
1.38k stars 644 forks source link

Composer plugin? #1247

Closed thom8 closed 4 years ago

thom8 commented 7 years ago

Issue Type

Summary

@geerlingguy have you considered making DVM a composer plugin so you don't need a delegating Vagrantfile?

see https://github.com/beetboxvm/beetbox/blob/master/composer/src/Plugin.php

This creates/updates a Vagrantfile during a composer install

Then you'd just ignore the Vagrantfile in .gitignore

geerlingguy commented 7 years ago

Interesting... definitely something worth considering. It would take one extra step out of integrating Drupal VM straight into a project.

geerlingguy commented 7 years ago

The main concern I'd have is how easy it would be to allow customizations per project (e.g. if you want your config_dir to be vm or box or drupalvm or whatever).

thom8 commented 7 years ago

Completely understand, which is why it would be difficult to jump to PR stage.

We've also been testing the concept of adding config directly to composer.json -- https://github.com/thom8/drupal8-vagrant/blob/master/composer.json#L27

thom8 commented 7 years ago

Also for a bit of background we're moving to only support composer installations to help with breaking backwards compatibility -- https://github.com/beetboxvm/beetbox/issues/391

oxyc commented 7 years ago

So with beetbox as an example, we could do something like this to support specifying the config_dir in composer.json as well.

Edit: Nevermind, this needs more work as the host_drupalvm_dir needs to be resolved for two setups. Unless the composer plugin scaffolds the current example delegating Vagrantfile rather than Drupal VMs real one (for sanity this might actually be the best option if we want to support composer as well as downloading drupal-vm to a subdir/submodule).

diff --git a/Vagrantfile b/Vagrantfile
index 386e9d0..46fb62e 100644
--- a/Vagrantfile
+++ b/Vagrantfile
@@ -14,6 +14,16 @@ guest_config_dir = ENV['DRUPALVM_CONFIG_DIR'] ? "/vagrant/#{ENV['DRUPALVM_CONFIG

 drupalvm_env = ENV['DRUPALVM_ENV'] || 'vagrant'

+if File.exist?("#{host_project_dir}/composer.json")
+  require 'json'
+  composer_conf = JSON.parse(File.read("#{host_project_dir}/composer.json"))
+  config_dir = composer_conf['extra']['drupalvm']['config_dir'] rescue nil
+  if config_dir.is_a?(String)
+    host_config_dir = "#{host_project_dir}/#{config_dir}"
+    guest_config_dir = "/vagrant/#{config_dir}"
+  end
+end
+
 # Cross-platform way of finding an executable in the $PATH.
 def which(cmd)
   exts = ENV['PATHEXT'] ? ENV['PATHEXT'].split(';') : ['']
frob commented 7 years ago

We could use environment variables to allow users to create the VagrantFile with certain attributes.

composer install -- --config_dir="box"

Maybe not the most friendly solution, passing stuff like that is a standard way to run scripts in Composer. I cannot say I have ever seen that done on install before.

oxyc commented 7 years ago

@frob I liked that idea, would have been cool to be able to do something like:

composer require --dev geerlingguy/drupal-vm -- --config-dir=vm --docroot=web --drupalvm_webserver=nginx

Unfortunately it seems this only works for run-script :/

oxyc commented 7 years ago

I think it would be so much better if users didn't need to create their config.yml when using Drupal VM as a composer dependency. Here's a proof of concept (the open PR #1256 + config.yml scaffolding patch.

You can test it using these steps:

  1. Scaffold a project using drupal-project

    composer create-project drupal-composer/drupal-project:8.x-dev my-project --stability dev --no-interaction
    cd my-project
  2. Add my fork as a repository (for testing)

    "repositories": [
        {
            "type": "vcs",
            "url": "https://github.com/oxyc/drupal-vm.git"
        }
    ]
  3. Optional tasks, mostly useful for boilerplate projects (eg drupal-project) to pre-configure the required values.

    # Configure the location of the `config.yml` (not required, defaults to the root of the project)
    composer config extra.drupalvm.config_dir vm
    
    # Specify the docroot (not required with drupal-project as this defaults to `web`)
    composer config extra.drupalvm.docroot web
  4. Require Drupal VM which adds/updates the Vagrantfile and scaffolds a config.yml unless it already exists in config_dir.

    composer require --dev geerlingguy/drupal-vm:dev-composer-demo
  5. Build the VM

    vagrant up

If this would be merged the steps would be:

composer create-project drupal-composer/drupal-project:8.x-dev my-project --stability dev --no-interaction
cd my-project
composer require --dev geerlingguy/drupal-vm
vagrant up

Also pinging https://github.com/drupal-composer/drupal-project/pull/214

oxyc commented 7 years ago

One issue is that this can quickly move too far into composer territory which we don't want to. It should only support the basics and then users can use drupal-vm-cli or other dedicated tools to configure more.

The scaffolded config should only have so much that the setup works out of the box. If a user wants to customize something, they'll need to read the docs.

oxyc commented 7 years ago

https://github.com/drupal-composer/drupal-project/pull/180 uses the drupal-web-dir composer variable, if this could be agreed upon we could just re-use that rather than have our own.

frob commented 7 years ago

I think it makes sense to use what drupal-project uses.

geerlingguy commented 7 years ago

Going to start working on going this route for the 5.x releases. Thanks everyone for the help with this so far! Follow along with #1247 and #1256 to follow progress.

geerlingguy commented 6 years ago

Copied from the PR that corresponds to this ticket:

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 commented 6 years ago

Leaving this issue open for now... but as in the real world, I see many fewer sites being built with Composer than I'd originally imagined still (definitely much less than 50%), I'm going to keep Drupal VM as-is, and not convert to a plugin—at least not yet. I'd rather keep the majority user experience optimized for now.

oxyc commented 6 years ago

For slightly easier docs on this we could put it in a separate package too.

Just copied the PR over to a new repo to try and it works fine except that rather than copying Drupal VMs Vagrantfile it copies over the delegating Vagrantfile.

https://github.com/oxyc/drupal-vm-composer

Add the repo as a vcs repository

   "repositories": [
     { "type": "vcs", "url": "https://github.com/oxyc/drupal-vm-composer.git" }
   ],
composer require oxyc/drupal-vm-composer @dev
geerlingguy commented 5 years ago

Popping off the 5.0.0 milestone for now. I like your suggestion, but for now, will just keep the status quo. I'm going to use 5.0.0 to update defaults to PHP 7.2, Ubuntu 18.04.

stale[bot] commented 4 years ago

This issue has been marked 'stale' due to lack of recent activity. If there is no further activity, the issue will be closed in another 30 days. Thank you for your contribution!

Please read this blog post to see the reasons why I mark issues as stale.

stale[bot] commented 4 years ago

This issue has been closed due to inactivity. If you feel this is in error, please reopen the issue or file a new issue with the relevant details.