[RFC] separating "delete" from "rewind"

[Sitaram Chamarty]

Hello all,

I need your advice/opinions/thoughts (again!)

"RW+" currently means "permission to rewind or delete a branch", because
in some sense they are similar -- they both wipe out (parts of)
history. Today someone on #git asked if I could separate these two --
allow one but not the other. My thoughts on this are as follows, and
I'd like yours before I write code!

It all comes down to the need to restrict. Is the project's need to
restrict "delete" as strong as the need to restrict a "rewind"? Maybe
in your project a delete is very rare, and you want to make sure that
only a few people can delete a branch. However, rewinds are more
useful; as long as the team communicates well, a rewind can sometimes
make a repo better.

Conversely, maybe feature branches are shortlived in your project, owned
only by the developer responsible for the feature, and he has to delete
the branch after it has been integrated into mainline, to prevent
cluttering the branch namespace. This makes deletes very common and
desirable. Conversely, due to whatever constraints [maybe
geographical], rewinds become hell, so they should be restricted.

So this is the opposite case to the above, but both are about keeping
rewind and delete separate.

Now I will argue *against* separating them:

(1) since gitolite will let you recover from either problem by using its
log files, it is not a big risk if someone with RW+ does the wrong
thing, so there is no need to worry so much. However this does require
shell access, which in a really large install may not be easy even for
gitolite-admin.

(2) just use "personal branches" to keep each developers damage to his
own branches.

Ideas? Opinions?

Thanks,

Sitaram

[Eli Barzilay]

On Mar 30, Sitaram Chamarty wrote:
> Hello all,
>
> I need your advice/opinions/thoughts (again!)
>
> "RW+" currently means "permission to rewind or delete a branch",
> because in some sense they are similar -- they both wipe out (parts
> of) history. Today someone on #git asked if I could separate these
> two -- allow one but not the other. My thoughts on this are as
> follows, and I'd like yours before I write code!

> [...]

Just one quick additional case where separating might be good: if I
had a way to do that, then I'd allow myself to delete branches and
tags but not to rewind them. The reason is that I can understand
deletions fine, but I'm much less comfortable with rewinding and I'd
prefer to have another safety pin that guards against that. (I also
get the distinct impression that recovering from a rewind can be
messy, while a deletion is easy to undo.)

--
((lambda (x) (x x)) (lambda (x) (x x))) Eli Barzilay:
http://barzilay.org/ Maze is Life!

[Sitaram Chamarty]

well it turned out to be small enough, (only 2 new lines added, and 2
existing lines changed very slightly; as usual the docs take more time!)
so I did it.

I'd still like to hear opinions from others if you have time, especially
if you're against the idea for some reason...

thanks,

sitaram

--
Sitaram Chamarty
+91-40-6667-3521 (work)
+91-92462-22927 (cell; please note new number)
440-3521 (TCS India VOIP)

[Greg Lonnon]

I am for it.

Both activities are separate and there's good reasons to allow or disallow one or the other.

Greg

--
To unsubscribe, reply using "remove me" as the subject.

[Hiren D. Patel]

I like the idea of separating permission to rewind and permission to delete branches.  However, the use cases where the separation is truly important are unclear to me. Is cluttering the namespace a real problem?

-- Hiren

[Teemu Matilainen]

On Wed, 31 Mar 2010, Sitaram Chamarty wrote:

> I'd still like to hear opinions from others if you have time, especially
> if you're against the idea for some reason...

I'm not actually against it as long as I am not forced to declare both
permissions (so the implementation is fine for me).

But anyway I find those quite useless. As the documentation notes,
rewind can normally be done with delete and push. And branch can be
virtually deleted (well, replaced by totally different content) using
rewrite. And from all of these cases it is easy to recover either from
some clone or directly from the server.

Cheers,
- Teemu

[Sitaram Chamarty]

Hello all,

There've been a few folks saying that having to specify "D" separately
from RW or RW+ is cumbersome, and although I don't actually use this
feature, I can see the point.

I've been thinking about it a bit more, and it seems to me that one
clean way to think of this is:

- RW and RW+ were the only existing branch level rights
- it doesnt make sense to have D rights without W (hence RW) rights
- so we simply suffix a D to these if required.

Thus you can have RW, RW+, RWD, RW+D.

I hope the (hopefully few) of you who have started to use this feature
will convert your configs when you next upgrade to "pu".

