[PATCH] umask for multiple commiters
Aurelien Jacobs
aurel at gnuage.org
Wed Oct 25 21:27:43 UTC 2006
On Tue, 24 Oct 2006 01:31:19 -0300
"Alexis S. L. Carvalho" <alexis at cecm.usp.br> wrote:
> 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...
I don't care if it's implemented this way or with umask (like in my
initial patch), as long as one of them is picked and applied.
> > 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.
Example of a situation: someone want to create a new repo on the
server with "hg init ssh://server//dir/repo". If it's not created
with parents mode, it won't be group writable and thus other users
won't be able to commit on it.
Telling users to chmod after a hg init is not an option as their
shell is the restricted hgsh.
But I can also see how this feature could be controversial.
> (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).
Indeed. I personnaly prefer this new behavior, but I guess others don't.
Obviously such a behavior change shouldn't be tied to my patch.
Anyway, I removed that hunk.
> > 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.
Right.
> > + 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).
I don't even know why I used lstat ! Probably a copy/paste from somewhere.
> And this line is starting to get a bit too hard to read.
Ok.
> > @@ -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.
Damn, I'm spotted ! Indeed, I'm not a python coder. I'm a C coder.
So don't hesitate to nitpick if my code is not exactly pythonish.
Updated patch attched.
It now only affect file/directory creation inside .hg.
Mode setting on .hg itself must still be done by hand.
So this should really be safe.
Aurel
-------------- next part --------------
A non-text attachment was scrubbed...
Name: inherit-mode.diff
Type: text/x-diff
Size: 2003 bytes
Desc: not available
URL: <http://lists.mercurial-scm.org/pipermail/mercurial-devel/attachments/20061025/dafe0f2e/attachment.diff>
More information about the Mercurial-devel
mailing list