Add PSR-0 autoloader to drush

Looks good. Just revert the drush_register_file_for_deletion() change in favor of unlink().

Awesome stuff, I'm coming from the Aegir project, where we really need some kind of autoloader for our classes.

It would seem that because the way that this has been coded, the only classes that can be loaded by this autoloader would be in the Drush namespace, and if say, a contrib drush extension wanted to make use of classes, the way this code is impling it should be used would be to spin up another instance of the autoloader, registering its own namespace, rather than somehow adding its namespace to the drush autoloader? Is that the intention?

We're considering a major re-working of a lot of our code in Aegir's backend because we aren't extendible at all, contrib extensions must mix their code in with ours, ideally, we could just register with some central registry, and ideally this central registry would be provided by Drush.

Ideally, what would be awesome would be a hook that a commandfile could implement to register namespaces with drush, is there such a hook already? I guess a simpler solution would be to have a central way to register namespaces, and then commandfiles could just call it in their main body of code, which would get loaded as soon as Drush loads it.

Renaming/hacking the autoload class is a bad idea. That's just going to confuse people. It also makes proper attribution for licensing purposes harder. Just do as core is doing and include the code verbatim, in a properly namespaced folder, and wire it up with the code from bootstrap.inc.

I've been dying to get something implemented in Provision, so as you can have multiple autoloaders just fine, I've added the following:

/**
 * Return an instance of the provision autoloader.
 *
 * This will instiatate an instance if it needs to.
 */
function provision_autoload() {
  static $instance = NULL;

  if (is_null($instance)) {
    require_once(__DIR__ . '/Symfony/Component/ClassLoader/UniversalClassLoader.php');
    $instance = new UniversalClassLoader();
    // Activate the autoloader.
    $instance->register();
  }

  return $instance;
}

/**
 * Register a PECL style prefix with the provision autoloader.
 *
 * @param $prefix
 *   The class prefix to register.
 * @param $dir
 *   The directory to search for the classes in.
 * @param $prepend
 *   If the directory should be searched first for the classes in the given
 *   prefix, set this to TRUE, otherwise, the default, FALSE, is fine.
 */
function provision_autoload_register_prefix($prefix, $dir, $prepend = FALSE) {
  
  // Get any current directories set for this prefix.
  $current_prefixes = provision_autoload()->getPrefixes();
  if (isset($current_prefixes[$prefix])) {
    $dirs = $current_prefixes[$prefix];
  }
  else {
    $dirs = array();
  }
  
  // Now add the new one.
  if ($prepend) {
    array_unshift($dirs, $dir);
  }
  else {
    array_push($dirs, $dir);
  }
  
  // Set the prefixes.
  provision_autoload()->registerPrefix($prefix, $dirs);
}

// Add our prefix to the autoloader.
provision_autoload_register_prefix('Provision_', __DIR__);

Into our dev branch, note that we're using class prefixes and a slightly modified version of the autoloader so we can be PHP 5.2 compatible.

With the above code we can do things like this, in http.drush.inc:

/**
 * Implements hook_drush_init().
 */
function http_drush_init() {
  http_provision_register_autoload();
}

/**
 * Register our directory as a place to find provision classes.
 */
function http_provision_register_autoload() {
  static $loaded = FALSE;
  if (!$loaded) {
    $loaded = TRUE;
    provision_autoload_register_prefix('Provision_', __DIR__);
  }
}

Which is quite nice. Anyway, it would be cool if Drush provided some of this for free, but actually, it's not that much code, but it seems like a shame to include an autoloader but lock it away so no-one else can benefit from it.

Using the unmodified class loader would be a great idea per #4, but I am still concerned about conflicts with the Symfony universal class loader in Drupal core/includes. Drush may need to use the class loader before Drupal is bootstrapped, but if we require_once the same UniversalClassLoader.php from a different location, then php will die when the second class loader is included.