I now regret pushing the previous syntax to master -- lots of people
use master only, and on the next promotion of pu the syntax will
change. To reduce this exposure, this amendment will be promoted to
master very quickly, like by this weekend.

I hope people don't get too mad at me :)

Sitaram

> --
> To unsubscribe, reply using "remove me" as the subject.
>

--
Sitaram

[Eli Barzilay]

On Apr 15, Sitaram Chamarty wrote:
> [...]

> Thus you can have RW, RW+, RWD, RW+D.

> [...]

Just to explain what prompted me to ask for this: if I have

RW+ foo
RW+ bar

in one place in the file, then some completely different place I need
to add a D, then I now need to find all of these places and change
them to

RW+ foo
D foo
RW+ bar
D bar

Just adding a D seems better to me.

[Andrew Donkin]

On Wednesday, 31 March 2010 03:01:54 UTC+13, Eli Barzilay wrote:

 

Just one quick additional case where separating might be good: if I

had a way to do that, then I'd allow myself to delete branches and
tags but not to rewind them.

I'll chime in nearly a year late :-)  I, too, want people to delete branches after they have been merged, but I don't want anyone performing operations that lose commits.  In other words, I want to allow "git branch -d" but not "-D".  From reading the documentation, it's not clear to me whether the D permission will allow a user to delete the last reference to a commit, causing its branch to unravel into the garbage collector.

Can you set me straight?  Thanks.

[Sitaram Chamarty]

First, the -d/-D thing is only on the client side.

Second, -d complains if the branch you're deleting is not merged
into your *current* HEAD. So in a picture like this (where the
numbers are branch names):

--- * --- B --- * --- C
\
--- D --- * --- E (your current HEAD)

"git branch -d B" will fail, while "[...] -d D" will work.

There is no exact equivalent on the server, because the "current
HEAD" is either fixed or meaningless anyway. If you're thinking
in this direction, what you want is to prevent deletion of *any*
branch that is not already part of some *other* branch.

But if you did that, you'd then have to start applying a
different set of rules for tags (a tag name, even though it is
on an internal node, is much more important than a branch name
-- merely being able to reach the commits is not sufficient to
maintain sanity).

At this point it becomes a human responsibility -- it gets to be
quite difficult to do this automatically and keep the syntax
sane and tractable.

Which is where the -D kicks in. You allow certain responsible
people the "RW+D" or "RWD" or such, and assume they know what
they're doing.

regards

sitaram

[Eli Barzilay]

Three hours ago, Sitaram Chamarty wrote:
>
> There is no exact equivalent on the server, because the "current
> HEAD" is either fixed or meaningless anyway. If you're thinking in
> this direction, what you want is to prevent deletion of *any* branch
> that is not already part of some *other* branch.

