[PATCH] umask for multiple commiters

[Aurelien Jacobs]

On Sun, 12 Nov 2006 23:18:58 +0100
Aurelien Jacobs <aurel at gnuage.org> wrote:

> On Thu, 9 Nov 2006 17:47:40 +0100
> "Benoit Boissinot" <bboissin at gmail.com> wrote:
> 
> > 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)
> 
> Are you sure that 2 umask syscall is cheaper that 1 chmod syscall on a
> file that was just created (ie. inode is still in write buffer) ?
> 
> Anyway, I'm fine with your patch except for one detail. It does the couple
> of umask call for every file write which is useless. umask is only needed
> for file creation (and this makes a much bigger speed difference than
> umask vs. chmod).
> So the attached patch is a very small variation of yours, which do
> umask only for file creation.
> The patch is tested and working correctly.

So will someone finally choose one of the various solution proposed in
this thread ?

Aurel