Executable flagging complement

Gustavo Niemeyer gustavo at niemeyer.net
Thu Oct 6 19:52:35 BST 2005


The attached patch will change the way executable flagging works
to how Aaron suggested recently in the mailing list. Now it won't
interfere with other mode settings anymore and the flag is
considered binary valued: either the file is executable or not
(any of the x-bits turned on means the file is executable).

This has a nice side-effect: it's impossible to get a conflict
when three-way-merging a binary value.

Cheers!

-- 
Gustavo Niemeyer
http://niemeyer.net
-------------- next part --------------
=== modified file 'bzrlib/changeset.py'
--- bzrlib/changeset.py
+++ bzrlib/changeset.py
@@ -45,50 +45,57 @@
     return newdict
 
        
-class ChangeUnixPermissions(object):
+class ChangeExecFlag(object):
     """This is two-way change, suitable for file modification, creation,
     deletion"""
-    def __init__(self, old_mode, new_mode):
-        self.old_mode = old_mode
-        self.new_mode = new_mode
+    def __init__(self, old_exec_flag, new_exec_flag):
+        self.old_exec_flag = old_exec_flag
+        self.new_exec_flag = new_exec_flag
 
     def apply(self, filename, conflict_handler, reverse=False):
         if not reverse:
-            from_mode = self.old_mode
-            to_mode = self.new_mode
-        else:
-            from_mode = self.new_mode
-            to_mode = self.old_mode
+            from_exec_flag = self.old_exec_flag
+            to_exec_flag = self.new_exec_flag
+        else:
+            from_exec_flag = self.new_exec_flag
+            to_exec_flag = self.old_exec_flag
         try:
-            current_mode = os.stat(filename).st_mode &0777
+            current_exec_flag = bool(os.stat(filename).st_mode & 0111)
         except OSError, e:
             if e.errno == errno.ENOENT:
-                if conflict_handler.missing_for_chmod(filename) == "skip":
+                if conflict_handler.missing_for_exec_flag(filename) == "skip":
                     return
                 else:
-                    current_mode = from_mode
-
-        if from_mode is not None and current_mode != from_mode:
-            if conflict_handler.wrong_old_perms(filename, from_mode, 
-                                                current_mode) != "continue":
+                    current_exec_flag = from_exec_flag
+
+        if from_exec_flag is not None and current_exec_flag != from_exec_flag:
+            if conflict_handler.wrong_old_exec_flag(filename,
+                        from_exec_flag, current_exec_flag) != "continue":
                 return
 
-        if to_mode is not None:
+        if to_exec_flag is not None:
+            current_mode = os.stat(filename).st_mode
+            if to_exec_flag:
+                umask = os.umask(0)
+                os.umask(umask)
+                to_mode = current_mode | (0100 & ~umask)
+                # Enable x-bit for others only if they can read it.
+                if current_mode & 0004:
+                    to_mode |= 0001 & ~umask
+                if current_mode & 0040:
+                    to_mode |= 0010 & ~umask
+            else:
+                to_mode = current_mode & ~0111
             try:
                 os.chmod(filename, to_mode)
             except IOError, e:
                 if e.errno == errno.ENOENT:
-                    conflict_handler.missing_for_chmod(filename)
+                    conflict_handler.missing_for_exec_flag(filename)
 
     def __eq__(self, other):
-        if not isinstance(other, ChangeUnixPermissions):
-            return False
-        elif self.old_mode != other.old_mode:
-            return False
-        elif self.new_mode != other.new_mode:
-            return False
-        else:
-            return True
+        return (isinstance(other, ChangeExecFlag) and
+                self.old_exec_flag == other.old_exec_flag and
+                self.new_exec_flag == other.new_exec_flag)
 
     def __ne__(self, other):
         return not (self == other)
@@ -916,31 +923,16 @@
         Exception.__init__(self, "Conflict applying changes to %s" % this_path)
         self.this_path = this_path
 