Post #1, the Drush bootstrap was rewritten to locate the location of the Drupal site that will be loaded very early in the bootstrap process. _drush_bootstrap_select_drupal_site() is called before every Drush bootstrap phase. So, the location of the Drupal root is set prior to drush_bootstrap_drush(), but might change again just prior to drush_bootstrap_drupal_root() if a config file changes --root, or if a site alias is selected by drush_sitealias_check_arg().

I propose that for Drush 6 we move the call to _drush_bootstrap_select_drupal_site() such that it is called once, somewhere in drush_bootstrap_drush() after drush_sitealias_check_arg(). At this point, we will consider that the selected Drupal site has been permanently fixed, We can then load the class loader directly from DRUPAL_ROOT . '/core/includes/Symfony/Component/ClassLoader/UniversalClassLoader.php', or load our own copy if no Drupal site has been selected, or if there is no selected site, or if Drupal does not include Symfony. I do still worry that we might not be able to find the class loader if it has been installed by the symfony module, since it does this:

  // Load Composer's autoloader to expose Symfony.
  $path = drupal_get_path('module', 'symfony');
  $loader = @include_once($path . '/vendor/.composer/autoload.php');

If we can reconcile this, then we should not need to rename the autoloader. Perhaps we should hop over to the symfony module issue queue, and suggest that Symfony should go in sites/all/libraries?

We discussed mapping all our namespaces in a composer.json in root of drush. one of us would then use composer tool to fetch all our classes and put then in /vendor. composer would also write out an autoloader for each listed class.

For contribs we would discussed registering a mount point in a /lib subdir beneath each commandfile.

Mark could explain it better.

Maybe I don't understand symfony well enough, but why do we need an autoloader for each class?

Is the problem with #8 that Drupal might use a different version of Symfony than Drush was using? That could be a problem. Is the composer tool renaming or renamespacing all of the vendor classes to make sure that doesn't happen? Not sure that I am clear on all of the implications yet.

Sorry, I meant one autoloader that knows how to load all the used by core drush.

