drush dl attempts download to default when nothing specified
mojzis - April 14, 2009 - 22:34
| Project: | Drush |
| Version: | All-Versions-HEAD |
| Component: | Code |
| Category: | bug report |
| Priority: | critical |
| Assigned: | Owen Barton |
| Status: | closed |
Description
the original behavior with 'all' as a default directory was much nicer :-)
added is a suggested patch to make sure some site was specified.
| Attachment | Size |
|---|---|
| drush-dl-defall.patch | 971 bytes |

#1
when i last touched this code it would only use a site specific dir if *modules* had been created for that site dir. to me that suggests that you like to keep your modules there.
this is all overrideable in .drushrc so you might want to just do that. i will add more docs to that file soon.
i will take feedback on this patch but not so sure.
#2
I've run into this problem too. Expected behavior is that if I don't specify the URI parameter or the --destination parameter, then downloaded modules/themes should be saved in sites/all. Alternatively, drush could try and save in sites/default if the appropriate directories exist, then fall back to sites/all if not. Currently, drush saves in the document root if, for example, sites/default/modules does exist when downloading a module.
#3
what it does now is that it tries default/modules, finds out it is not a directory and thus falls into current folder. I added a check that a uri was specified, and if not it takes the all directory instead. Probably the change from before is that the "DRUSH_DRUPAL_SITE_ROOT" wasnt filled ?
#4
So, the ideal behavior (if no valid URL is provided) is that it will check for a default/modules directory and if that doesn't exist will check for an all/modules directory and use that if it exists, before falling back to the current directory? I have not problem with adding a check for that, especially if someone can pull together a patch.
#5
I think the original behavior which went first to all/modules in case no url provided is ideal (sorry if I was unclear - in my last post I was referring to the current behavior). And I think that's what my suggested patch does - checks for the -l option. Probably I should also add a check for -url ? But otherwise I think it should be OK ?
#6
i tried my patch on a different machine and it didnt work, so sending a new attempt, complete with the uri switch (equivalent to l). I am quite surprised noone is unhappy about this ... it is annoying to have downloads fall into the current dir :-)
#7
From the point of view of what is generally considered best practise, and also given that statistically it's more likely that a site will be operating as a standalone unit, I'd favour the idea of selecting sites/all/modules before sites/[default]/modules.
Normally, in a single site setup, I'd use sites/default/modules for custom modules and sites/all/modules for all my contribs.
Given that (let's be honest (aegir aside for a minute)) multi-site setups are pretty rare, I think it's totally fair to provide docs to help configure this for the more niche cases, but have it working "more simply, more of the time" - i.e. with de facto standard expected behaviour, out of the box.
I'd love to hear a good argument against that, I'm sure *someone* probably has one! ;)
#8
After chatting with Moshe I agree that sites/all/modules is a better destination that sites/default/module for non-multisite setups, especially since that is the suggested module location for Drupal 7.
However, the patch in #6 breaks intelligent behavior for downloading modules to a multisite directory when you are calling drush within that directory (and using Drush automatic URI detection, without needing a --uri option or drushrc.php). Attached is a patch that fixes this, but maintains the rest of the behavior of the previous patch.
Here is what happens in each case:
server:~/workspace/drupal-6/sites$ ls *
all:
modules
default:
modules settings.php
wia.6:
modules settings.php
server:~/workspace/drupal-6/sites$ drush dl diff
Project diff (6.x-2.1-alpha1) downloaded to /var/www/workspace/drupal-6/sites/all/modules/. [success]
server:~/workspace/drupal-6/sites$ cd default/
server:~/workspace/drupal-6/sites/default$ drush dl diff
Project diff (6.x-2.1-alpha1) downloaded to /var/www/workspace/drupal-6/sites/all/modules/. [success]
server:~/workspace/drupal-6/sites/default$ cd ..
server:~/workspace/drupal-6/sites$ cd wia.6
server:~/workspace/drupal-6/sites/wia.6$ drush dl diff
Project diff (6.x-2.1-alpha1) downloaded to /var/www/workspace/drupal-6/sites/wia.6/modules/. [success]
server:~/workspace/drupal-6/sites/wia.6$ cd ..
server:~/workspace/drupal-6/sites$ drush dl diff --uri=wia.6
Project diff (6.x-2.1-alpha1) downloaded to /var/www/workspace/drupal-6/sites/wia.6/modules/. [success]
server:~/workspace/drupal-6/sites$ drush dl diff --destination=/var/www/workspace/drupal-6/sites/default/modules
Project diff (6.x-2.1-alpha1) downloaded to /var/www/workspace/drupal-6/sites/default/modules/. [success]
As the last example demonstrates, the only thing that is a little harder now is downloading to sites/default/modules in the case where you *do* want to do that. We could easily tweak the behavior so that it installs to sites/default/modules when you are running drush from within the sites/default directory (i..e the second example above), but sites/all/modules when your are anywhere else (except another valid multisite directory). Would folks on this ticket be happy with that, or would you still expect modules to be downloaded to sites/all/modules, even when you are invoking drush from the sites/default directory?
Either way, I think this is an improvement on the current drush dl behaviour!
#9
thanks a lot :-) and sorry for my ignoring the geniality of the uri detection :-)
BTW : i must have missed something, I do use multisite, but still have modules in all, not in default ....
and yes, I am happy about the solution :-)
#10
Looks sane to me. Committed.
#11
Actually, I was waiting for feedback on if it is reasonable to tweak the behavior of this patch just a tiny bit further so that it installs to sites/default/modules when you are actually running drush from within the sites/default directory, but install to sites/all/modules when your are anywhere else (except another valid multisite directory).
This seems a bit more logical to me, but I wasn't 100% sure if this would mess things up for the sites/all/modules'ers :)
I can't see how it would, since they probably aren't spending much time in the sites/default directory if all their modules are in sites/all, I would assume that they would be calling drush from sites/all/module or from the drupal root or the sites directory (all of which would still default to sites/all/modules). Still, it seems good to check - no need to revert this patch though, it's certainly an improvement.
Also, what do people think about creating the sites/*/modules directory, if it doesn't exist - various people seem to bump into this issue. Of course, we could just improve the documentation, but I don't see any major issues if we just create it and move on.
#12
both of those tweaks sound good. putting download in sites/default when user runs command from there and auto creating sites/all/modules[themes]. note that d7 ships with both of those dirs.
#13
#14
OK, here is a patch with various heuristics that I think make this much more intuitive for most major usages of the all, default and multisite functionality.
// Attempt 0: Use the user specified destination directory, if it exists.
// Attempt 1: If we are in a specific site directory, and the destination directory already exists, then we use that.
// Attempt 2: If the destination directory already exists for sites/all, then we use that.
// Attempt 3: If a specific (non default) site directory exists and sites/all does not exist, then we create destination in the site specific directory.
// Attempt 4: If sites/all exists, then we create destination in the sites/all directory.
// Attempt 5: If site directory exists (even default), then we create destination in the this directory.
// Attempt 6: If we didn't find a validdirectory yet (or we somehow found one that doesn't exist) we always fall back to the current directory.
#15
#16
Wouldn't it be more logical to default to "sites/default" before "/sites/all" ?
When I type "drush dl thismodule", it says : "Initialized Drupal site at sites/default" and then "Using destination directory /srv/mysite/sites/all/modules/", which is contradictory.
#17
it isn't contradictory. in d7, we recommend people use sites/all. see earlier discussion on this. if you are still not pleased, just set a destination in your .drushrc and forget about it.
#18
@Viybel
It's not hard to get it to use default, even without specifying a manual destination. If you run drush from the sites/default directory it will use that first. Also, if you delete your "all" directory it will also use the default directory if that correlates to the bootstrapped site.
#19
Automatically closed -- issue fixed for 2 weeks with no activity.