Skip to content

Conversation

weitzman
Copy link
Contributor

  1. Add a new installer-path to composer.json so that Drush projects installed via Composer are automatically saved to the correct directory (drush/commands).
  2. Add a new Drush top level directory and 3 subdirectories for storing project-specific config, aliases, and commands.
  3. During post-install, create a wrapper script called 'dr' which launches drush and points to our new subdirs and skips loading from global locations. I didn't call it 'drush' because the top level directory is already called that and unix disallows a file and directory having same name. An alternative to dr could be drush.sh or dru.sh

1. Add a new installer-path to composer.json so that drush projects installed via composer are automatically saved to the correct directory.

2. Add a new drush top level directory and 3. subdirectories for storing project specific config, aliases, and commands.

3. During post-install, create a wrapper script called 'dr' which launches drush and points to our new subdirs and skips loading from global locations.
drush/README.md Outdated
Copy link
Collaborator

Choose a reason for hiding this comment

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

packages.drush.org should probably be added to the "repositories" section too.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

AFAIK, packages.drush.org is just query tool for packagist.org. So i'm thinking it does not need to be listed in 'repositories'.

Copy link
Collaborator

Choose a reason for hiding this comment

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

Ah yes, you are correct.

I also notice that packagist.drupal-composer.org (or, more accurately, the parser that builds component info from drupal.org projects) and packagist.org (which is to say, what everyone seems to be entering into their composer.json files) are both standardized on the type 'drupal-drush' now, which is good. For a while, some folks were using 'drush-extension', but that usage has thankfully fallen into disuse.

@greg-1-anderson
Copy link
Collaborator

What is the intended workflow for the 'dr' script in the project root? Are you envisioning that teams will add their project root to their $PATH? It seems that usually, executable scripts should go in vendor/bin, not the project root. Could we do that here? That would be one less path for folks to maintain -- presuming that they put vendor/bin for their active project in their current PATH, anyway.

Another option would be to set $options['drush-script'] to point to the site-local Drush (in vendor/bin/drush). If you do this, then the Drush on your $PATH will launch the Drush indicated by drush-script. This only works on Drush 7+, but it is a little safer, because with a 'dr' script you can accidentally get pollution from the global namespace if you accidentally run 'drush' instead of 'dr'. This can lead to mysterious runtime errors if classes from Drush extensions conflict with classes in Drupal core, or Drush extensions in the project, so I think that standardizing on $options['drush-script']might be better.

@weitzman
Copy link
Contributor Author

Another option would be to set $options['drush-script']

In what file would you set that? If it were in project specific config then you'd already have pointed to the right config by then. Regardless, I'm still wary of "unnecessary" redispatches between two full copies of Drush. See my comment at drush-ops/drush#1138 (comment)

I moved the dr script to vendor/bin

@weitzman
Copy link
Contributor Author

  1. Perhaps Greg was thinking that the config dir was inside Drupal root so that alias that points to proper root would redispatch to local drush. I put the drush subdirs outside of Drupal.
  2. I thought about adding a --root=web to vendir/bin/dr but wouldn't that take precedence over a 'root' in a site alias? That would be bad if you could only use dr on local drupal project.

@derhasi
Copy link
Member

derhasi commented Apr 15, 2015

I never worked with a local drush before and I wasn't aware of that local-mode either. I like that idea.

As I understand, with the current approach the team would need to call ./vendor/bin/dr (from the root) to use drush for the project. Calling global drush would still be possible and there currently is no way to avoid that.
I guess when I use a local drush, I would want my team always to use that one. So from that perspective a redispatch would be helpful to avoid calls to global drush. But I guess without making changes to drush itself, that would not be possible. Do you see a way to achieve that?

@greg-1-anderson
Copy link
Collaborator

If you set $options['drush-script'] in a drushrc.php file in a location parsed by Drush (e.g. ROOT/drush or ROOT/sites/all/drush), then your global Drush will redispatch to this local Drush. If you are already running the local Drush, the redispatch is ignored, of course. This code is already in Drush.

The local redispatch works the same as a remote redispatch, which has been in use for a long time. The biggest problem with the redispatch is that when it happens, stderr is redirected to stdout. There has been a feature request to fix this in backend invoke for a long time. Perhaps we'll do this when we switch backend invoke over to use the Symfony process component.

I am still wary of --local turning off all of your personal drushrc.php and alias files. It hardly matters that --web takes precedence over the contents of the alias file when Drush isn't reading your alias files any longer. The workaround to that would be to look for a ~/.drush/local-customizations.drushrc.php, and include that if found. (A better name for this file could be picked, of course.) This could be done either in Drush (with an additional code change), by adding a pattern that is still included even with --local, or it could be done manually in all of the drushrc.php files in Drupal projects that use the local Drush feature.