Will Drush ever use the Symfony that is included with Drupal (c.f. symfony module in #8), or will we use our own private copy?

In Drupal 8 I don't see why it couldn't use the core class loader. If you want to maintain Drupal 7 compatibility, though, you'd need to include your own since Drupal 7 has no PSR-0 capable class loader.

In Drupal 7, there is a symfony module that will load Symfony. If running Drupal 8 or Drupal 7 w/ symfony, Drush must either use Drupal's copy, or rename / renamespace all of the Symfony classes. It seems to me that the former is the right approach, but I don't understand all of #9 yet.

c.f. http://drupal.org/project/composer

This should probably be in Drush core too.

Forgot the patch in #16?

Definitely is interesting... Composer and Drush Make do have similar paths, but their feature set is quite different. Using Composer's ClassLoader for this, as well as using it to bring in the additional dependencies makes almost perfect sense. Would we be packing the dependencies with Drush itself?

Very slick. The only flaw is interoperability with symfony in Drupal. See #8.

I think we need to do something like this:

  if (version_compare(PHP_VERSION, '5.3.0') < 0) {
    if (module_exists('symfony')) {
      $autoloadpath = drupal_get_path('module', 'symfony') . '/vendor/.composer/autoload.php';
    }
    else {
      $autoload_path = DRUSH_BASE_PATH . '/vendor/.composer/autoload.php';
    }
    if (file_exists($autoload_path)) {
      require_once $autoload_path;
    }
  }

We'll also need some code to check to see if we are running under Drupal 8. Hm, of course there is an obvious flaw in the code above; we will need to require_once the autoloader before we bootstrap Drupal, but drupal_get_path is not going to work until Drupal is bootstrapped. Postponing initializing the autoloader until after bootstrap would mean that we could not use any symfony features during bootstrap. Since we were thinking about using Symfony for the new context class, we have another chicken-and-the-egg problem, because we then need to initialize Symfony before we load our Drush configuration files, but Drush cannot figure out where Drupal is until it has loaded said configuration files, since they may have settings which affect site selection.

Right now, the only general-purpose solution I can think of it to add code to Drupal 8 and the Drupal 7 symfony module to check and see if the autoloader has been included. A better solution, I think would be to require PHP 5.3.0 or later for Drush-6, and restrict Symfony use to Drush-6 and later. If the autoloader is provided by php, then Composer will make sure that we do not have code conflicts for any other modules. Then we should interoperate with Drupal just fine.

My only remaining question about Symfony and Composer is, who tells Composer to load Drush's composer.json?

if (module_exists('symfony')) {
      $autoloadpath = drupal_get_path('module', 'symfony') . '/vendor/.composer/autoload.php';
    }

Not sure Drush should depend on the Symfony module since that's a Drupal module rather than a Drush component. It's possible for Drush to download and install its own dependencies via Composer during the Drush bootstrap. We could either ship composer.phar, or have it automatically lazy-load download it if composer.phar isn't found. If we do that, then we can install Drush's dependencies directly through the .phar file:

    // Autoload the Composer classes from composer.phar.
    if (include('phar://' . $composer . '/vendor/.composer/autoload.php')) {
      // Set up the application arguments to be passed to Composer.
      $_SERVER['argv'] = array($composer);
      foreach (func_get_args() as $arg) {
        $_SERVER['argv'][] = $arg;
      }
 
      // Create a new Composer application
      $app = new Composer\Console\Application();
      $app->run();
    }
    else {
      // Display an error message about the phar file not loading.
      drush_log(dt('Although Composer was downloaded, it was not processed correctly.'), 'error');
    }

What I'm suggesting here is....

1. Have Drush lazy-download composer.phar (or ship Drush with the Composer source)
2. When someone runs a Drush command, have Drush check for vendor/.composer/autoload.php, if it doesn't exist, then install them
3. Have happy fun dance time since we're now automatically downloading and install dependencies without changing the user experience

#21 would work just fine iff the symfony module and Drupal 8[*] were changed to also use include('phar://' . $composer . '/vendor/.composer/autoload.php') (or include_once). If Drush runs code per #21, and then Drupal comes along and trys to again load the autoloader per the code in the symfony module (see #8), then PHP will throw an error, because the same code is being loaded from two different paths. The different paths defeats _once, so you get duplicate function errors.

[*] Haven't looked at the D8 code. If it requires PHP 5.3, and does not load the autoloader, then perhaps there is no conflict there.

D8 requres PHP 5.3, and at the moment uses the Symfony ClassLoader for any namespaced code. Switching to Composer is under consideration: #1424924: Use Composer for updating Symfony components (without removing Symfony code from repo) and #1398772: Replace .info.yml with composer.json for extensions.

I don't think any changes are necessary for phar:// support, is there? It should Just Work(tm) in PHP 5.3.

The attached patch...

• Automatically installs the Drush dependencies if they're not available when Drush is run
• Ships composer.phar with Drush at lib/composer
• Exposes Composer via a drush composer command

Would be nice if we...

• Move to completely using vendor/pear-pear/Console_Table rather than lib/Console_Table
• Update the docs in lib directory saying it's for Drush's PSR-0 stuff
• Avoid using the phar file and use the Composer/Symfony sources directly
• Figure out a better place for the Composer to live since /lib would be switching to Drush's PSR-0 classes

Moved to Composer's provided Console_Table and updated the folder docs.

If we are going to manage Console_Table through Composer, it seems like we should do the same with httpserver also (I would be happy to figure out how to package it and post it to one of the supported repositories, if snagging direct from github is not Composer supported).

Great idea! Got both the Console_Table and the HTTPServer autoloaded via the Composer autoloader. Needed to switch from the PEAR downloader to SVN though, which actually seems speedier and more responsive.

I'm still concerned about this part:

+  $composer = DRUSH_BASE_PATH . '/vendor/composer/composer.phar';
+
+  // Autoload the Composer classes from composer.phar.
+  // @todo: Use the Composer source rather than the Phar file.
+  if (include('phar://' . $composer . '/vendor/.composer/autoload.php')) {

I expect if two components (Drush and Drupal) try to include autoload.php via two paths, then the second one to run will fail. Haven't had a chance to try it yet, but drush @site status should do the trick if @site is D8 -or- D7 w/ the symfony module enabled. Anyone try that yet? If it works, we're good.

n.b. Don't really know what's meant by 'the composer source'; perhaps that is the solution to this problem?

Composer like any other PHP project is, um, a bunch of PHP files. :-) It's compiled into a phar file (kind of like a Java JAR file) for convenience, but works either way. I presume the todo there is to replace the phar file with the original raw PHP files.

That said, I do not understand the rest of #28. What's going to break and why, exactly?

Suppose you have a file 'foo.php' that defines a single function a() that just prints "hello".

Then:

$ cp foo.php b
ga@nagai:~/tmp/phptest$ php -a
Interactive shell

php > include_once("foo.php");
php > print a();
hello
php > include_once("foo.php");
php > include_once("b/foo.php");
PHP Fatal error:  Cannot redeclare a() (previously declared in /home/ga/tmp/phptest/foo.php:4) in /home/ga/tmp/phptest/b/foo.php on line 5

The second call to include_once behaves correctly; foo.php is already included, so it is not included a second time. The third call to include_once fails; if you have a copy of foo.php at a different path ("b/foo.php"), include_once presumes that this must be a different foo, so it goes ahead and includes it again. You get an error, because the function a() is already included.

So, if we include_once(".../autoloader.php");, then we need to make sure that any other code that also calls include_once(".../autoloader.php") uses exactly the same path. Otherwise, php will throw 'cannot redeclare' errors.

Oh, I just tried it, and it works just fine. There is no problem here... because Symfony rocks, of course. The file autoloader.php defines no functions itself; it is designed to be included again and again, and therefore already has a check in place to insure that it does not bomb out on its second invocation. This only makes sense, as it would be kind of hard to resolve issues such as #28 / #30 if you could only include the autoloader once, because it needs to be possible for components to be able to 'bootstrap' symfony independently without getting in each other's way, or require some global at the application layer, etc.

The key bit from the autoloader looks like this:

if (!class_exists('Composer\\Autoload\\ClassLoader', false)) {
    require __DIR__.'/ClassLoader.php';
}

So, +1 for #27. It works great, and I see no reason why it would cause problems in Drush-5, either on its own, with Drupal 7 + symfony module, or Drupal 8 (although I did not try the last).

Another thing: this patch also commits Symfony code directly to the Drush project's repository; the license is GPL-compatible, although not GPL. Drupal policy says GPL-only here: http://drupal.org/node/66113

Perhaps this is changing, with Symfony going into Drupla 8 core?

I suggested in #15 that the composer drush command should probably be in core because the symfony module for D7 uses it to install symfony in Drupal. I imagined that other modules might also start doing the same thing. An alternative, though, is that we could submit modifications to the symfony module & elsewhere to automatically use composer (via direct php call, not a drush command) in their pm-enable pre-enable or post-enable hooks. With a little sample code, and a little more adoption, perhaps the composer drush command would not be needed very often. Then again, I don't really have a good answer for why folks thought that the composer drush command was needed at all, when composer already has a command line interface. Shrug.

Rob Loach thought a composer Drush command was needed. I disagree. :-)

