[PATCH] umask for multiple commiters
Benoit Boissinot
bboissin at gmail.com
Thu Nov 9 16:47:40 UTC 2006
On 11/8/06, Aurelien Jacobs <aurel at gnuage.org> wrote:
> On Thu, 2 Nov 2006 00:47:10 +0100
> Aurelien Jacobs <aurel at gnuage.org> wrote:
> > On Thu, 26 Oct 2006 11:08:04 +0200
> > Thomas Arendsen Hein <thomas at intevation.de> wrote:
> > > Maybe there are still too many stat/chmod calls, I suggest the
> > > following:
> > >
> > > On repo object creation, look at the mode of the .hg and check if
> > > the user's umask is permissive enough to create it.
> > > Something like this code:
> > >
> > > umask = os.umask(0)
> > > os.umask(umask)
> > > repomask = umask & ~st.st_mode
> > > if repomask != umask:
> > > self.ui.warn("Overriding your umask %04o with $04o for repository\n"
> > > % (umask, repomask))
> > > self.repomask = repomask
> > > else:
> > > self.repomask = None
> > >
> > > And whenever writing to the repository (i.e. below .hg):
> > >
> > > if self.repomask is not None:
> > > umask = os.umask(self.repomask)
> > > else:
> > > umask = None
> > > foo(bar, whatever)
> > > if umask is not None:
> > > os.umask(umask)
> > >
> > > Instead of changing with the umask, I can imagine doing a chmod on
> > > file or directory creation, too, but only on creation, not every
> > > time writing to a file.
> >
> > First let me say that this previous patch was already doing a chmod
> > only for file/directory creation, not for simple file write.
> >
> > But I agree with you that a stat for each file creation might be a bit
> > too much. So I improved this.
> > I still kept chmod instead of changing umask. I guess that doing a chmod
> > on a just created (ie. buffered) file is not worst than doing 2 umask call.
> >
> > About the complexity for creating N files:
> > * previous patch:
> > - N stat()
> > - N chmod()
> > * new patch:
> > - 1 stat() if N > 0
> > - N chmod() only if needed (ie. if current umask is not enough)
> >
> > I hope this patch is now ok.
>
> Did anyone had a look at it ?
>
I implemented Thomas' suggestion which I find cleaner (umask is a lot
cheaper than chmod or stat, altough I don't know if it matters)
please review,
Benoit
-------------- next part --------------
diff -r 7d3d603e7df9 mercurial/localrepo.py
--- a/mercurial/localrepo.py Tue Nov 07 21:43:19 2006 +0100
+++ b/mercurial/localrepo.py Thu Nov 09 17:44:57 2006 +0100
@@ -47,7 +47,16 @@ class localrepository(repo.repository):
self.origroot = path
self.ui = ui.ui(parentui=parentui)
self.opener = util.opener(self.path)
- self.sopener = util.opener(self.path)
+ # setup umask
+ umask = os.umask(0)
+ os.umask(umask)
+ repomask = ~stat.S_IMODE(os.stat(self.path).st_mode) & 0777
+ if repomask == umask:
+ repomask = None
+ else:
+ self.ui.warn(_("repository mode overrides umask with %04o\n"
+ % repomask))
+ self.sopener = util.opener(self.path, umask=repomask)
self.wopener = util.opener(self.root)
try:
diff -r 7d3d603e7df9 mercurial/util.py
--- a/mercurial/util.py Tue Nov 07 21:43:19 2006 +0100
+++ b/mercurial/util.py Thu Nov 09 17:41:53 2006 +0100
@@ -754,7 +754,7 @@ else:
return _("stopped by signal %d") % val, val
raise ValueError(_("invalid exit code"))
-def opener(base, audit=True):
+def opener(base, audit=True, umask=None):
"""
return a function that opens files relative to base
@@ -763,6 +763,7 @@ def opener(base, audit=True):
"""
p = base
audit_p = audit
+ umask_p = umask
def mktempcopy(name):
d, fn = os.path.split(name)
@@ -817,12 +818,15 @@ def opener(base, audit=True):
def o(path, mode="r", text=False, atomic=False, atomictemp=False):
if audit_p:
audit_path(path)
+ old_umask = None
f = os.path.join(p, path)
if not text:
mode += "b" # for that other OS
if mode[0] != "r":
+ if umask_p is not None:
+ old_umask = os.umask(umask_p)
try:
nlink = nlinks(f)
except OSError:
@@ -836,7 +840,10 @@ def opener(base, audit=True):
return atomictempfile(f, mode)
if nlink > 1:
rename(mktempcopy(f), f)
- return posixfile(f, mode)
+ fh = posixfile(f, mode)
+ if old_umask is not None:
+ os.umask(old_umask)
+ return fh
return o
More information about the Mercurial-devel
mailing list