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

From: Johan Herland <johan@herland.net>
Date: Feb 5, 2012, 22:49 UTC
Message-ID: <CALKQrgcAsPXziQCTReZkCKnnXTX=rwPFrzp0wJ3ZYwn0b_M5Tw@mail.gmail.com>
In-Reply-To: <201202052146.56458.jnareb@gmail.com>

On Sun, Feb 5, 2012 at 21:46, Jakub Narebski <jnareb@gmail.com> wrote:

Show 20 quoted lines

> On Sun, 5 Feb 2012, Johan Herland wrote:
>> 2012/2/5 Jakub Narebski <jnareb@gmail.com>:
>> > > Being able to mark temporary, out of sequence or other hacks as Secret could
>> > > be useful, as would recording where Public commits had been sent.
>> >
>> > Marking as 'secret' must I think be explicit, but I think 'public' phase
>> > should be inferred from remote-tracking branches.  The idea of phases is
>> > to allow UI to ask about status of commits: can we amend / rebase it or
>> > not, can we push it or not.
>>
>> 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).

Show 16 quoted lines

>> Unfortunately, the pre-rebase hook only affects 'git rebase', and in
>> order to do the same check on 'git commit --amend' you'd have to write
>> a similar pre-commit hook (don't know how easy it is to find the
>> amended commit from within the hook). Maybe we should add a
>> pre-rewrite hook that trigger in the same situations as the
>> post-rewrite hook.
>
> pre-rewrite hook would be a really nice to have, especially that it would
> (I hope) cover third party tools like various GUIs for git; and also
> git-filter-branch.
>
> 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.

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.

Show 11 quoted lines

>> This should take care of the simplest 'public' use case in a
>> push-based workflow. If you publish commits by other means
>> (send-email, bundles, pulling directly from your repo), you need some
>> other way to mark the 'public' commits. One solution would be using
>> 'git notes' to annotate the 'public' commits on a 'refs/notes/public'
>> notes ref. Your pre-rebase/pre-rewrite hook should then check if any
>> of the commits to-be-rewritten are reachable from any commit annotated
>> as 'public'.
>
> Another solution would be to create "fake" remote-tracking branches
> by git-bundle and git-send-email.

Good point. Creating such "fake" remote-tracking branches might be a good idea in those workflows anyway, simply to keep track of what has been shared, and where.

Show 5 quoted lines

>> 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_.)

Show 10 quoted lines

>> 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.

Have fun! :)

...Johan

-- 
Johan Herland, <johan@herland.net>
www.herland.net