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

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



On Sun, Feb 5, 2012 at 21:46, Jakub Narebski <jnareb@xxxxxxxxx> wrote:
> On Sun, 5 Feb 2012, Johan Herland wrote:
>> 2012/2/5 Jakub Narebski <jnareb@xxxxxxxxx>:
>> > > 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).

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

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

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

>> 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@xxxxxxxxxxx>
www.herland.net
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at  http://vger.kernel.org/majordomo-info.html


[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]