I'm trying to use drush to download Drupal core into a folder called "d6".. So I execute and get the following...

[nthompson@devbox html]$ drush pm-download --drupal-project-rename=d6 drupal
Project drupal (6.17) downloaded to /var/www/html/drupal.

Why is it not putting it in a d6 folder?
In my drushrc.php I have uncommented the line for CVS ($options['package-handler'] = 'cvs';), could this be the problem?

If I execute the above command verbosely, I get:

[nthompson@devbox html]$ drush pm-download --drupal-project-rename=d6 drupal --verbose
Downloading release history from http://updates.drupal.org/release-history/drupal/6.x
Executing: wget http://updates.drupal.org/release-history/drupal/6.x
Calling unlink(6.x)
Executing: svn info '/var/www/html/'
Downloading project drupal ...
Executing: cd /var/www/html/ ; cvs -z6 -dP -d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal checkout -r DRUPAL-6-17 -d drupal drupal
Checking out drupal was successful.
Project drupal (6.17) downloaded to /var/www/html/drupal.
Command dispatch complete

Any clues?

Comments

nicholasthompson’s picture

Version: All-versions-3.0 »

Fyi... I have the same problem with HEAD.

nicholasthompson’s picture

add1sun’s picture

I have the same problem with the latest version of HEAD. No worky.

greg.1.anderson’s picture

Assigned: Unassigned » greg.1.anderson
corbacho’s picture

Component: Code » PM (dl, en, up ...)

This is fixed in HEAD?

I'm using Drush All versions-3.0

drush dl drupal-6.x --drupal-project-rename=demo
It works as expected. It is renamed to "demo"

drush dl drupal-6.x --drupal-project-rename=demo --package-handler=cvs
It doesn't work the renaming.

It has different errors with verbose mode on:

First time I run it:

cvs
Downloading release history from http://updates.drupal.org/release-history/drupal/6.x           
Executing: svn info '/mnt/vbox-shared/'                                                     
Downloading project drupal ...                                                                  
Undefined variable: path cvs.inc:95                                                   
Executing: cd /mnt/vbox-shared/ ; cvs -z6 -dPC                                         
-d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal update -r DRUPAL-6-19 drupal drupal
Checking out drupal was successful.                                                          
Project drupal (6.19) downloaded to /mnt/vbox-shared/drupal.               [success]
Command dispatch complete 

Second time (now there is a existing directory called "drupal" )
Note that error path cvs.inc:95

cvs
Downloading release history from http://updates.drupal.org/release-history/drupal/6.x           
Executing: svn info '/mnt/vbox-shared/'                                                     
Downloading project drupal ...                                                                  
Undefined variable: path cvs.inc:95                                                   
Executing: cd /mnt/vbox-shared/ ; cvs -z6 -dPC                                         
-d:pserver:anonymous:anonymous@cvs.drupal.org:/cvs/drupal update -r DRUPAL-6-19 drupal drupal
Checking out drupal was successful.                                                          
Project drupal (6.19) downloaded to /mnt/vbox-shared/drupal.               [success]
Command dispatch complete 

The folder drupal is being cvs-updated when I didn't specify the folder "drupal",

jonhattan’s picture

Assigned: greg.1.anderson » jonhattan
Category: support » feature

--drupal-project-rename is only implemented for wget package handler. I'm working on cvs support.

jonhattan’s picture

Category: feature » bug

Fixing to make --drupal-project-rename is not hard but is a bit hackish as there're several overlapping things here I want to address altogether:

1/

$ drush dl drupal
Project drupal (6.19) downloaded to /tmp/drupal-6.19/.

$ drush dl drupal --package-handler=cvs
Project drupal (6.19) downloaded to /tmp/drupal.

both behaviours are legitimate but both package handlers doesn't behave the same, as it could be expected. In my opinion cvs should checkout to drupal-6.19, you can always do --drupal-project-rename if prefer just `drupal`.

2/
drush package handlers should fetch each project to a tmp folder (as drush_make), untar, and move to destination a/o relocate afterwards. Currently it does a chdir to destination directory and works on it (cwd if no --destination). It can be harmful. Example: /var/www/drupal-6.19 is my rocking website. I want to create another site:

$ cd /var/www
$ drush dl drupal --drupal-project-rename=otherdrupal

What happen is:

a) drupal-6.19 has been downloaded and untarred over the preexisting site.
b) my rocking website is now at /var/www/otherdrupal

Also, working on current folder makes cvs package handler to update instead of check out because it detects drupal/CVS does exist. This is what @corbacho reports in #5.

3/
pm-download should warn if the destination folder already exists. Currently it's just ignored. Same for relocation.
So we need to warn the user at two times: before download/checkout if --destination a/o --drupal-project-rename OR after download/co if no --destination and relocation wanted.

4/
package handlers should receive the path to tmp folder to work in and the definitive directory name (that is the project name or --drupal-project-rename). pm-download can move to destination or relocate after the package handler had made its work.

5/
Code in wget.inc seems to reveal there was an attempt to implement this:

drush dl cck zen --cck-project-rename=cck_renamed --zen-project-rename=zen_renamed

I see it of interest in some cases:

drush dl basic cck-6.x-3.x-dev --basic-project-rename=mynewtheme --cck-project-rename=cck-3.x-multigroup

---

I'll work on all of this but 5/. Some feedback specially on that point will be appreciated.

corbacho’s picture

awesome analysis Jonathan. Would be nice to get all that in drush, specially working on a temp folder to avoid surprises.

btw, the original issue was that --drupal-project-rename was not working with wget, but I think was fixed. I couldn't replicate that.