Things that this patch addresses...

• Push the drush composer command to a post-issue. I didn't think a Drush composer command was needed, just thought it would be nice :-P.
• Use Pear's repository directly rather then Pear's SVN. Had to declare the "classmap" in "autoload" instead of the package itself.
• Has a /vendor/.gitignore to ignore the downloaded third-party libraries

Things that this patch does not address...

1. Simplifying drush_bootstrap_prepare_dependencies(). We're using the Composer API directly here, so I'm not sure how to make it easier.
2. Avoid calling chdir(). Composer uses the current working path by default. It would be nice to be able to change that. Maybe https://github.com/composer/composer/issues/55 ? I was trying "config": { "vendor-dir": "vendor" } without success. I created https://github.com/composer/composer/issues/503 so that we arn't blocked by this.
3. Instead of shipping Composer, we could have Drush download it for us during installation. Think that might be a viable option regarding the note about licencing?

Updated issue summary.

What happens when we decide we need a newer version of httpserver, for example. In current Drush, we will automatically download a new version. I don't see that this happens with this patch.

Do we want to say anything about composer in Drush's README.txt?

I'd like new issues opened for the Follow Up tasks in the Summary or we remove items that no longer need Follow-up.

I don't completely get why folks are so excited about Symfony. Not that it matters, but I think that code like below has very poor readability:

