Running on Vista. Installed latest drush as of today. dl works fine and cvs works fine, but when I set the package handler for dl to use cvs, it fails at the checkout:

C:\wamp\www\drupal6>drush -v dl custom_search
Initialized Drupal 6.16 root directory at c:/wamp/www/drupal6           [notice]

Initialized Drupal site kindredcocktails at sites/kindredcocktails      [notice]

Downloading release history from                                        [notice]

http://updates.drupal.org/release-history/custom_search/6.x
Executing: svn info "sites/all/modules/"                                [notice]

Downloading project custom_search ...                                   [notice]

Executing: cd sites/all/modules/ ; cvs -z6 -dP                          [notice]

-d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal-contrib
checkout -r DRUPAL-6--1-2 -d custom_search
contributions/modules/custom_search
Unable to checkout custom_search from cvs.drupal.org.                    [error]

Unable to check out custom_search to sites/all/modules/custom_search     [error]

from cvs.drupal.org

My drushrc.php has:

<?php
// Specify a particular multisite.
$options['l'] = 'kindredcocktails';
// Specify your Drupal core base directory (useful if you use symlinks).
$options['r'] = 'c:/wamp/www/drupal6';// Specify CVS for checkouts
$options['package-handler'] = 'cvs';
$command_specific['dl'] = array('destination' => 'sites/all/modules');?>

I assume the odd carriage returns in the output are just word-wrapping. Otherwise, I don't see what's wrong. If I enter the same cvs command (without using drush), it works fine. I can also use my CVS GUI (TortoiseCVS) without any problem. I am new at drush and cvs, so I'm assuming this is a support issue and not a bug.

CommentFileSizeAuthor
#10 cd-and-exec.patch4.66 KBgreg.1.anderson

Comments

greg.1.anderson’s picture

greg.1.anderson commented

I'm not sure about your cvs problem, but as an unrelated note, you're going to want to remove that 'destination' setting for dl in your drushrc.php. Drush will already download modules to the location you specified. If you set the destination like this, then drush will also download themes and whatnot to the modules directory -- that is, you are overriding the usual algorithms drush uses to set the destination if you specify it explicitly.

I was able to dl custom_search using the cvs package handler; perhaps this is an odd Windows bug? I'm not sure what to suggest if the commands drush is emitting work fine when you enter them on the commandline...

danchadwick’s picture

danchadwick commented

@greg: thanks for the destination tip. Must have been a hold-over from earlier struggling with a Drupal multi-site installation.

More info: Running with the '-d' option gives:
The filename, directory name, or volume label syntax is incorrect.
Immediately after echoing the cvs command. What's odd is that if I enter this command manually, it works OK.

danchadwick’s picture

danchadwick commented

Found the problem. Another windows / unix thing. In cvs.inc, there is a hard-coded 'cd' command, and the path has forward slashes in it. Elsewhere the code uses chdir, which apparently can take either forward or backward slashes (I assume, since a) it works and b) I see examples on-line with backslashes).

So, the fix is at around line 121 of cvs.inc:
chdir ($checkout_path);
if (!drush_shell_exec('cvs ' . implode(' ', $cvsparts))) {
instead of:
# if (!drush_shell_exec('cd ' . $checkout_path . ' ; cvs ' . implode(' ', $cvsparts))) {

If a maintainer could test this and incorporate it, I'd appreciate it.

Many thanks to those who've made drush possible. I'm still learning all it can do.

danchadwick’s picture

danchadwick commented
Category: support » bug

doh.

dman’s picture

dman commented

Hm. Yeah, that would be a problem.
chdir will affect the working PHP process, whereas that inline cd affects only the exec shell in that line.

Easy but cumbersome way could be to

$cwd = getcwd();
drush_shell_exec(cvs ' . implode(' ', $cvsparts));
chdir($cwd)

I guess.
But there may also be an extra argument to give to cvs to define the destination folder better.

edited to correct func names

greg.1.anderson’s picture

greg.1.anderson commented

I like dman's fix best.

danchadwick’s picture

danchadwick commented

OK. I tested the following and it seems to work fine in Windows. Maybe we could get this tested on other platforms toward the goal of getting this committed?

$cwd = getcwd();
chdir($checkout_path);
$cvs_worked = drush_shell_exec('cvs ' . implode(' ', $cvsparts));
chdir($cwd);
if (!$cvs_worked) {
# if (!drush_shell_exec('cd ' . $checkout_path . ' ; cvs ' . implode(' ', $cvsparts))) {

moshe weitzman’s picture

moshe weitzman commented

Anyone up for testing this? Maybe jonhattan? We need someone to actively maintain the pm commands. They are super popular.

greg.1.anderson’s picture

greg.1.anderson commented
Assigned: Unassigned » greg.1.anderson
Status: Active » Needs review

This bug was kind of invisible in that it was in the 'active' state. I can review it.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Needs review » Reviewed & tested by the community
StatusFileSize
new cd-and-exec.patch4.66 KB

The above worked for me on Linux. I cleaned it up and applied the same technique to two other locations in the following patch. Anyone want to review it before I commit?

moshe weitzman’s picture

moshe weitzman commented

code looks good to me.

greg.1.anderson’s picture

greg.1.anderson commented
Status: Reviewed & tested by the community » Fixed

Committed; thanks.

Status: Fixed » Closed (fixed)

Automatically closed -- issue fixed for 2 weeks with no activity.