moshe weitzman’s picture

Looks like that rename code was last touched by Greg in #727436: drush dl drupal reports wrong directory path. The feature you describe in 5) looks quite exotic to me. I don't mind if we drop it. All the other points looks great.

moshe weitzman’s picture

Status: Active » Reviewed & tested by the community

@jonhattan is welcome to implement and commit #7

jonhattan’s picture

Status: Reviewed & tested by the community » Needs work

I'm working on this.

jonhattan’s picture

StatusFileSize
new43.78 KB

This is a big change. Attached patch to get feedback. I think still needs work on updatecode. not tested.

Summary of changes:
* + verbosity for drush help dl.
* supress trailing slashes.
* refactor destination lookup / relocation.
* --destination check moved to pm_download_validate().
* change hook adjust_download_destination to download_destination_alter. Simplified.
* do the work in a tmp directory and move to really final destination at the end (after "relocation").
* if destination exists ask the user to overwrite.
* some logic moved ot of wget package-handler (dodgy name, drupal-project-rename).
* handle edge cases as dl translation or profile variant core from cvs.
* fix #909008: Project drupal contains 39 cores.

moshe weitzman’s picture

I took a look at the code and that looks good to me. Nice code comments and sensible flow. Should we test pm-download now or does that untested too?

jonhattan’s picture

Status: Needs work » Needs review
StatusFileSize
new47.16 KB

pm-download was ready to test. pm-updatecode doesn't seem to be affected by this changes.

I tested pm-download even more and found some glitches:
* install profiles: by default we were downloading the core variant even if in a drupal root. So I've put some validation.
* translations. It worked fine before this patch. Now that we work on a tmp dir it is a special case.

sun’s picture

Overall, this patch looks very good. The functional changes really make sense.

+++ drush.api.php	10 Oct 2010 18:00:55 -0000
@@ -165,11 +165,13 @@
+ * ¶

Minor: Trailing white-space (also elsewhere in this patch).

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -77,7 +77,14 @@
+If a destination is not explicitly provided, it will defaults to a directory based on the project type:

"If no destination is provided, then it depends on the project type:"

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -192,10 +199,10 @@
-      'drush dl' => 'Download latest version of Drupal core.',
...
+      'drush dl' => 'Download latest stable version of Drupal core.',

I think the added "stable" is incorrect -- it downloads the latest recommended release, AFAIK.

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -206,7 +213,7 @@
-      '--drupal-project-rename' => 'Alternate name for "drupal" directory when downloading drupal project.',
+      '--drupal-project-rename' => 'Alternate name for "drupal-x.y" directory when downloading Drupal project. If this option is present but empty, the alternate name will be "drupal".',

The second sentence can be shortened to

'Defaults to "drupal".'

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -994,28 +1001,26 @@
-    @drush_op('mkdir', $destination, 0777, TRUE);
+    drush_mkdir($destination);

Circumvents the --simulate flag.

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -1302,203 +1291,296 @@
+ * Implement drush_COMMAND_init().
...
+ * Implementation of drush_COMMAND_validate().

Should be

"Implements [hook-name]."

(i.e., third-person form)

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -1302,203 +1291,296 @@
+ * pm-download can operate on any bootstrap level but it's preferred to work
+ * on highests available. So here we do.

"highests" looks odd. "So here we do." can be removed. Also, it would be good to document why this is preferred.

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -1302,203 +1291,296 @@
+  // Validate a/o enforce --variant to a sane default.

"a/o" is a very uncommon abbreviation, and in general, an "and/or" construct usually means that something can be expressed in a simpler way. E.g.,

"Validate --variant or enforce a sane default."

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -1302,203 +1291,296 @@
+    // Get release history. Don't rely on UPDATE_DEFAULT_URL since perhaps we are not fully bootstrapped.

All code comments should wrap at 80 chars. See http://drupal.org/node/1354 for documentation standards. (also many times elsewhere in this patch)

+++ commands/pm/pm.drush.inc	10 Oct 2010 18:00:56 -0000
@@ -1579,3 +1661,73 @@
+  // Nothing to say about translations or theme engines.
+  else {
+  }
+}

That looks a bit odd. It's better to move the information about the special case into the phpDoc description of the function.

Powered by Dreditor.

jonhattan’s picture

StatusFileSize
new51.14 KB

Much more testing here. New patch with fixes for updatecode and some corrections pointed by sun. Note: --simulate is not circumvented as this command finally relies on drush_shell_exec that is --simulate aware.

Will take a coffee, review again and commit this in some minutes.

jonhattan’s picture

Status: Needs review » Fixed

committed.

sun’s picture

Status: Fixed » Needs work
+++ commands/pm/updatecode.pm.inc	12 Oct 2010 11:48:55 -0000
@@ -226,6 +229,9 @@
 function _pm_update_move_files($src_dir, $dest_dir, $skip_list, $remove_conflicts = TRUE) {
+  print "_pm_update_move_files($src_dir, $dest_dir \n";
+  var_dump($skip_list);
+  var_dump($remove_conflicts);

ugh :)

Powered by Dreditor.

jonhattan’s picture

Status: Needs work » Fixed
corbacho’s picture

Thanks for this patch jonhattan. Good work. I tested --drupal-project-rename with --package-handler=cvs and worked smoothly. Nice message:

Project drupal contains: [success]
- 1 profile: default
- 6 themes: pushbutton, minnelli, garland, marvin, chameleon,
bluemarine
- 33 modules: user, upload, ...

How was managing drupal before drush? :)

Status: Fixed » Closed (fixed)

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