On 04/19/10 08:43 PM, Richard Lowe wrote:
I'd like code review for: 15686 pkgsend create-repository should not create cfg_cache with mode 0600 webrev: http://cr.opensolaris.org/~richlowe/pkg_15686 This breaks the -d option of onu(1) in most cases, since onu will be running as root and translated to 'nobody' by NFS, so I'd like to get it fixed promptly. As described in the bug, this was broken by #15190 not fixing the file permissions after switching to using a temporary file created by tempfile.mkstemp() rather than clobbering cfg_cache in place. The change I'm making here preserves the permissions of an existing cfg_cache and uses 0644 as the default. I'm still on the fence about attempting to preserve the existing permissions, in most cases I've found the code treats the depot directory structure as entirely its own, but I'm not sure if this is intent, and I'm also not sure it'd be the right thing to do with cfg_cache anyway. Further, should this attempt to preserve ownership? If so, what should it do if it cannot?
The cfg_cache file is a bit special since the depot allows this file to live outside of the repository via the --cfg-file option. In light of that, preserving the permissions seems fine.
The only nit I have is that it seems like you could do the os.stat() on self.__pathname once and skip the extra os.path.exists. Otherwise, looks fine to me.
Cheers, -Shawn _______________________________________________ pkg-discuss mailing list [email protected] http://mail.opensolaris.org/mailman/listinfo/pkg-discuss
