[Rd] R CMD INSTALL and file permission settings

[Simon Urbanek]

On Jun 14, 2013, at 11:27 PM, Dirk Eddelbuettel wrote:

> 
> On 14 June 2013 at 16:56, Simon Urbanek wrote:
> | On Jun 14, 2013, at 4:35 PM, Dirk Eddelbuettel wrote:
> | > But up until right now I could not update a package a colleague installed,
> | > and vice versa -- unless we sudo.  
> | > 
> | 
> | But you should be able to simply removing it, and re-installing, right? (This is not to suggest it as a work-around, but rather to make sure we're taking about the same situation). 
> 
> I think not:  
> 
>  -- 'Alice' and 'Bob' are both members of 'r-users'. 
> 
>  -- Alice installs, it becomes '644' / '755' owned by 'alice:r-users'.  
> 
>  -- Bob tries to update (or remove, as you suggest)
> 
>  -- Bob has no group-write (despite sharing the group) or world-write rights
> 
>  -- Hence this fails
> 

Nope this is wrong. If you have write rights on a directory, you can delete files even if you don't have write rights on them. That's why rf -rf + re-install works but update doesn't.


>  -- My patch (reworked by Martin) fixes precisely this
> 
> | The implementation of the group-wrtable part of is great improvement, but my point is that this doesn't solve the update.packages() problem that triggered the fix, because the flag is ignored then:
> | 
> | $ bin/R CMD INSTALL --group-writable ~/develop/R/packages/base64enc_0.1-0.tar.gz 
> | $ ls -ld library/base64enc
> | drwxrwxr-x  10 urbanek  admin  340 Jun 14 16:48 library/base64enc
> 
> Mode 775, good.
> 
> | $ sudo -u user2 bin/R -e 'update.packages(ask=F)'
> | $ ls -ld library/base64enc
> | drwxr-xr-x  11 user2  admin  374 Jun 14 16:49 library/base64enc
> 
> Mode 755 -- why?
> 

Because update.packages() doesn't restore the group-writable bit. Which leads us to my point that this is not what you really want.


> | $ bin/R -e 'update.packages(ask=F)'
> | [...]
> | mv: rename /Volumes/Data/Builds/R-builds/rd/library/base64enc to /Volumes/Data/Builds/R-builds/rd/library/00LOCK-base64enc/base64enc: Permission denied
> | Warning in file.copy(f, instdir, TRUE) :
> |   problem copying ./NAMESPACE to /Volumes/Data/Builds/R-builds/rd/library/base64enc/NAMESPACE: Permission denied
> | Warning in file.copy(f, instdir, TRUE) :
> |   problem copying ./NEWS to /Volumes/Data/Builds/R-builds/rd/library/base64enc/NEWS: Permission denied
> | Warning in file(file, ifelse(append, "a", "w")) :
> |   cannot open file '/Volumes/Data/Builds/R-builds/rd/library/base64enc/DESCRIPTION': Permission denied
> | Error in file(file, ifelse(append, "a", "w")) : 
> |   cannot open the connection
> | ERROR: installing package DESCRIPTION failed for package ‘base64enc’
> 
> I will admit to having written and tested the patch at home, under single
> user, not work.  But at work I "simulated" the exact same situation by doing
> repeated 'sudo chmod -R g+w /usr/local/lib/R/site-library' after which 'Bob'
> can indeed updated packages installed by 'Alice'.
> 
> Which is the whole point.
> 
> | So, as I was suggesting to use the patch to implement a more durable solution which doesn't need an extra flag but just checks the permissions on the library. While at it, it could also consult umask and thus be a good citizen ...
> 
> We can always refine, but I still think that we're better off with the patch
> than before.
> 

I wasn't disputing the fact that it's great to have group-writable bit under control, but adding the flag to INSTALL doesn't solve the problem - and as you saw the result is a bit misleading in the context that it was discussed and thus may cause more confusion. I am arguing that the solution would be to (at least as default) take the directory writable bit because that is really what decides things anyway. You had a good point of also combining it with umask.

Cheers,
Simon


> Dirk
> 
> -- 
> Dirk Eddelbuettel | edd at debian.org | http://dirk.eddelbuettel.com
> 
>