drjuls / AMBiT

GNU General Public License v3.0
32 stars 15 forks source link

New AngularData files need to be writable by group, not just owner #11

Closed drjuls closed 7 years ago

drjuls commented 7 years ago

Today's disaster: raijin:/short/gx16/ambit/AngularData$ ls -l total 15460592 -rw-r----- 1 ek6256 gx16 35988 Feb 15 02:58 0.0.even.0.angular -rw-r----- 1 ek6256 gx16 16676 Feb 14 12:42 0.0.odd.0.angular -rw-r----- 1 ek6256 gx16 51804 Feb 15 02:59 0.2.even.2.angular -rw-r----- 1 ek6256 gx16 25100 Feb 14 12:42 0.2.odd.2.angular -rw-r----- 1 ek6256 gx16 51524 Feb 15 03:02 0.4.even.4.angular -rw-r----- 1 ek6256 gx16 25364 Feb 14 12:42 0.4.odd.4.angular -rw-r----- 1 ek6256 gx16 37660 Feb 15 03:03 0.6.even.6.angular -rw-r----- 1 ek6256 gx16 19516 Feb 14 12:42 0.6.odd.6.angular -rw-rw---- 1 jcb561 gx16 865009308 Feb 23 17:43 1.1.even.1.angular -rw-rw---- 1 jcb561 gx16 1582001396 Feb 2 05:08 1.1.odd.1.angular .... I cannot write 0.0.even.0.angular because Emily made it :(

emilyviolet commented 7 years ago

Do we want to make all files generated by AMBiT writable by group to head off further incidents like this? If so, we could fix this relatively easily on posix systems by using umask() to set the default permissions for the whole run of AMBiT, so we don't have to muck around with chmod. This is implementation defined and limited to posix though, and I have no idea how to do this on Windows, so I'm not sure how you feel about this solution?

drjuls commented 7 years ago

YES! That's exactly what I want. Could we use umask so that the r/w permissions are always given to group? Can we include this as part of the install so that even when ambit is recompiled (or a new install) it still works? And if people are compiling on Windows, we'll assume that's for a single-user system.

emilyviolet commented 7 years ago

Yep, that's definitely doable. I think the easiest way to make sure it survives recompilation would be to #include<sys/stat.h> in Include.h (or similar) and have AMBiT call the posix umask function at the top of main() (or at least before we open any streams). Basically, Raijin's default umask is 0027, which strips write access to users in Group and all access from users in Other, so if we want to make sure Group has rw permissions and Other has r permissions (for example), then we overwrite it like:

int main(...){
...
// Only call umask if we're on a posix system
#ifdef UNIX
    umask(0003) // Don't strip write permission from Group or read permission from Other
#endif
OutStreams::InitialiseStreams();
...

umask is kind of annoyingly opaque, since it takes an integer representation of the familiar chmod input (e.g. g+rw), but I've tested it on my local machine and I'm confident it'll do what we want it to.

If this looks acceptable to you then I'll make the changes and run some tests on raijin and katana, otherwise I'm happy to keep searching around for a better solution.

drjuls commented 7 years ago

This seems like it'll work. Thank you. As you suggest, put #include <sys/stat.h> in Include.h, but make sure it is unix-only.

emilyviolet commented 7 years ago

Manually overriding the umask seems to work fine on raijin:

~/short/AMBiT (dev *)$ ls -lh AngularData/ total 8.0K -rw-rw-r-- 1 ek6256 gx16 1.9K Jul 6 12:45 2.0.even.0.angular -rw-rw-r-- 1 ek6256 gx16 996 Jul 6 12:46 2.2.even.2.angular

Code passes all unit tests as well, so I've pushed the changes to the dev branch (I hope this is okay).