[PATCH] Simple patch to allow use of CREATOR in DEFAULT_ROLE_PERMS

[Kevin Pulo]

Hi,

I have a MANAGERS role that I use to control who can push to master in
wild repos (while all other users can only push to USER/..* branches).
It was getting really tedious for creators to have to add themselves
to MANAGERS every time they create a new repo. So I added "MANAGERS
CREATOR" to my DEFAULT_ROLE_PERMS in my rc file, but it didn't work.

So I wrote this tiny, nearly trivial patch. Now when users create
repos, they are automatically added as to its MANAGERS role. I hope
that this can be considered for inclusion upstream, in part because it
is so simple.

(Actually, the plan is to use the MANAGERS role for more than just
this, which is why I want the creators to truly be in the MANAGERS
role, and not just duplicate the access rules I have for MANAGERS and
have another rule for CREATOR (if that's even possible).)

Thanks,

Kev

--
Dr Kevin Pulo kevin...@anu.edu.au
Academic Consultant / Systems Programmer www.kev.pulo.com.au
NCI NF / ANU SF +61 2 6125 7568

[Sitaram Chamarty]

On Fri, Mar 15, 2013 at 5:36 PM, Kevin Pulo <kevin...@anu.edu.au> wrote:
> Hi,
>
> I have a MANAGERS role that I use to control who can push to master in
> wild repos (while all other users can only push to USER/..* branches).
> It was getting really tedious for creators to have to add themselves
> to MANAGERS every time they create a new repo. So I added "MANAGERS
> CREATOR" to my DEFAULT_ROLE_PERMS in my rc file, but it didn't work.

In other words, you want to assign one role to another role (CREATOR
is just a pseudo-role that's preset to have just one member).

That's not supported, and I'd prefer to find other solutions to your situation.

> So I wrote this tiny, nearly trivial patch. Now when users create
> repos, they are automatically added as to its MANAGERS role. I hope
> that this can be considered for inclusion upstream, in part because it
> is so simple.

Sorry but no :-)

(1) it touches "core". I'm very, *very*, picky about taking changes there

(2) there are other ways to do this; see attached file for how

Actually, that method is far more flexible too.

For example, let's say instead of adding the creator to MANAGERS, you
wanted to add the *group* he belongs to to MANAGERS. (That is, for
every wild repo created, the MANAGERS are all the "peers" of the
creator, where "peer" is defined by, for example, you putting each
user into one group, like @senior-devs, @junior-devs, and @interns,
etc.)

It's trivial to modify that script, parsing the output of 'gitolite
list-memberships @GL_USER' to find the group name(s) to add.

> (Actually, the plan is to use the MANAGERS role for more than just
> this, which is why I want the creators to truly be in the MANAGERS
> role, and not just duplicate the access rules I have for MANAGERS and
> have another rule for CREATOR (if that's even possible).)

You don't have to duplicate the rule itself, just add it on the same
line, like so:

RW+ master = MANAGERS CREATOR

[Sitaram Chamarty]

typo

On Fri, Mar 15, 2013 at 7:01 PM, Sitaram Chamarty <sita...@gmail.com> wrote:

> It's trivial to modify that script, parsing the output of 'gitolite
> list-memberships @GL_USER' to find the group name(s) to add.

should be $GL_USER of course

[Kevin Pulo]

On Fri, Mar 15, 2013 at 07:01:54PM +0530, Sitaram Chamarty wrote:

> (1) it touches "core". I'm very, *very*, picky about taking changes there
>
> (2) there are other ways to do this; see attached file for how
>
> Actually, that method is far more flexible too.

I understand, and thanks for the feedback. This way of fixing this
problem looks good, and I'm happy to use it.

I will comment, though, that it can sometimes be utterly perplexing
when keywords like CREATOR or USER work in many/most situations, but
not in others. In support of the original goal (but only for its own
sake) of allowing roles and/or special keywords like CREATOR to be
used in DEFAULT_ROLE_PERMS, consistency can also be a good thing. The
value of DEFAULT_ROLE_PERMS is diminished if a POST_CREATE trigger is
required in some situations. (IOW, following the argument to its
conclusion, why bother having DEFAULT_ROLE_PERMS at all? Just always
use such a trigger.)

> It's trivial to modify that script, parsing the output of 'gitolite
> list-memberships @GL_USER' to find the group name(s) to add.

Sure.

I will note, though, that list-memberships doesn't take into account
the results of GROUPLIST_PGM. Yes, you could also muck about getting
GROUPLIST_PGM out of gitolite config, and then run it yourself and
append its groups, but it's nevertheless misleading in that the
memberships used in the config file are not the same as those reported
by list-membership. Should this be opened as an issue?

There's also no API way (ie. using the perms command doesn't count,
because as you'll see below, it's the perms command I'm trying to
adjust) to get the roles of repo, or to query which roles a user is in
for a given repo. I bring this up because it leads me to what I'm
currently trying to do...

> > (Actually, the plan is to use the MANAGERS role for more than just
> > this,

I want to use the MANAGERS role as a kind of user-managable per-repo
wheel group. Specifically, I want the perms command to work for all
MANAGERS, not just the CREATOR.

My use case is that we have users in projects creating and
self-managing repos within the structure/framework we give them (so
they don't tie themselves in knots, many of them are git newbies).
Hence the restriction that only MANAGERS can push to master.
(Actually the rules are:

RW+ [^/]* = MANAGERS
RW+ USER/ = @project

ie. only MANAGERS can push to the "top-level" branches, whereas
everyone else can only work in their sandbox.)

The issue is that limiting the perms command to only the repo creator
is far too limiting. People drift in and out of these projects all
the time (while still keeping an account on the system and being
active in other projects). So having the MANAGERS role allows
collaborative administration of the repo contents, but not of access
to the repo itself. It currently always falls to the creator to
add/remove users from READERS/WRITERS/MANAGERS, whereas we would be
happy for anyone in the MANAGERS team to be doing that. This would
also allow seamless (ie. self-managed) handover of projects - a
MANAGER could add his/her replacement to the MANAGER role, and then as
their final act "abdicate" by removing themselves from the role.

It looks like it comes down to the perms command, where twice it is
hardcoded 'creator($repo) ne $ENV{GL_USER}' (for disallowing access).
I need this functionality (and soon) and will hack it in however
necessary (ie. just reading/parsing gl-perms directly if that's what
it takes), but of course it would be better if it could be done in a
way that was clean, upstreamable and so more easily usable by others
in the same situation.

The 'creator($repo) ne $ENV{GL_USER}' could be cleaned up to be 'not
owns($repo)' (and use Gitolite::Easy of course). This might then be
the start of something to allow a pluggable/configurable way of
choosing who should be allowed to run the perms command (so that
hacking the perms command isn't necessary at all). eg. the default
allow could be the perl expression 'owns($repo)', but this could
instead be configured as 'in_group("pseudoadmins")' (where
@pseudoadmins is defined in the config file, and is not the same as
is_admin(), ie. different to who can push to gitolite-admin), or
'in_role("MANAGERS")' - except of course, there is no corresponding
in_role() function in Gitolite::Easy (yet?). You could even say
'owns($repo) or in_group("pseudoadmins") or in_role("MANAGERS")'.

What do you think of this idea?

I actually have a similar, related problem, with the writable command.
I want our backup server (which uses rsync underneath) to do "ssh
gitolitehost writable @all off 'Daily backups'" before and "ssh
gitolitehost writable @all on" afterwards. But currently, this
requires having an ssh key on the backup server that can write to
gitolite-admin - but the backup server should only ever be allowed to
read the repositories (and that too, through the rsync side-channel,
not through gitolite). It certainly shouldn't be allowed to make
config changes.

For this I was planning on having a @backups group, with just the
backups user in it (and no access to any repositories, even those
readable by all regular users), and then hack the 'is_admin()' in the
writable command to be 'in_group("backups")'. I'm pretty sure this
should work, but it would be much nicer to have a solution that didn't
require hacking the actual command.

The per-repo writable usage also suffers from the exact same
'owns($repo)' issue as perms, ie. at our site I would rather that be
'in_role("MANAGERS")' or similar.

It strikes me that although gitolite's pluggable triggers system is
very good, there doesn't seem to be a good, pluggable/configuable way
of making these sorts of policy decisions. I'm wondering if using
perl expressions and the Gitolite::Easy API (suitably extended a bit
more) might go some way to addressing this.

Anyway, thanks for reading through my rambling if you get this far.
:)

[Sitaram Chamarty]

On Sat, Mar 16, 2013 at 3:19 AM, Kevin Pulo <kevin...@anu.edu.au> wrote:

> value of DEFAULT_ROLE_PERMS is diminished if a POST_CREATE trigger is
> required in some situations. (IOW, following the argument to its
> conclusion, why bother having DEFAULT_ROLE_PERMS at all? Just always
> use such a trigger.)

DEFAULT_ROLE_PERMS is just a convenience that adds a *fixed* text to
gl-perms, no more and no less. The most common use is what you see
commented out in the default rc file -- "READERS @all".

Anything else is done by the user. Manually.

You wanted something that is not a fixed text, but you didn't want the
user to have to do it manually. Hence the script I suggested.

>> It's trivial to modify that script, parsing the output of 'gitolite
>> list-memberships @GL_USER' to find the group name(s) to add.
>
> Sure.
>
> I will note, though, that list-memberships doesn't take into account
> the results of GROUPLIST_PGM. Yes, you could also muck about getting
> GROUPLIST_PGM out of gitolite config, and then run it yourself and

It's on my todo list. Been there for a while :-(

And you don't have to "muck about" too much to call it:

`gitolite query-rc GROUPLIST_PGM` $GL_USER

gets you a list. The only catch is this is space-separated, all in
one line, while list-memberships delivers it's list one of each line.

> append its groups, but it's nevertheless misleading in that the
> memberships used in the config file are not the same as those reported
> by list-membership. Should this be opened as an issue?

Memberships used in the config file **are** the same as those reported
by list-membership.

Memberships that come from GROUPLIST_PGM are not in the conf file at
all and list-memberships is missing them. (The actual reason is a bit
more involved, and the solution may have to use a new flag to
list-memberships... I have to think about how best to do this).

> wheel group. Specifically, I want the perms command to work for all
> MANAGERS, not just the CREATOR.

It's perfectly acceptable to fork any command and use your own; see
http://gitolite.com/gitolite/cust.html#localcode.

> 'in_role("MANAGERS")' - except of course, there is no corresponding
> in_role() function in Gitolite::Easy (yet?). You could even say
> 'owns($repo) or in_group("pseudoadmins") or in_role("MANAGERS")'.

in_role() is a good idea. I'll think about it.

> For this I was planning on having a @backups group, with just the
> backups user in it (and no access to any repositories, even those
> readable by all regular users), and then hack the 'is_admin()' in the
> writable command to be 'in_group("backups")'. I'm pretty sure this
> should work, but it would be much nicer to have a solution that didn't
> require hacking the actual command.

You call it hacking, I call it site-local customisation :)

[Kevin Pulo]

On Sat, Mar 16, 2013 at 05:25:15AM +0530, Sitaram Chamarty wrote:

> DEFAULT_ROLE_PERMS is just a convenience that adds a *fixed* text to
> gl-perms, no more and no less.

I just double-checked the docs again - to me, it's not clear that it's
merely a fixed string. Might be worthwhile adding the word "fixed"
somewhere there, to make that completely explicit.

> And you don't have to "muck about" too much to call it:
>
> `gitolite query-rc GROUPLIST_PGM` $GL_USER
>
> gets you a list. The only catch is this is space-separated, all in
> one line, while list-memberships delivers it's list one of each line.

The other catch is that if GROUPLIST_PGM isn't defined, it will try to
run a command called $GL_USER.

> > append its groups, but it's nevertheless misleading in that the
> > memberships used in the config file are not the same as those reported
> > by list-membership. Should this be opened as an issue?
>
> Memberships used in the config file **are** the same as those reported
> by list-membership.

No. Memberships *defined* in the config file are what's reported by
list-memberships. I am using plenty of groups in my config file where
the memberships come solely from GROUPLIST_PGM. In fact, viewed in
isolation, my config file looks non-sensical - lots of permissions are
granted to groups that aren't defined anywhere. But users are
nonetheless in those groups when it comes time to do the actual
authorisation.

> It's perfectly acceptable to fork any command and use your own; see
> http://gitolite.com/gitolite/cust.html#localcode.

Of course, that's what I'm doing already. I've installed from my
github fork, the installation is the "git clone" way (so no need of
LOCAL_CODE). I have a branch, and work in there. The problem is that
doing things this way puts a lot more management burden on me. Every
time the code around my mods changes, I need to resolve conflicts.
Every time data structures I'm dealing with change, I need to resolve
conflicts. This gets really annoying really quickly.

That's why the manager trigger is great - I can be pretty confident
that I'll never have to worry about conflicts with it. perms, or
writable, OTOH, are much more likely to have upstream changes down the
track.

This is why I would *much* rather a system that lets me do:

#ALLOW_PERMS => 'owns($repo)',
ALLOW_PERMS => 'in_role($repo, "MANAGERS")',

in the rc file, as a matter of configuration, instead of:

+use Gitolite::Easy;

...

- _die $generic_error if repo_missing($repo) or creator($repo) ne $ENV{GL_USER};
+ _die $generic_error if repo_missing($repo) or not (owns($repo) or in_role($repo, "MANAGERS"));

in the actual code. Using git considerably lowers the burden of
carrying around these sorts of patchsets, but it doesn't remove it
completely.

I can see that to you these two alternatives might feel equivalent,
but for me the difference is significant. This is a large part of why
it would be good to get this done "properly" upstream - not just in
case others have similar requirements, and not just "for the good of
the code", but also out of sheer self-interest.

I guess I'm figuring that others (and possibly a not insignificant
number of others) will also find the restrictions of "only creators
may change perms or repo writable" and "only admins may change @all
writable" to be too limiting. So I'm trying to find a solution that
lets users configure this stuff cleanly (like how triggers work well
for their purpose), without increasing your maintenance burden or
risk.

> in_role() is a good idea. I'll think about it.

Glad to hear it. It sounds like for the time being I'll be grepping
gl-perms directly. :)

> > For this I was planning on having a @backups group, with just the
> > backups user in it (and no access to any repositories, even those
> > readable by all regular users), and then hack the 'is_admin()' in the
> > writable command to be 'in_group("backups")'. I'm pretty sure this
> > should work, but it would be much nicer to have a solution that didn't
> > require hacking the actual command.
>
> You call it hacking, I call it site-local customisation :)

Well, when the changes are generic, or general enough that a lot of
users might benefit from them, it feels (to me) less like site
customisation and more like trying to improve the software. It's not
like I'm asking for MANAGERS to be added as a default role alongside
READERS and WRITERS. Just a way of configuring policy ("who can
change a wild repo's permissions at our site?" without needing to dive
headlong into code.

I find the issue with is_admin() in writable particularly troubling,
because one of its intended purpose is for backups - yet it makes
configuring secure backups basically impossible.

FWIW, I've implemented it, and it is also very simple:

--- a/src/commands/writable
+++ b/src/commands/writable
@@ -25,7 +25,7 @@ my $repo = shift;
my $on = ( shift eq 'on' );

if ( $repo eq '@all' ) {
- _die "you are not authorized" if $ENV{GL_USER} and not is_admin();
+ _die "you are not authorized" if $ENV{GL_USER} and not (is_admin() or in_group("writable"));
} else {
_die "you are not authorized" if $ENV{GL_USER} and not owns($repo);
}

Personally, I wouldn't mind if this was adopted and the "writable"
group declared to be "special". But that's only because I haven't
been using it before. :) I understand of course that others might
legitimately be using it for other purposes, and might resent having
this change foisted upon them.

The associated config change was just:

+# The backups user is allowed to toggle @all writability, and that is all.
+@writable = backups
+repo @all
+ - = backups
+ option deny-rules = 1

It works quite nicely now.

[Sitaram Chamarty]

On Sat, Mar 16, 2013 at 8:04 AM, Kevin Pulo <kevin...@anu.edu.au> wrote:

>> Memberships used in the config file **are** the same as those reported
>> by list-membership.
>
> No. Memberships *defined* in the config file are what's reported by

oops yes; I wasn't awake enough!

> #ALLOW_PERMS => 'owns($repo)',
> ALLOW_PERMS => 'in_role($repo, "MANAGERS")',

I would rather not put in code bits like that in the rc.

All this is moot until I do in_role(), but I envisage a hardcoded role
called OWNERS that will be used in the perms command and possibly desc
also. The actual checks would be similar to what you have here:

> + _die $generic_error if repo_missing($repo) or not (owns($repo) or in_role($repo, "MANAGERS"));

> I find the issue with is_admin() in writable particularly troubling,
> because one of its intended purpose is for backups - yet it makes
> configuring secure backups basically impossible.

Although it looks like a hack, the fact is this:

repo gitolite-admin
- = backup
RW = backup

should work just fine, is supported, and (if you grokked rules.html)
is documented:

The backup user now satisfies is_admn() but can't actually write anything.

Just don't use "deny-rules" on the repo!

[Kevin Pulo]

On Sat, Mar 16, 2013 at 08:46:04AM +0530, Sitaram Chamarty wrote:

> On Sat, Mar 16, 2013 at 8:04 AM, Kevin Pulo <kevin...@anu.edu.au> wrote:
>
> > #ALLOW_PERMS => 'owns($repo)',
> > ALLOW_PERMS => 'in_role($repo, "MANAGERS")',
>
> I would rather not put in code bits like that in the rc.

The problem then is that it can be hard to give users flexibility and
generality.

> All this is moot until I do in_role(), but I envisage a hardcoded role
> called OWNERS that will be used in the perms command and possibly desc
> also.

That sounds good. And the per-user part of commands/writable too, I
hope.

> > I find the issue with is_admin() in writable particularly troubling,
> > because one of its intended purpose is for backups - yet it makes
> > configuring secure backups basically impossible.
>
> Although it looks like a hack, the fact is this:
>
> repo gitolite-admin
> - = backup
> RW = backup
>
> should work just fine, is supported, and (if you grokked rules.html)
> is documented:
>
> The backup user now satisfies is_admn() but can't actually write anything.

Uhhh, but what about all the other rules I have that give R to @all on
various repos/patterns? Wouldn't I have to add "- = backup" to all of
them? And not forget to include it on any new rules?

> Just don't use "deny-rules" on the repo!

I don't understand. Why not? What's wrong with how I did it? I
thought that was the whole point of deny-rules? I should also mention
that that was the first rule in my file. And it does seem to work in
practice.

And I'm using deny-rules in a similar way elsewhere, which seems fine:

repo p/projA/..*
C = @projA

RW+ [^/]* = MANAGERS

RW+ USER/ = @projA

...

repo p/..*/private/..*
- = @all
option deny-rules = 1

repo p/..*
R = @all
C = @admins
RW+ = @admins

This works well to give projects a place to have repos readable by
all, with a "private" area visible only to project members.

Or is this not the way it's supposed to be used?

Or, did you just mean don't do this?

repo gitolite-admin
- = backup
RW = backup

options deny-rules = 1

[Sitaram Chamarty]

On Sat, Mar 16, 2013 at 12:29 PM, Kevin Pulo <kevin...@anu.edu.au> wrote:
> On Sat, Mar 16, 2013 at 08:46:04AM +0530, Sitaram Chamarty wrote:

>> On Sat, Mar 16, 2013 at 8:04 AM, Kevin Pulo <kevin...@anu.edu.au> wrote:
>> Although it looks like a hack, the fact is this:
>>
>> repo gitolite-admin
>> - = backup
>> RW = backup
>>
>> should work just fine, is supported, and (if you grokked rules.html)
>> is documented:
>>
>> The backup user now satisfies is_admn() but can't actually write anything.
>
> Uhhh, but what about all the other rules I have that give R to @all on
> various repos/patterns? Wouldn't I have to add "- = backup" to all of
> them? And not forget to include it on any new rules?

I may have missed some nuance of what you are doing with your backup
userid. I just looked at your previous message and you mentioned
"rsync side channel". I don't know how that works.

All I'm doing here is to show that this satisfied is_admin() while
disallowing a push to the admin repo. It *does* allow 'backup' to
read the admin repo. I notice you said you don't want that but I
don't know what difference it makes if the raw files are readable thru
rsync anyway.

>> Just don't use "deny-rules" on the repo!
>
> I don't understand. Why not? What's wrong with how I did it? I

[snipped some stuff I didn't read because I latched onto this:]

> Or, did you just mean don't do this?
>
> repo gitolite-admin
> - = backup
> RW = backup
> options deny-rules = 1

yes. If you have deny-rules on, then is_admin() will no longer be true.

[Kevin Pulo]

On Sat, Mar 16, 2013 at 12:43:45PM +0530, Sitaram Chamarty wrote:

> [snipped some stuff I didn't read because I latched onto this:]
>
> > Or, did you just mean don't do this?
> >
> > repo gitolite-admin
> > - = backup
> > RW = backup
> > options deny-rules = 1
>
> yes. If you have deny-rules on, then is_admin() will no longer be true.

Got it.

Thanks for your help with all of this.

When I get my hacky MANAGERS patch to perms done, I might post it
anyway, in case anyone else is trying to do a similar thing and finds
it useful in the interim until gitolite proper gets OWNERS or similar.

I also have some other work in progress that I might post later on,
which might be useful for anyone trying to integrate gitolite with
existing user accounts (particularly LDAP-backed).

[Kevin Pulo]

So I started hacking commands/perms to read gl-perms and look for (and
use) MANAGERS. It wasn't too hard, but it was pretty hacky. I needed
to do it for both getperms() and setperms(), so I moved it into a
function.

Then I also wanted it in writable, desc and D. I realised that I
could put it into owns() in Gitolite::Easy (which was conveniently
already well-named), and then have that as my single, central point
when "ownership" is defined. So I moved it into there. I wrote
commands/owners, which is just like commands/creator, except that it
uses owns() (and owners(), which at this time owns() used), and that
took care of the shell scripts, too.

Then I realised that to ease transitions, I could easily fall back to
checking creator() if a repo had no MANAGERS defined. So I did that
too. Looking good.

Then I saw that it would be easy to use $rc{OWNER_ROLE} instead of
hardcoding the role name, so I did that too.

Then I noticed that because I was only matching directly on the text
in gl-perms, that adding a group to a role wouldn't work. Before I
knew it, I had written in_roles() in Gitolite::Easy, with list_roles()
and list_repo_roles() in Gitolite::Conf::Load supporting it (and
owns()), and everything was looking rather generic.

I don't expect this to be adopted as-is, but I think that there is
probably a lot here that is worth considering or keeping, ie. that
this may be substantively along the right lines.

At the very least, *please* take the patch that makes commands/perms
use owns() instead of directly comparing creator(). That alone will
allow owns() to be used as a nearly single place that sites can
redefine what ownership means.

But I would also be happy for you to take as much of the rest as you'd
like. And I'm also happy to get feedback, or to rejig things if you'd
like, eg. if you'd like me to break things down into proper, separate,
ordered, bite-sized commits (instead of this giant wad), etc.

Patches attached. It's also in the "owners" branch of my fork:

https://github.com/devkev/gitolite/tree/owners

PS. Hope you're enjoying the test match. ;)

[Sitaram Chamarty]

I just pushed a commit that contains the following changes:

- gitolite list-memberships, as per the usage message change I
emailed about earlier
- Easy.pm has an in_role(rolename, reponame) function, which is now
used by "owns"

To allow others to do 'perms' changes, first the rc file should have
something like

OWNER_ROLENAME => 'OWNER',

This defines the rolename that is used for ownership checking in perms.

Then the creator can give someone (or some group) the OWNER role and
that will be all that is needed.

I have not fixed up D and desc. I think desc change is rare enough,
and "D" usage important/significant/sacred enough, that restricting
them to the actual creator should be ok.

I think this should work fine for what you need.

Documentation is not yet done, but it boils down to adding the rc file
entry and then telling creators to add certain whomever they want to
the rolename you chose.

regards

sitaram

[Kevin Pulo]

On Sun, Mar 17, 2013 at 10:54:01PM +0530, Sitaram Chamarty wrote:

> To allow others to do 'perms' changes, first the rc file should have
> something like
>
> OWNER_ROLENAME => 'OWNER',
>
> This defines the rolename that is used for ownership checking in perms.
>
> Then the creator can give someone (or some group) the OWNER role and
> that will be all that is needed.

Excellent, that's great to see - thanks.

I notice you have "creator or owner" in owns(), which I can probably
live with (and even if not, having owns() as the single point to make
the site hack is very good).

> I have not fixed up D and desc. I think desc change is rare enough,
> and "D" usage important/significant/sacred enough, that restricting
> them to the actual creator should be ok.

I understand. For our purposes, though, I think I would like D and
desc to respect owners.

A problem is that there's no longer a shell equivalent for owns().
The comment near owns() still says to do "if gitolite creator repo
user; then". What's actually needed is crazy:

if gitolite creator repo $GL_USER || gitolite query-rc -q OWNER_ROLENAME && gitolite list-memberships -u $GL_USER -r repo | grep -x `gitolite query-rc OWNER_ROLENAME` > /dev/null; then

But the real problem with this isn't that it's a mouthful, it's that
it's a duplication of the logic in own() (and in another language). I
would much prefer to have a small script (eg. commands/owns) which is
basically just a perl to shell bridge for the actual owns() function.

Would you consider including either of the attached small shell
scripts as commands/owns? Then users who (like me) want "owner"
behaviour for desc and D just need to change "gitolite creator ..." to
"gitolite owns ...".

At first I was thinking the first script would be better, but now I
think I like the second - it much more closely matches owns(), makes
that perfectly clear, and I can't really think of any possible use
case for wanting to query the ownership status of a user other than
$GL_USER anyway.

I'm not too fussed either way, since carrying around a separate file
in our site's custom patchset isn't too bad.

[Sitaram Chamarty]

On Mon, Mar 18, 2013 at 5:33 AM, Kevin Pulo <kevin...@anu.edu.au> wrote:

> I notice you have "creator or owner" in owns(), which I can probably

if you don't have "creator or" you get a catch-22 when someone creates
a new repo and wants to *manually* (i.e., not automatically via that
trigger script we discussed some time ago) add someone to the OWNER
role.

> live with (and even if not, having owns() as the single point to make
> the site hack is very good).

> A problem is that there's no longer a shell equivalent for owns().
> The comment near owns() still says to do "if gitolite creator repo
> user; then". What's actually needed is crazy:

The comment is flat wrong and an oversight when I made the code change.

> At first I was thinking the first script would be better, but now I
> think I like the second - it much more closely matches owns(), makes

I like the second one better too; I'll use that and push something out
within the next day.

[Kevin Pulo]

On Mon, Mar 18, 2013 at 06:24:19AM +0530, Sitaram Chamarty wrote:

> if you don't have "creator or" you get a catch-22 when someone creates
> a new repo and wants to *manually* (i.e., not automatically via that
> trigger script we discussed some time ago) add someone to the OWNER
> role.

Yes, that's why my code didn't quite do this, it was a little more
nuanced. Even in the presence of $rc{OWNER_ROLENAME}, if a repo had
no gl-perms lines that actually gave that role to some user/group,
then it would fall back to using the creator.

The catch then is if that the creator basically needs to add
themselves first (unless they really truly are creating the repo for
someone else, which seems pointless) or a group that they are a member
of. If they typo, then they're locked out. :)

Which is why "creator or owner" is probably fine in practice - the
only actual use-case it doesn't handle is the repo creator permanently
handing off repo control to someone else. That's probably rare enough
that an admin to ssh to the repo and edit gl-creator for them, and
then the handover is much more likely to go well anyway.

> I like the second one better too; I'll use that and push something out
> within the next day.

Excellent, that's great to hear, thanks!