+      // Create the Composer Console IO mappings, telling Composer that we want
+      // to install Drush's dependencies.
+      $input = new Symfony\Component\Console\Input\ArrayInput(array('install'));
+      $output = new Symfony\Component\Console\Output\ConsoleOutput();
+      $helperset = new Symfony\Component\Console\Helper\HelperSet();
+      $io = new Composer\IO\ConsoleIO($input, $output, $helperset);
+
+      // Create the Composer and Installer objects using Drush's composer.json.
+      $composer = Composer\Factory::create($io, $path . '/composer.json');
+      $installer = Composer\Installer::create($io, $composer);

Good call, Moshe. We should cache the installed .json file somewhere, and run a Composer update when the hash is changed.

I agree that we should move to use statements in there. Do you think we should have this stuff in a composer.inc file or something just to help decouple it from the bootstrap?

Does php / Symfony have anything like Java's import statement? Seems that a lot of these features parallel Java language features; not all of these are my favorites either. But anyway, with imports, the above could become:

+      // Create the Composer Console IO mappings, telling Composer that we want
+      // to install Drush's dependencies.
+      $input = new ArrayInput(array('install'));
+      $output = new ConsoleOutput();
+      $helperset = new HelperSet();
+      $io = new Composer\IO\ConsoleIO($input, $output, $helperset);
+
+      // Create the Composer and Installer objects using Drush's composer.json.
+      $composer = Composer\Factory::create($io, $path . '/composer.json');
+      $installer = Composer\Installer::create($io, $composer);

I have to admit, even with this simplification I am not terribly excited by constructs like new ArrayInput(array('install')) just to pass an 'install'command to composer. :?

1. Moved to composer.inc to clean up the bootstrap
2. Has "use" statements to clean up the inline code
3. Saves the installed composer.json hash so that when composer.json is changed, the dependencies are re-processed

Do we want to say anything about composer in Drush's README.txt?

Maybe when we bring in the drush composer command. There isn't currently anything user-facing with this patch. Do you think we should make a note about using it to install the dependencies?

That's a great improvement. I don't think that we should ever do this, though:

+use Composer\Factory;

There are presumably a lot of classes that have Factories; new Factory(...); is just not explicit enough. I think that this should always be written as new Composer\Factory(...);. We could use use statements for everything else. (Coding standards for OO Drupal needed...)

To avoid class name conflicts, you can do:

use Composer/Factory as ComposerFactory;
// ...
$factory = new ComposerFactory(...);

But we don't have any other Factory classes anywhere. There arn't any other Factorys that we're working with in composer.inc. The use statements are going into Drupal 8. Are there any other third-party libraries that Drush makes use of that we could push into Composer?

Do you think that we should have Drush download Composer rather than shipping Composer.phar with it?

I don't want to bikeshed this discussion too much. Either use Composer\Factory as ComposerFactory or new Composer\Factory(...); would be fine. Drush should follow the OO coding conventions established by Drupal, but those are still formative right now. I think it is safe to say that if we start adopting OO, then we will have multiple factories in some files, and there should be a convention for keeping it readable. But that's my opinion; others may feel that $composer = Factory::create(...) is clear enough, as the variable name indicates what kind of Factory is intended.

