Skip to content
This repository has been archived by the owner on Feb 13, 2023. It is now read-only.

Composer plugin? #1247

Closed
thom8 opened this issue Mar 23, 2017 · 18 comments
Closed

Composer plugin? #1247

thom8 opened this issue Mar 23, 2017 · 18 comments

Comments

@thom8
Copy link
Collaborator

thom8 commented Mar 23, 2017

Issue Type

  • Feature Idea

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
Copy link
Owner

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

@geerlingguy
Copy link
Owner

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
Copy link
Collaborator Author

thom8 commented Mar 23, 2017

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
Copy link
Collaborator Author

thom8 commented Mar 23, 2017

Also for a bit of background we're moving to only support composer installations to help with breaking backwards compatibility -- beetboxvm/beetbox#391

@oxyc
Copy link
Collaborator

oxyc commented Mar 24, 2017

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
Copy link
Contributor

frob commented May 25, 2017

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
Copy link
Collaborator

oxyc commented Jul 8, 2017

@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
Copy link
Collaborator

oxyc commented Jul 8, 2017

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 drupal-composer/drupal-project#214

@oxyc
Copy link
Collaborator

oxyc commented Jul 8, 2017

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
Copy link
Collaborator

oxyc commented Jul 8, 2017

drupal-composer/drupal-project#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
Copy link
Contributor

frob commented Aug 17, 2017

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

@geerlingguy
Copy link
Owner

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 geerlingguy added this to the 5.0.0 milestone Oct 7, 2017
@geerlingguy
Copy link
Owner

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
Copy link
Owner

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
Copy link
Collaborator

oxyc commented Nov 23, 2018

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 geerlingguy removed this from the 5.0.0 milestone Dec 28, 2018
@geerlingguy
Copy link
Owner

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
Copy link

stale bot commented Mar 6, 2020

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 stale bot added the stale label Mar 6, 2020
@stale
Copy link

stale bot commented Apr 5, 2020

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.

@stale stale bot closed this as completed Apr 5, 2020
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

4 participants