-class MergePermissionConflict(Exception):
-    def __init__(self, this_path, base_path, other_path):
-        this_perms = os.stat(this_path).st_mode & 0755
-        base_perms = os.stat(base_path).st_mode & 0755
-        other_perms = os.stat(other_path).st_mode & 0755
-        msg = """Conflicting permission for %s
-this: %o
-base: %o
-other: %o
-        """ % (this_path, this_perms, base_perms, other_perms)
-        self.this_path = this_path
-        self.base_path = base_path
-        self.other_path = other_path
-        Exception.__init__(self, msg)
-
 class WrongOldContents(Exception):
     def __init__(self, filename):
         msg = "Contents mismatch deleting %s" % filename
         self.filename = filename
         Exception.__init__(self, msg)
 
-class WrongOldPermissions(Exception):
-    def __init__(self, filename, old_perms, new_perms):
-        msg = "Permission missmatch on %s:\n" \
-        "Expected 0%o, got 0%o." % (filename, old_perms, new_perms)
+class WrongOldExecFlag(Exception):
+    def __init__(self, filename, old_exec_flag, new_exec_flag):
+        msg = "Executable flag missmatch on %s:\n" \
+        "Expected %s, got %s." % (filename, old_exec_flag, new_exec_flag)
         self.filename = filename
         Exception.__init__(self, msg)
 
@@ -964,7 +956,7 @@
         Exception.__init__(self, msg)
         self.filename = filename
 
-class MissingPermsFile(Exception):
+class MissingForSetExec(Exception):
     def __init__(self, filename):
         msg = "Attempt to change permissions on  %s, which does not exist" %\
             filename
@@ -1027,17 +1019,14 @@
         os.unlink(new_file)
         raise MergeConflict(this_path)
 
-    def permission_conflict(self, this_path, base_path, other_path):
-        raise MergePermissionConflict(this_path, base_path, other_path)
-
     def wrong_old_contents(self, filename, expected_contents):
         raise WrongOldContents(filename)
 
     def rem_contents_conflict(self, filename, this_contents, base_contents):
         raise RemoveContentsConflict(filename)
 
-    def wrong_old_perms(self, filename, old_perms, new_perms):
-        raise WrongOldPermissions(filename, old_perms, new_perms)
+    def wrong_old_exec_flag(self, filename, old_exec_flag, new_exec_flag):
+        raise WrongOldExecFlag(filename, old_exec_flag, new_exec_flag)
 
     def rmdir_non_empty(self, filename):
         raise DeletingNonEmptyDirectory(filename)
@@ -1048,8 +1037,8 @@
     def patch_target_missing(self, filename, contents):
         raise PatchTargetMissing(filename)
 
-    def missing_for_chmod(self, filename):
-        raise MissingPermsFile(filename)
+    def missing_for_exec_flag(self, filename):
+        raise MissingForExecFlag(filename)
 
     def missing_for_rm(self, filename, change):
         raise MissingForRm(filename)
@@ -1265,9 +1254,9 @@
         return new_meta
     elif new_meta is None:
         return old_meta
-    elif isinstance(old_meta, ChangeUnixPermissions) and \
-        isinstance(new_meta, ChangeUnixPermissions):
-        return ChangeUnixPermissions(old_meta.old_mode, new_meta.new_mode)
+    elif (isinstance(old_meta, ChangeExecFlag) and
+          isinstance(new_meta, ChangeExecFlag)):
+        return ChangeExecFlag(old_meta.old_exec_flag, new_meta.new_exec_flag)
     else:
         return ApplySequence(old_meta, new_meta)
 
@@ -1391,7 +1380,7 @@
         stat_a = self.lstat(full_path_a)
         stat_b = self.lstat(full_path_b)
 
-        cs_entry.metadata_change = self.make_mode_change(stat_a, stat_b)
+        cs_entry.metadata_change = self.make_exec_flag_change(stat_a, stat_b)
 
         if id in self.tree_a and id in self.tree_b:
             a_sha1 = self.tree_a.get_file_sha1(id)
