[PATCH 1 of 2] opener: add new resetflags param to __call__

Matt Mackall mpm at selenic.com
Sun Dec 5 18:30:36 CST 2010


On Sun, 2010-12-05 at 19:49 +0100, Adrian Buehlmann wrote:
> On 2010-12-04 23:45, Matt Mackall wrote:
> > On Sat, 2010-12-04 at 13:39 +0100, Adrian Buehlmann wrote:
> >>              try:
> >>                  if 'w' in mode:
> >> -                    st_mode = os.lstat(f).st_mode & 0777
> >> +                    if not resetflags:
> >> +                        st_mode = os.lstat(f).st_mode & 0777
> > 
> > When do we not want to reset the mode on write?

> As an experiment, I found that with the following patch (alone), the
> testsuite passes:

Ok, so simply put, the answer seem to be that the patcher (and only the
patcher!) needs to preserve modes. So why don't we do something like
this instead?

diff -r c97ded7b6e79 mercurial/patch.py
--- a/mercurial/patch.py	Wed Dec 01 21:46:08 2010 +0100
+++ b/mercurial/patch.py	Sun Dec 05 18:24:36 2010 -0600
@@ -433,6 +433,10 @@
         if islink:
             fp = cStringIO.StringIO()
         else:
+            try:
+                st_mode = os.lstat(fname).st_mode & 0777 # preserve the mode
+            except OSError:
+                pass
             fp = self.opener(fname, 'w')
         try:
             if self.eolmode == 'auto':
@@ -449,6 +453,7 @@
                     fp.write(l)
             else:
                 fp.writelines(lines)
+                os.chmod(fname, st_mode)
             if islink:
                 self.opener.symlink(fp.getvalue(), fname)
         finally:
diff -r c97ded7b6e79 mercurial/util.py
--- a/mercurial/util.py	Wed Dec 01 21:46:08 2010 +0100
+++ b/mercurial/util.py	Sun Dec 05 18:24:36 2010 -0600
@@ -879,7 +879,6 @@
             mode += "b" # for that other OS
 
         nlink = -1
-        st_mode = None
         dirname, basename = os.path.split(f)
         # If basename is empty, then the path is malformed because it points
         # to a directory. Let the posixfile() call below raise IOError.
@@ -890,7 +889,6 @@
                 return atomictempfile(f, mode, self.createmode)
             try:
                 if 'w' in mode:
-                    st_mode = os.lstat(f).st_mode & 0777
                     os.unlink(f)
                     nlink = 0
                 else:
@@ -910,10 +908,7 @@
                     rename(mktempcopy(f), f)
         fp = posixfile(f, mode)
         if nlink == 0:
-            if st_mode is None:
-                self._fixfilemode(f)
-            else:
-                os.chmod(f, st_mode)
+            self._fixfilemode(f)
         return fp
 
     def symlink(self, src, dst):

-- 
Mathematics is the supreme nostalgia of our time.




More information about the Mercurial-devel mailing list