As for downloading Composer rather than including it, this should also be decided per Drupal policy. If the policy is unclear, downloading is safer, as it conforms with the historic policy. Per #34, it seems that non-GPL code is being committed to d.o already, so I think it's probably safe for us to follow suit.

For my part, I am fine with committing #43 as-is, although I would sort of like to see one of the Composer\Factory alternatives for clarity. Up to Moshe.

Sounds good with me! I did it for the Installer too, just for consistency.

  $composer = Composer/Factory::create($io, $composerFile);
  $installer = Composer/Installer::create($io, $composer);

I feel a lot better about #47; thanks. The code works well, and correctly calls the installer when the composer.json file changes. The README could, perhaps, use some credits info for symfony / composer, but if we did that, it seems we should also put something in for the http server and console table.

Wait, I'm not sure that this is right. If I change composer.json, and then change it back (with Drush runs between each step), then I get this message:

Your lock file is out of sync with your composer.json, run "composer.phar update" to update dependencies

Is composer supposed to leave the composer.lock file sitting around? It seems like the lock file is supposed to allow composer to take care of updates automatically, but it takes a really long time to call the installer every single time Drush runs. Maybe we should run a composer update command instead of a composer install when the composer.json hash changes?

Ah, I see what the problem is. Instead of using the ArrayInput(), we should be interacting with the installer directly. This actually gives us more control over how the installation process works.

  // Set up the installer to install and update any dependencies.
  $installer
    ->setDryRun(FALSE) // Actually perform actions.
    ->setVerbose(TRUE) // Output messages more frequently.
    ->setPreferSource(TRUE) // Use package sources when possible.
    ->setInstallRecommends(TRUE) // Retrieve recommended packages.
    ->setInstallSuggests(TRUE) // Retrieve suggested packages too.
    ->setUpdate(TRUE); // Update any dependencies during installation.

Thanks for finding that one, Greg!

--binary

That works brilliantly. Nice to see the ArrayInput going away; the new code looks much much better.

If we used the Composer sources rather than composer.phar, do you think this could be committed?

I'm presuming you were talking with Moseh in IRC. I suppose it is less than ideal to have composer checked in to our repository; I don't think that checking in the source instead of the phar makes this aspect any better.

What if we ran curl -s http://getcomposer.org/installer | php instead? That's from the composer README file; seems that's better than using git, which might not be available (e.g. on Windows under DOS or Powershell). This would be pretty easy to do using drush_shell_cd_and_exec; that would take care of having composer embedded inside of Drush.

Also, the README also advises that php composer.phar self-update can be used to get a newer version of Composer. I wonder if there is a time when we should do this? Maybe we put it off until Drush needs a newer version of Composer to download something; then we could add a Composer version check + self-update at that time.

curl -s http://getcomposer.org/installer | php just retrieves composer.phar and saves it to the current working directory with permissions so you can run ./composer.phar update. If we were to have Drush download composer.phar rather than ship composer.phar with Drush, we'd do something similar to make sure the phar file was available.

I'm just trying to understand if there were any blockers here. Was wondering if using the Phar file was the problem. Using composer.phar directly does have its benefits, but if it's a blocker for getting this in, then we could use the source directly.

As I said in #54, I have no preference vis-a-vis committing composer.phar vs. committing composer sources to the repository. Committing a binary is a little dirtier than committing sources, but either way, you're still committing an external component, which in my mind is the greater problem.

If we're going to download rather than check in composer, I similarly don't think it matters if we download the phar file or the sources; downloading the phar seems better to me, if we go this route.

Which of these issues, if any, are blockers is up to Moshe.

Actually, running curl -s http://getcomposer.org/installer | php also ensures that php is configured correctly for composer to be able to run.

