inherit mode patch

Matt Mackall mpm at selenic.com
Sun Mar 4 12:54:44 CST 2007


On Sun, Mar 04, 2007 at 04:00:26PM -0300, Alexis S. L. Carvalho wrote:
> Thus spake Alexis S. L. Carvalho:
> > Thus spake Matt Mackall:
> > > Wouldn't it be simpler to just pass mode=x down and be done with it?
> > > Where mode=None if umask is agreeable? Then opener can simply do:
> > > 
> > >   if mode:
> > >     chmod(f)
> > > 
> > > I'd really like to avoid making opener any more complicated than it
> > > already is.
> > 
> > Fair enough.
> > 
> > > I don't see any useful reason to have just subsets of a repo being
> > > group-writable. That'll just cause more confusion. So it should really
> > > be an all or nothing thing based on the permissions of .hg.
> > 
> > When the new layout with store/ was introduced, the idea of having more
> > than one repo sharing the same store was circulated.  This suggests that
> > we should get the permissions from the store directory instead of .hg.
> 
> Something like this?  It ends up creating repo.opener twice...
> 
> Alexis
> 
> diff -r 193e0f8d9a47 mercurial/localrepo.py
> --- a/mercurial/localrepo.py	Sun Mar 04 14:35:11 2007 -0300
> +++ b/mercurial/localrepo.py	Sun Mar 04 15:58:07 2007 -0300
> @@ -78,7 +78,14 @@ class localrepository(repo.repository):
>              self.encodefn = lambda x: x
>              self.decodefn = lambda x: x
>              self.spath = self.path
> -        self.sopener = util.encodedopener(util.opener(self.spath), self.encodefn)
> +
> +        mode = os.stat(self.spath).st_mode
> +        if (0777 & ~util._umask) == (0777 & mode):
> +            mode = None
> +
> +        self.opener = util.opener(self.path, mode=mode)
> +        self.sopener = util.encodedopener(util.opener(self.spath, mode=mode),
> +                                          self.encodefn)

Why not util.encodedopener(self.opener, self.encodefn)?

> diff -r 193e0f8d9a47 mercurial/util.py
> --- a/mercurial/util.py	Sun Mar 04 14:35:11 2007 -0300
> +++ b/mercurial/util.py	Sun Mar 04 15:58:07 2007 -0300
> @@ -764,6 +764,9 @@ def linkfunc(path, fallback):
>          return lambda x: is_link(os.path.join(path, x))
>      return fallback
>  
> +_umask = os.umask(0)
> +os.umask(_umask)
> +
>  # Platform specific variants
>  if os.name == 'nt':
>      import msvcrt
> @@ -890,8 +893,6 @@ if os.name == 'nt':
>  
>  else:
>      nulldev = '/dev/null'
> -    _umask = os.umask(0)
> -    os.umask(_umask)
>  
>      def rcfiles(path):
>          rcs = [os.path.join(path, 'hgrc')]

Separate patch.

> @@ -1057,7 +1058,23 @@ def encodedopener(openerfn, fn):
>          return openerfn(fn(path), *args, **kw)
>      return o
>  
> -def opener(base, audit=True):
> +def makedirs(name, mode=None):
> +    """recursive directory creation with parent mode inheritance"""
> +    parent = os.path.abspath(os.path.dirname(name))
> +    try:
> +        os.mkdir(name)
> +        if mode is not None:
> +            os.chmod(name, mode)
> +        return
> +    except OSError, err:
> +        if err.errno == errno.EEXIST:
> +            return
> +        if err.errno != errno.ENOENT:
> +            raise
> +    makedirs(parent)
> +    makedirs(name)
> +
> +def opener(base, audit=True, mode=None):
>      """
>      return a function that opens files relative to base
>  
> @@ -1066,6 +1083,7 @@ def opener(base, audit=True):
>      """
>      p = base
>      audit_p = audit
> +    repomode = mode

There's no such thing as a repo at this level.

>  
>      def mktempcopy(name):
>          d, fn = os.path.split(name)
> @@ -1121,6 +1139,7 @@ def opener(base, audit=True):
>          if audit_p:
>              audit_path(path)
>          f = os.path.join(p, path)
> +        st_mode = None
>  
>          if not text:
>              mode += "b" # for that other OS
> @@ -1131,7 +1150,9 @@ def opener(base, audit=True):
>              except OSError:
>                  d = os.path.dirname(f)
>                  if not os.path.isdir(d):
> -                    os.makedirs(d)
> +                    makedirs(d, repomode)
> +                if repomode is not None:
> +                    st_mode = repomode & 0666
>              else:
>                  if atomic:
>                      return atomicfile(f, mode)
> @@ -1139,7 +1160,10 @@ def opener(base, audit=True):
>                      return atomictempfile(f, mode)
>                  if nlink > 1:
>                      rename(mktempcopy(f), f)
> -        return posixfile(f, mode)
> +        pf = posixfile(f, mode)
> +        if st_mode is not None:
> +            os.chmod(f, st_mode)
> +        return pf
>  
>      return o
> 
> _______________________________________________
> Mercurial-devel mailing list
> Mercurial-devel at selenic.com
> http://selenic.com/mailman/listinfo/mercurial-devel

-- 
Mathematics is the supreme nostalgia of our time.


More information about the Mercurial-devel mailing list