Re: [rdiff-backup-users] [PATCH] Preserve symlink permissions

[Andrew Ferguson]

dean gaudet wrote:
> btw -- adding two syscalls per symlink creation is a bit of a waste for 
> platforms where it doesn't matter.  any chance you'd consider adding a 
> test to fs_abilities and conditionalizing on it?

Dean,

I have added this test and a new symlink_perms global to avoid the
unnecessary syscalls on platforms where it doesn't matter. The attached
patch was applied to CVS this morning.

Andrew

-- 
Andrew Ferguson - address@hidden

diff -Nur rdiff-backup-cvs/rdiff_backup/Globals.py 
rdiff-backup-symlinkfs/rdiff_backup/Globals.py
--- rdiff-backup-cvs/rdiff_backup/Globals.py    2005-11-24 00:07:09.000000000 
-0500
+++ rdiff-backup-symlinkfs/rdiff_backup/Globals.py      2006-12-29 
10:47:44.000000000 -0500
@@ -221,6 +221,11 @@
 # prevent highbit permissions on systems which don't support them.)
 permission_mask = 07777
 
+# If true, symlinks permissions are affected by the process umask, and
+# we should change the umask when creating them in order to preserve
+# the original permissions
+symlink_perms = None
+
 def get(name):
        """Return the value of something in this module"""
        return globals()[name]
diff -Nur rdiff-backup-cvs/rdiff_backup/fs_abilities.py 
rdiff-backup-symlinkfs/rdiff_backup/fs_abilities.py
--- rdiff-backup-cvs/rdiff_backup/fs_abilities.py       2006-01-11 
16:23:30.000000000 -0500
+++ rdiff-backup-symlinkfs/rdiff_backup/fs_abilities.py 2006-12-29 
11:04:13.000000000 -0500
@@ -46,6 +46,7 @@
        name = None # Short string, not used for any technical purpose
        read_only = None # True if capabilities were determined 
non-destructively
        high_perms = None # True if suid etc perms are (read/write) supported
+       symlink_perms = None # True if symlink perms are affected by umask
 
        def __init__(self, name = None):
                """FSAbilities initializer.  name is only used in logging"""
@@ -87,6 +88,7 @@
                                                          ('Directory inc 
permissions',
                                                           self.dir_inc_perms),
                                                          ('High-bit 
permissions', self.high_perms),
+                                                         ('Symlink 
permissions', self.symlink_perms),
                                                          ('Extended 
filenames', self.extended_filenames)])
                add_boolean_list([('Access control lists', self.acls),
                                                  ('Extended attributes', 
self.eas),
@@ -145,6 +147,7 @@
                self.set_resource_fork_readwrite(subdir)
                self.set_carbonfile()
                self.set_high_perms_readwrite(subdir)
+               self.set_symlink_perms(subdir)
 
                subdir.delete()
                return self
@@ -395,6 +398,21 @@
                else: self.high_perms = 1
                tmp_rp.delete()
 
+       def set_symlink_perms(self, dir_rp):
+               """Test if symlink permissions are affected by umask"""
+               sym_source = dir_rp.append("symlinked_file1")
+               sym_source.touch()
+               sym_dest = dir_rp.append("symlinked_file2")
+               sym_dest.symlink(sym_source.path)
+               sym_dest.setdata()
+               assert sym_dest.issym()
+               orig_umask = os.umask(077)
+               if sym_dest.getperms() == 0700: self.symlink_perms = 1
+               else: self.symlink_perms = 0
+               os.umask(orig_umask)
+               sym_dest.delete()
+               sym_source.delete()
+               
 def get_readonly_fsa(desc_string, rp):
        """Return an fsa with given description_string
 
@@ -455,6 +473,9 @@
                if not self.dest_fsa.high_perms:
                        SetConnections.UpdateGlobal('permission_mask', 0777)
 
+       def set_symlink_perms(self):
+               SetConnections.UpdateGlobal('symlink_perms',
+                                                                       
self.dest_fsa.symlink_perms)
 
 class BackupSetGlobals(SetGlobals):
        """Functions for setting fsa related globals for backup session"""
@@ -612,6 +633,7 @@
        bsg.set_fsync_directories()
        bsg.set_change_ownership()
        bsg.set_high_perms()
+       bsg.set_symlink_perms()
        bsg.set_chars_to_quote(Globals.rbdir)
 
 def restore_set_globals(rpout):
@@ -632,6 +654,7 @@
        # No need to fsync anything when restoring
        rsg.set_change_ownership()
        rsg.set_high_perms()
+       rsg.set_symlink_perms()
        rsg.set_chars_to_quote(Globals.rbdir)
 
 def single_set_globals(rp, read_only = None):
@@ -650,5 +673,6 @@
                ssg.set_hardlinks()
                ssg.set_change_ownership()
                ssg.set_high_perms()
+               ssg.set_symlink_perms()
        ssg.set_chars_to_quote(Globals.rbdir)
 
diff -Nur rdiff-backup-cvs/rdiff_backup/rpath.py 
rdiff-backup-symlinkfs/rdiff_backup/rpath.py
--- rdiff-backup-cvs/rdiff_backup/rpath.py      2006-11-05 16:54:52.000000000 
-0500
+++ rdiff-backup-symlinkfs/rdiff_backup/rpath.py        2006-12-29 
10:49:01.000000000 -0500
@@ -103,9 +103,9 @@
        elif rpin.issym():
                # some systems support permissions for symlinks, but
                # only by setting at creation via the umask
-               orig_umask = os.umask(0777 & ~rpin.getperms())
+               if Globals.symlink_perms: orig_umask = os.umask(0777 & 
~rpin.getperms())
                rpout.symlink(rpin.readlink())
-               os.umask(orig_umask)    # restore previous umask
+               if Globals.symlink_perms: os.umask(orig_umask)  # restore 
previous umask
        elif rpin.ischardev():
                major, minor = rpin.getdevnums()
                rpout.makedev("c", major, minor)