[PATCH] umask for multiple commiters

[Alexis S. L. Carvalho]

Thus spake Aurelien Jacobs:
> # HG changeset patch
> # User Aurelien Jacobs <aurel at gnuage.org>
> # Date 1161034334 -7200
> # Node ID 29513230e3858b572c6e2fb1d60bcb15b11ab2a4
> # Parent  5207cf649abe173239db83cabf68ea8dedddfd77
> inherit parent directory mode when creating new file

I don't have a strong opinion on the feature (I think some users will
like it, while others will say that one shouldn't reinvent the wheel and
just use umask), but...

> diff -r 5207cf649abe -r 29513230e385 mercurial/localrepo.py
> --- a/mercurial/localrepo.py	Mon Oct 16 12:56:41 2006 +0200
> +++ b/mercurial/localrepo.py	Mon Oct 16 23:32:14 2006 +0200
> @@ -34,10 +34,7 @@ class localrepository(repo.repository):
>  
>          if not os.path.isdir(self.path):
>              if create:
> -                if not os.path.exists(path):
> -                    os.mkdir(path)
> -                os.mkdir(self.path)
> -                os.mkdir(self.join("data"))
> +                util.makedirs(self.join("data"))

I do think that repo creation should just follow the umask - I don't see
why you would want to inherit the permissions from the parent in this
specific case.

(And this patch hunk would probably change the current "hg init doesn't
create intermediate directories" behaviour that was just discussed on
the other list).

> diff -r 5207cf649abe -r 29513230e385 mercurial/util.py
> --- a/mercurial/util.py	Mon Oct 16 12:56:41 2006 +0200
> +++ b/mercurial/util.py	Mon Oct 16 23:32:14 2006 +0200
> @@ -423,6 +423,20 @@ def system(cmd, environ={}, cwd=None, on
>          if cwd is not None and oldcwd != cwd:
>              os.chdir(oldcwd)
>  
> +def makedirs(name):

A docstring would be nice.

> +    try:
> +        os.mkdir(name)
> +        os.chmod(name,os.lstat(os.path.abspath(os.path.dirname(name))).st_mode)

You really don't want to use lstat to get the permissions (if it's a
symlink, you'll get the permissions of the symlink itself - 0777).

And this line is starting to get a bit too hard to read.

> @@ -802,7 +817,8 @@ def opener(base, audit=True):
>              except OSError:
>                  d = os.path.dirname(f)
>                  if not os.path.isdir(d):
> -                    os.makedirs(d)
> +                    makedirs(d)
> +                st_mode = os.lstat(d).st_mode & 0666

lstat again.

> @@ -810,7 +826,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 != None:

"st_mode is not None" is more idiomatic - but I'm already nitpicking.

Alexis