I have strange user:group using drush dl on Mac OS X:

az:~ artem$ id
uid=501(artem) gid=20(staff) groups=20(staff),402(com.apple.sharepoint.group.1),204(_developer),100(_lpoperator),98(_lpadmin),81(_appserveradm),80(admin),79(_appserverusr),61(localaccounts),12(everyone),403(com.apple.sharepoint.group.2),401(com.apple.access_screensharing)
az:~ artem$ drush dl drupal
az:~ artem$ ls -ld drupal-7.2
drwxr-xr-x 29 artem wheel 986 May 25 23:56 drupal-7.2

As far as I understand it's because of behavior regarding the group of a newly created files. In Mac OS X (and BSD UNIX) the file is created with the group ID of the directory in which it is located.

az:~ artem$ ls -l /tmp
lrwxr-xr-x@ 1 root wheel 11 Mar 28 2010 /tmp -> private/tmp
az:~ artem$ ls -ld /private/tmp
drwxrwxrwt 38 root wheel 1292 Jun 20 02:02 /private/tmp

So each file created in /tmp belongs to the group 'wheel'. Then drush make a copy of files with same permissions to proper place.

Solution:
In my particular case I would prefer to use sys_get_temp_dir() as temporary directory to avoid such a permission problem. But the selection order in function drush_find_tmp() forces using of /tmp even if an existing/writable sys_get_temp_dir() is found. I would suggest that we use existing/writable sys_get_temp_dir() even if /tmp is available to avoid group problems.

CommentFileSizeAuthor
#4 drush-find-tmp-1193526-4.patch1.14 KBAnonymous (not verified)
0001-drush_find_tmp-use-existing-wirtable-sys_get_temp_di.patch835 bytesaz

Comments

jonhattan’s picture

jonhattan
Primary language Spanish
Location Plasencia
commented

There's a reasoning for the current implementation at #950928: Can't drush dl on Mac OSX 10.6 and MAMP (rsync link_stat fails). It was reported that sys_get_temp_dir() cause problems in mac.

Vasudeva’s picture

Vasudeva commented

I got problem's too because of the selection order in drush_find_tmp.
My OS is Ubuntu Linux.
I think there is a need to be able to set a custom path for the tmp directory.

So there are several possibilities:

1.) If the sys_get_temp() dir causes problems under Mac OSX. Add a condition, that it is not executed in this case.

2.) Add as first assignment $directories[] = getenv('TMPDIR');

3.) Possibly it would be configurable via drushrc.php.

SolInvictus’s picture

SolInvictus commented

The sys_get_temp() command will defer to the TMPDIR environment setting (at least on Ubuntu). The use of '/tmp' is problematic on shared hosting ISPs as their are opportunities for name-space collisions. The current test only checks write permissions before accepting the directory and then other commands could fail if a filename collision occurs.

Anonymous’s picture

Anonymous (not verified) commented
Version: 7.x-4.4 » 7.x-5.x-dev
Priority: Normal » Major
Status: Active » Needs review
StatusFileSize
new drush-find-tmp-1193526-4.patch1.14 KB

Unfortunately, due to this issue drush_move_dir("$tmp/$docroot", $destination) in drush_archive_restore() can't just move inside partition but has to copy between partitions which results in reset timestamps and considerably longer restoration time.

To cope with this in a compatible way I introduced function to check for MacOS use and ensured that operating systems other than Windows/MacOS will use temporary directory provided by sys_get_temp_dir().

I attach patch (compatible with both 7.x-5.x-dev and 8.x-6.x-dev branches) which resolves this issue. Please review, test under Windows/MacOS and commit. Thank you!

moshe weitzman’s picture

moshe weitzman commented
Status: Needs review » Closed (won't fix)

you can now set an env variable to use whatever tmpdir you want. see new 5.9 release and 8.x-6.x in git.

so, i don't think this is needed anymore.

Anonymous’s picture

Anonymous (not verified) commented

Indeed, 8.x-6.x branch changed tmp directory selection algorithm in a way that my patch is no longer needed.

For the references of other drush users: as of now 7.x-5.x branch still uses old tmp directory selection algorithm, so it is still broken.

Anyway, as long as 8.x-6.x should be released in a few months (prior to DrupalCon Portland as per Greg Anderson) I guess it's ok to close this issue as obsolete.

Thank you!

anarcat’s picture

anarcat commented

actually, that's not true: there are still hardcoded /tmp in drush's source, in 6.x.

Anonymous’s picture

Anonymous (not verified) commented

Can't agree (just checked the code once again). While /tmp is still hard-coded it is checked after directories contained in TEMP/TMP environment variables. This way if either TEMP or TMP environment variable contains valid path it's used instead of hard-coded /tmp directory. This is what I tried to achieve with my original patch (use directories contained in TEMP/TMP environment variables instead of /tmp), so my patch indeed is now obsolete for current 8.x-6.x branch.