Adding the composer source to the drush repo definitely sounds like the wrong approach to me (would that even work? Doesn't composer itself use a composer.phar to get its own dependencies or is that my brutal misunderstanding of this repo https://github.com/composer/composer?).

Three possible methods:

1. Ship with composer.phar

This is what the patch suggests currently. Composer can still be updated, the patch is as simple as it could be.

2. Have Drush download composer.phar

The same as what the patch is now, except that Drush would facilitate downloading Composer if it wasn't available. This is one step deeper than shipping with composer.phar, and allows users to upgrade it. composer.phar would be .gitignored and downloaded when it wasn't there.

3. Ship with Composer sources

As Kat mentions, we'd also have to ship Composer's dependency sources too. Users could not update Composer and that would put the responsibility of updating Composer on Drush's shoulders, but it does mean that we wouldn't depend on the phar file.

I'm happy with whichever solution we go with.

We're committing symfony sources to drupal

This isn't a foregone conclusion - see #1424924: Use Composer for updating Symfony components (without removing Symfony code from repo) and #1475510: Remove external dependencies from the core repo and let Composer manage the dependencies instead.

@msonnabaum yes, that's why there are separate issues for them in the core issue queue. My point was that it doesn't make sense to justify adding composer/symfony source to drush on the basis that we're adding symfony source to core, because the latter is not necessarily true.

Here it is with the Composer source, including dependencies, instead of the .phar file.

Forgot --binary again.

To commit it, you use:

git clone --branch master http://git.drupal.org/project/drush.git drush-1316322
cd drush-1316322
wget http://drupal.org/files/1316322-sources-binary.patch
git apply 1316322-sources-binary.patch
git rm includes/.gitignore
git add .gitignore commands/runserver/runserver.drush.inc includes/bootstrap.inc includes/environment.inc lib/README.txt composer.json includes/composer.inc vendor
git commit -m "Add PSR-0 autoloading to Drush."
git push origin master

No, don't skip .phar use #1185360: Refactor code loading and support loading from PHAR archives

This is for Drush, not Drupal. The patch using the .phar file is at #51.

Here's patches with the phar file, or with the sources. Your choice!

Off topic: Forked Console_Table for Composer support for funsies.

Thought I'd do an update of the patch.

Talked with the Console_Table maintainer, and he agreed that having both PEAR and Composer support for Console_Table was a good idea. End result is that Drush can download Console_Table much faster as it doesn't have to go through PEAR.

+++ b/composer.json
@@ -0,0 +1,43 @@
+  "license": "GPLv3",

Eh? Not if it's hosted on d.o, it's not.

• GPL 2
• [""] for the HTTPServer classmap
• Used copy() and chmod() for the initial install stuff to make it as Windows-compatibile as possible.
• Removed the change directory requirement in the initial composer.phar install
• Renamed some of the functions so they live more appropriate live in the drush_composer namespace
• Removed an empty whitespace
• Ships with a vendor directory, with README.txt and a .gitignore which ignores everything but README.txt

tagging

So... I love this version of the patch - so clean and lovely.

When you run a drush command for the first time after updating, would it be possible to have some messaging to the effect that "Drush is performing a self-update, this may take a few seconds. This is necessary because your version of drush has been updated but its dependencies have not yet been updated."? Otherwise you're kinda left hanging there after just running drush --version or whatever, and then after a while it starts doing all this crazy shit, installing stuff, and well... it could cause some confusion I think.

Also, should we consider including the composer.lock file for the same reason we're considering including it for core, i.e. so that the composer.json could reference dev versions while ensuring that drush end users all have the exact same versions of the dependencies?

Anyhoo, would be awesome to get at least *some* of the Composer love consummated some time soon... :-)

Nice! Looks awesome. Not sure why vendor dir isn't in the root .gitignore anymore, but it's all good.

Not sure why I'd get errors when you didn't, but I think the "classmap": [ "" ] needs to be "classmap": [ "." ]. At least it did when I was playing with Composer before

I'm on the fence about Composer. IMO it is solving a problem that we don't have - downloading dependencies. Drush has very few dependencies, and has a solid system for fetching them. Composer does write an autoload.php which is nice but we could surely do that ourselves.