@@ -1405,16 +1394,15 @@
                                                              stat_b)
         return cs_entry
 
-    def make_mode_change(self, stat_a, stat_b):
-        mode_a = None
+    def make_exec_flag_change(self, stat_a, stat_b):
+        exec_flag_a = exec_flag_b = None
         if stat_a is not None and not stat.S_ISLNK(stat_a.st_mode):
-            mode_a = stat_a.st_mode & 0777
-        mode_b = None
+            exec_flag_a = bool(stat_a.st_mode & 0111)
         if stat_b is not None and not stat.S_ISLNK(stat_b.st_mode):
-            mode_b = stat_b.st_mode & 0777
-        if mode_a == mode_b:
-            return None
-        return ChangeUnixPermissions(mode_a, mode_b)
+            exec_flag_b = bool(stat_b.st_mode & 0111)
+        if exec_flag_a == exec_flag_b:
+            return None
+        return ChangeExecFlag(exec_flag_a, exec_flag_b)
 
     def make_contents_change(self, full_path_a, stat_a, full_path_b, stat_b):
         if stat_a is None and stat_b is None:

=== modified file 'bzrlib/merge.py'
--- bzrlib/merge.py
+++ bzrlib/merge.py
@@ -431,7 +431,7 @@
         return tree.tree.inventory
 
     inv_changes = merge_flex(this_tree, base_tree, other_tree,
-                             generate_cset_optimized, get_inventory,
+                             generate_changeset, get_inventory,
                              MergeConflictHandler(this_tree, base_tree,
                              other_tree, ignore_zero=ignore_zero),
                              merge_factory=merge_factory, 

=== modified file 'bzrlib/merge_core.py'
--- bzrlib/merge_core.py
+++ bzrlib/merge_core.py
@@ -260,18 +260,18 @@
     metadata = entry.metadata_change
     if metadata is None:
         return None
-    if isinstance(metadata, changeset.ChangeUnixPermissions):
-        if metadata.new_mode is None:
+    if isinstance(metadata, changeset.ChangeExecFlag):
+        if metadata.new_exec_flag is None:
             return None
-        elif metadata.old_mode is None:
+        elif metadata.old_exec_flag is None:
             return metadata
         else:
             base_path = base.readonly_path(entry.id)
             other_path = other.readonly_path(entry.id)    
-            return PermissionsMerge(base_path, other_path)
+            return ExecFlagMerge(base_path, other_path)
     
 
-class PermissionsMerge(object):
+class ExecFlagMerge(object):
     def __init__(self, base_path, other_path):
         self.base_path = base_path
         self.other_path = other_path
@@ -283,14 +283,26 @@
         else:
             base = self.other_path
             other = self.base_path
-        base_stat = os.stat(base).st_mode
-        other_stat = os.stat(other).st_mode
-        this_stat = os.stat(filename).st_mode
-        if base_stat &0777 == other_stat &0777:
-            return
-        elif this_stat &0777 == other_stat &0777:
-            return
-        elif this_stat &0777 == base_stat &0777:
-            os.chmod(filename, other_stat)
-        else:
-            conflict_handler.permission_conflict(filename, base, other)
+        base_mode = os.stat(base).st_mode
+        base_exec_flag = bool(base_mode & 0111)
+        other_mode = os.stat(other).st_mode
+        other_exec_flag = bool(other_mode & 0111)
+        this_mode = os.stat(filename).st_mode
+        this_exec_flag = bool(this_mode & 0111)
+        if (base_exec_flag != other_exec_flag and
+            this_exec_flag != other_exec_flag):
+            assert this_exec_flag == base_exec_flag
+            current_mode = os.stat(filename).st_mode
+            if other_exec_flag:
+                umask = os.umask(0)
+                os.umask(umask)
+                to_mode = current_mode | (0100 & ~umask)
+                # Enable x-bit for others only if they can read it.
+                if current_mode & 0004:
+                    to_mode |= 0001 & ~umask
+                if current_mode & 0040:
+                    to_mode |= 0010 & ~umask
+            else:
+                to_mode = current_mode & ~0111
+            os.chmod(filename, to_mode)
+

=== modified file 'bzrlib/selftest/test_merge_core.py'
--- bzrlib/selftest/test_merge_core.py
+++ bzrlib/selftest/test_merge_core.py
@@ -11,7 +11,7 @@
 from bzrlib.osutils import file_kind, rename
 from bzrlib import changeset
 from bzrlib.merge_core import (ApplyMerge3, make_merge_changeset,
-                                BackupBeforeChange, PermissionsMerge)
+                               BackupBeforeChange, ExecFlagMerge)
 from bzrlib.changeset import Inventory, apply_changeset, invert_dict
 
 