I wish this were all more automatic, but I'm not sure how --local can differentiate between helpful global personalization from options that might interfere with the local settings, so I'm not fully sure what to recommend. Hopefully we can come up with a good story to tell folks by DrupalCon LA.

@weitzman
Copy link
Contributor Author

I am still wary of --local turning off all of your personal drushrc.php and alias files. It hardly matters that --web takes precedence over the contents of the alias file when Drush isn't reading your alias files any longer.

But Drush is reading project specific alias files from the new drush/site-aliases folder.

The new dr script could add ~/.drush/local-customizations.drushrc.php to its --config value. If not found, would be harmless.

@weitzman
Copy link
Contributor Author

If someone used the global Drush on this site they would not get the project specific config, aliases, and commands. If thats still undesireable, then this project could do one of:

  1. Add $options['drush-script'] per @greg-1-anderson suggestion above to cause redispatch to local Drush
  2. In that same options file, return an error if DRUSH_BASE_BATH is not same as project local drush.

Not perfect, but just tossing out ideas.

@greg-1-anderson
Copy link
Collaborator

Why don't you put the project-specific config and aliases in ROOT/drush? Then they would be available for both global and local Drush.

I am also thinking that --local is too local. I'll make a PR in the Drush queue to discuss how I think it should actually work, though.

@greg-1-anderson
Copy link
Collaborator

I was testing site-local Drush today, and my experience was that Drush dies consistently in $kernel->terminate() when Drush is in the same autoloader as Drupal, but works fine when they have separate autoloaders. It was seemingly working before only because of the bug where terminate was not actually being called, which we fixed a short while back. I investigated the issue briefly, and it seems that autoload.php is being included correctly, and Drush seems to be passing the autoloader to Drupal correctly. I didn't manage to track down the actual cause in the time I had--just mentioning it here in case anyone is investigating the alternatives.

On this note, this morning, Moshe and I were discussing the options, and it seemed to us that the best thing was to not have the wrapper script, and instead just contrive a mechanism to manage your PATH easily, so folks could use something akin to the use command to target the site-local Drush. We should probably make the site-local Drush implicitly set --root if it detects that its autoloader is in the vendor directory of some Drupal site (and no other site is selected, e.g. by an alias).

Of course, the redispatch thing will also work for folks who want to use it; just set drush-script in the drushrc.php file of any Drupal site with a site-local Drush. The bug I mention above is a problem regardless of how you target your site-local Drush, though.

…re under web/ and are automatically loaded by Drush.
@weitzman
Copy link
Contributor Author

Added another commit which implements more minimalist approach as Greg said above. No more wrapper script needed. People can use vendor/bin/drush directly.

@greg-1-anderson
Copy link
Collaborator

Yeah, I like this. PATH management and other alterations mentioned above to happen in the Drush queue.

@greg-1-anderson
Copy link
Collaborator

For discussion on managing your PATH via Drush, see drush-ops/drush#1343

@greg-1-anderson
Copy link
Collaborator

The site-local Drush bug in terminate() I mentioned above was a configuration problem, not a Drush bug. A while ago I had installed a Drush extension that was including its own autoload.php file, and this was causing Composer to provide old versions of some Symfony classes. An object was inheriting from an older version of its base class--a situation that the debugger was ill equipped to diagnose. Deleting the old autoload file (and the rogue Drush extension that depended on it) cleared up the problem.

@greg-1-anderson
Copy link
Collaborator

Once we are done here, we should probably make another matching PR in https://github.com/composer/installers

@weitzman
Copy link
Contributor Author

This PR is ready for review/merge, in case that was not clear.

@weitzman
Copy link
Contributor Author

weitzman commented May 4, 2015

Anything else needed here, @webflo ?

@weitzman
Copy link
Contributor Author

weitzman commented May 4, 2015

To test this out, you can add any of the packages listed at http://packages.drush.org/drackage/browse (this site queries packagist.org for its data).

webflo added a commit that referenced this pull request May 4, 2015
Standardize Drush usage for teams
@webflo webflo merged commit 939e0ad into drupal-composer:8.x May 4, 2015
@webflo
Copy link
Member

webflo commented May 4, 2015

Thanks, works like a charm!. @weitzman @greg-1-anderson Please open another PR for the dr script.

@derhasi derhasi mentioned this pull request Sep 3, 2015
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants