Subject: Re: [RFD] Rewriting safety - warn before/when rewriting published history

From: Jakub Narebski <[email-masked]>
Date: Mon, 6 Feb 2012 15:44:11 +0100

On Sun, 5 Feb 2012, Johan Herland wrote:
> On Sun, Feb 5, 2012 at 21:46, Jakub Narebski <[email-masked]> wrote:
>> On Sun, 5 Feb 2012, Johan Herland wrote:
>>> 2012/2/5 Jakub Narebski <[email-masked]>:

[...]
>>> I agree that the 'public' state should (by default) be automatically
>>> inferred from remote-tracking branches. As it stands, we can do this
>>> with current git, by writing a pre-rebase hook that checks if any of
>>> the commits to-be-rebased are reachable from any remote-tracking
>>> branch.
>>
>> It is nice that we can achieve a large part of this feature with existing
>> infrastructure.  It would be nice if we ship such pre-rebase hook with
>> git, so people can just enable it if they want to use this functionality,
>> like the default pre-commit hook that checks for whitespace errors.
> 
> Yeah. As it is, the pre-rebase hook shipped with v1.7.9 (when
> activated) does something similar (i.e. prevent rewriting 'public'
> commits). However, it's highly workflow-specific, since it determines
> whether the branch being rebased has been merged into "next" or
> "master". IMHO, a hook that tested for reachability from
> remote-tracking refs would be more generally useful. Obviously, the
> two can be combined, and even further combinations may be desirable
> (e.g. also checking for reachability from commits annotated in
> refs/notes/public).

Relying on (default) hooks to implement this feature has the disadvantage
that it wouldn't be turned on by default... while this feature would be
most helpful for users new to git (scared by refuse to push).
 
I am not sure either if everything (wrt. safety net) can be implemented
via hooks.  One thing that I forgot about is preventing rewinding of
branch past the published commit using e.g. "git reset --hard <commit>".
Unless `pre-rewrite` hook could be used for that safety too...

[...]
>> Note however that the safety net, i.e. refusing or warning against attempted
>> rewrite of published history is only part of issue.  Another important part
>> is querying and showing "phase" of a commit.  What I'd like to see is
>> ability to show among others in "git log" and "git show" output if commit
>> was already published or not (and if it is marked 'secret').
> 
> Today, you can use --decorate to display remote-tracking refs in the
> log/show output. However, only the tip commits are decorated, so if
> the commits shown are not at the tip, you're out of luck. I believe
> teaching log/show to decorate _all_ commits that are reachable from
> some given ref(s) should be fairly straightforward.

That would be nice.
 
> If you use 'git notes' to annotate 'public' and 'secret' states, then
> you can also use the --show-notes=<ref> option to let show/log display
> the annotations on 'public'/'secret' commits.

First, in my opinion annotating _all_ commits with their phase is I think
out of question, especially annotating 'public' commits.  I don't think
git-notes mechanism would scale well to annotating every commit; but
perhaps this was tested to work, and I am mistaken. 
 
Second, I have doubts if "phase" is really state of an individual commit,
and not the feature of revision walking.

Take for example the situation where given commit is reference by 
remote-tracking branch 'public/foo', and also by two local branches:
'foo' with upstream 'public/foo', and local branch 'bar' with no upstream.

Now it is quite obvious that this feature should prevent rewriting 'foo'
branch, for which commits are published upstream.  But what about branch
'bar'?  Should we prevent rewriting (e.g. rebase) here too?  What about
rewinding 'bar' to point somewhere else.  What if 'bar' is really detached
HEAD? 

These questions need to be answered...

[...]
>>> Also, if you want to record where 'public' commits have been sent
>>> (other than what can be inferred from the remote-tracking branches),
>>> you could write this into the refs/notes/public annotation.
>>
>> I wonder if this too can be done by hook...
> 
> You're looking for someting like a post-push hook that runs on the
> _client_ after a successful push. AFAIK, that doesn't exist yet. (Not
> to be confused with the receive/update hooks that run on the
> _server_.)

And such hook could react to what was successfully pushed.  Without
such hook we would have to write wrapper around git-push and parse
its output...
 
Nb. such hook could create "fake" remote-tracking branches if given
push-only remote doesn't have them set up.

>>> As for 'secret' commits, you could annotate these on a
>>> refs/notes/secret notes ref, and then teach 'git push' (or whatever
>>> other method for publishing commits you use) to refuse to publish
>>> commits annotated on this notes ref. Possibly we would want to add a
>>> "pre-push" or "pre-publish" hook.
>>
>> Well, addition of pre-push / pre-publish was resisted on the grounds
>> that all it does is something that can be as easy done by hand before
>> push.  Perhaps this new use case would help bring it forward, don't
>> you think?
> 
> Maybe. I didn't follow the original discussion. From my POV, you could
> argue that instead of another hook, you could always write a script
> that does the 'secret' check before invoking 'git push', and then
> you'd use that script instead of 'git push'. But you could argue the
> same point for pretty much all of the other existing hooks (e.g.
> instead of a pre-commit hook you could have your own commit wrapper
> script). So I don't think that's a sufficient argument to refuse the
> existence of a pre-push/publish hook.

Right, This would be for this feature very much like pre-commit hook.

-- 
Jakub Narebski
Poland