@@ -223,11 +223,10 @@
             self.change_perms_tree(id, self.other, other)
 
         if base is not None or other is not None:
-            old_perms = os.stat(self.base.full_path(id)).st_mode &077
-            new_perms = os.stat(self.other.full_path(id)).st_mode &077
-            contents = changeset.ChangeUnixPermissions(old_perms, 
-                                                       new_perms)
-            self.cset.entries[id].metadata_change = contents
+            old_exec = bool(os.stat(self.base.full_path(id)).st_mode & 0111)
+            new_exec = bool(os.stat(self.other.full_path(id)).st_mode & 0111)
+            metadata = changeset.ChangeExecFlag(old_exec, new_exec)
+            self.cset.entries[id].metadata_change = metadata
 
     def change_name_tree(self, id, tree, name):
         new_path = tree.child_path(self.cset.entries[id].parent, name)
@@ -448,30 +447,21 @@
     def test_perms_merge(self):
         builder = MergeBuilder()
         builder.add_file("1", "0", "name1", "text1", 0755)
-        builder.change_perms("1", other=0655)
+        builder.change_perms("1", other=0644)
         builder.add_file("2", "0", "name2", "text2", 0755)
-        builder.change_perms("2", base=0655)
+        builder.change_perms("2", base=0644)
         builder.add_file("3", "0", "name3", "text3", 0755)
-        builder.change_perms("3", this=0655)
+        builder.change_perms("3", this=0644)
         cset = builder.merge_changeset(ApplyMerge3)
         assert(cset.entries["1"].metadata_change is not None)
-        assert(isinstance(cset.entries["1"].metadata_change,
-                          PermissionsMerge))
-        assert(isinstance(cset.entries["2"].metadata_change,
-                          PermissionsMerge))
+        assert(isinstance(cset.entries["1"].metadata_change, ExecFlagMerge))
+        assert(isinstance(cset.entries["2"].metadata_change, ExecFlagMerge))
         assert(cset.entries["3"].is_boring())
         builder.apply_changeset(cset)
-        assert(os.stat(builder.this.full_path("1")).st_mode &0777 == 0655)
+        assert(os.stat(builder.this.full_path("1")).st_mode &0777 == 0644)
         assert(os.stat(builder.this.full_path("2")).st_mode &0777 == 0755)
-        assert(os.stat(builder.this.full_path("3")).st_mode &0777 == 0655)
+        assert(os.stat(builder.this.full_path("3")).st_mode &0777 == 0644)
         builder.cleanup();
-        builder = MergeBuilder()
-        builder.add_file("1", "0", "name1", "text1", 0755)
-        builder.change_perms("1", other=0655, base=0555)
-        cset = builder.merge_changeset(ApplyMerge3)
-        self.assertRaises(changeset.MergePermissionConflict, 
-                     builder.apply_changeset, cset)
-        builder.cleanup()
 
 class FunctionalMergeTest(TestCaseInTempDir):
 



More information about the bazaar mailing list