+++ b/commands/runserver/runserver.drush.inc
@@ -223,10 +192,7 @@ function drush_core_runserver($uri = NULL) {
+    // Include the Drush class that uses the autoloaded HTTPServer.
     require_once 'runserver-drupal.inc';

What's this about? Can't the autoloader handle all the require_once?

+++ b/composer.json
@@ -0,0 +1,43 @@
+  "name": "drupal/drush",

I'm not a fan of putting Drush in the Drupal namespace. Drush is its own project that happens to be hosted on drupal.org. Could we go with drush/drush?

+++ b/composer.json
@@ -0,0 +1,43 @@
+    "symfony/yaml": "2.1.*",

Drush core doesn't currently use YAML AFAIK. Why is this here?

+++ b/composer.json
@@ -0,0 +1,43 @@
+    "youngj/httpserver": "1.0.0"

httpserver is an optional component. many hosting environments can't run it anyway. could we put this in an optional section. if we did that, what would ramifications be?

+++ b/includes/composer.inc
@@ -0,0 +1,148 @@
+  static $composer = NULL;

Why would this function get called twice in same request? If it won't, please remove the static. We don't add statics "just in case". They have to prove merit first.

+++ b/includes/composer.inc
@@ -0,0 +1,148 @@
+    $composerHash = $hash ? md5_file($composerFile) : TRUE;

Is this really supposed to be TRUE at the end? The code flow here is not clear, IMO

+++ b/includes/composer.inc
@@ -0,0 +1,148 @@
+  // Composer uses the current working directory, so we'll mock it as the path
+  // as the current working directory.

Grammer is borked here.

+++ b/includes/composer.inc
@@ -0,0 +1,148 @@
+  // Run the installation process, ignoring any PHP warnings that may pop up.
+  $installed = @$installer->run();

Why are we suppressing warnings here? Is this not useful information?

+++ b/includes/composer.inc
@@ -0,0 +1,148 @@
+ * Download a temporary version of composer.phar and runs a Composer install.

s/runs/run

+++ b/includes/composer.inc
@@ -0,0 +1,148 @@
+  $composer_file = sys_get_temp_dir() . '/composer.phar';

Would be good to use drush_tempdir() or similar. Maybe we can move the composer run later in the bootstrap like _drush_bootstrap_drush_validate() where the console_table stuff is now.

+++ b/includes/composer.inc
@@ -0,0 +1,148 @@
+  copy('http://getcomposer.org/composer.phar', $composer_file);

No error handling?

+++ b/includes/composer.inc
@@ -0,0 +1,148 @@
+  return (bool)system($composer_file . ' install');

Drush has wrappers for system(). We could possibly use them if we move the composer run later (see above).

+++ b/includes/environment.inc
@@ -131,34 +131,6 @@ function drush_environment_lib() {
-  // Try using the PEAR installed version of Console_Table.
-  $tablefile = 'Console/Table.php';

We have lost the ability to reuse an installed PEAR Console_Table. This is a bit lame, considering that we currently ask folks to install via PEAR.

+++ b/vendor/README.txt
@@ -0,0 +1,7 @@
+This directory should be writable by users who runs Drush so that these

This writable requirement is going to be a problem for hosting firms who kindly provide drush to their clients.

Marked #1792550: Facilitate installing via Composer. as a duplicate.

This issue was marked closed (won't fix) because Drush has moved to Github.

If this feature is still desired, you may copy it to our Github project. For best results, create a Pull Request that has been updated for the master branch. Post a link here to the PR, and please also change the status of this issue to closed (duplicate).

Please ask support questions on Drupal Answers.

Updated issue summary.

XREF (slightly related)

- provision #2143043: Add composer dev for UML Diagrams and PHPUnit tests
- https://github.com/drush-ops/drush/pull/297 Add UML Diagram generation to tests.

Patch from #73 needs a big re-roll I guess.