The obvious thing here is to check that the commit is included in the
`master' line. And the obvious next thing is that in some projects
you'd have a different workflow. This means that you really want to
customize it somehow...

> But if you did that, you'd then have to start applying a different
> set of rules for tags (a tag name, even though it is on an internal
> node, is much more important than a branch name -- merely being able
> to reach the commits is not sufficient to maintain sanity).

...which makes this point stronger. So IMO, the way to do that is
using your own hook to reject bad pushes.

But I think that there's another point to this story: if you force all
branches to be merged, then that makes branches much less useful. So
you would probably have admin users that can delete unmerged branches,
and that seems like a bad idea... (But maybe in some big companies
that would be desirable?)

[Sitaram Chamarty]

It *should* be easy enough, in existing gitolite, to cater to
any amount of complexity in this decision, by using an
update.secondary hook (that will get chained by the normal
gitolite update hook). In this hook:

- check if the ref being deleted is unmerged. There is no
single command that does this, and regardless of how you do
it, it's a complex script. I'd be happy to be proven wrong;
I need this in some other places where I'm using something
kludgy.

Actually, pre-receive may be better. Otherwise you'll get
caught by a rare race condition (say branches A and B both
point to the same commit, both are not merged in by any
other, and both are deleted in the same push)

- once you've determined it *is* a delete of an unmerged
branch, check to see if the person doing it has permission.
This is where you'd use the "in_group" function in the adc
common functions file (it should work fine in a hook as
well). So you'd designate people who are allowed to delete
unmerged branches like this:

@GRIM_REAPERS = alice bob carol

then check in your script if the current user is "in_group
GRIM_REAPERS".

All this is assuming shell; if you want to use some other
language you ought to be able to pick apart what the shell part
of contrib/adc/adc.common-functions is doing and rewire it in
your own language.

regards

sitaram

[Eli Barzilay]

9 hours ago, Sitaram Chamarty wrote:
> On Tue, Jan 31, 2012 at 10:12:44PM -0500, Eli Barzilay wrote:
> > But I think that there's another point to this story: if you force
> > all branches to be merged, then that makes branches much less
> > useful. So you would probably have admin users that can delete
> > unmerged branches, and that seems like a bad idea... (But maybe
> > in some big companies that would be desirable?)
>
> It *should* be easy enough, in existing gitolite, to cater to any
> amount of complexity in this decision, by using an update.secondary
> hook (that will get chained by the normal gitolite update hook). In
> this hook:

Yes, I mentioned that earlier -- but in the above I question the
utility of such a feature.

> - check if the ref being deleted is unmerged. There is no single
> command that does this, and regardless of how you do it, it's a
> complex script. I'd be happy to be proven wrong; I need this in
> some other places where I'm using something kludgy.

Why is this not a simple matter of going over all refs other than the
deleted ones and checking if the commit is in one of them?

[Seth Keel]

Was this ever implemented? I'd like to use this functionality and wasn't able to find anything about it in the docs. Thanks,

Seth

[Sitaram Chamarty]

On 12/20/2013 08:58 AM, Seth Keel wrote:
>
> Was this ever implemented? I'd like to use this functionality and
> wasn't able to find anything about it in the docs. Thanks,

Going purely by the subject line, yes. But the snippets below indicate
this is more than just that, though it's not enough to tell me precisely
what we are talking about.

Perhaps you could tell us what it is you're looking for, and I'll see
how (and if) it can be done in v3. The below stuff seems to be about v2
anyway...

regards
sitaram

[Seth Keel]

Sitaram,

Thanks for the quick response. Sorry for not being clear, I quoted one post in particular that was later in the discussion.

I would like to let some developers delete merged branches, but not unmerged branches. We can live without it, but it would be convenient.

Thanks,

Seth

[Sitaram Chamarty]

On 12/20/2013 10:04 AM, Seth Keel wrote:
> Sitaram,
>
> Thanks for the quick response. Sorry for not being clear, I quoted
> one post in particular that was later in the discussion.

no problem...

> I would like to let some developers delete merged branches, but not
> unmerged branches. We can live without it, but it would be convenient.

Ok now some of that stuff is coming back to me :-)

Would your developers ever delete more than one branch in the same push?
Specifically, would they ever try to delete two branches that both point
to the same commit?

Much depends on that answer ;-)

[Seth Keel]

> > I would like to let some developers delete merged branches, but not
>
> > unmerged branches. We can live without it, but it would be convenient.
>
>
>
> Ok now some of that stuff is coming back to me :-)
>
>
>
> Would your developers ever delete more than one branch in the same push?
>
> Specifically, would they ever try to delete two branches that both point
>
> to the same commit?
>
>
>
> Much depends on that answer ;-)

Generally no, but never say never... I'm sure it would happen at some point. It may be infrequent enough that the effort required to recover would be acceptable.

[Sitaram Chamarty]

[changed subject line]

Anyway here's some very simple code for this. As usual ther're more
English than perl, because I decided to explain the race condition and
possible workarounds, if someone cares.

Please TEST IT! I didn't give it a thorough shake!

See http://gitolite.com/gitolite/cust.html#localcode for details on
how/where to put this VREF.

[Seth Keel]

Sitaram,

Wow. Thank you so much! That was very fast. I've only just started looking at the code and haven't implemented it yet. I'll post once it's tested and contribute back changes if any. Thanks again!!

Seth

[Tim Nordell]

On our server here I integrated a local VREF called "trash" that anytime a ref is changing (either from a branch deletion or from a rewind) it creates a local refspace copy of that branch in a "trash" namespace automatically. See attached if you'd like to try it. It doesn't try to do any automatic checking to see if a branch is merged into another.

E.g. if one were to do a rewind or a delete on the branch, the previous ref is saved off into "refs/trash/<path_to_ref>_<timestamp>" which is not normally cloned by remotes but still accessible by remotes. This allows the git repository to never lose any refs that were given to it, but also allows these refs to be in a space that isn't normally pulled by clients.

- Tim