Re: [PATCH] git-push: update remote tags only with force
- From
Felipe Contreras <felipe.contreras@gmail.com>
- Date
- Oct 31, 2012, 05:55 UTC
- Message-ID
- <CAMP44s2T9Rmfjd8r+2+eYh8JBPXEofm3cHuEkkY+R3cW6R6HxA@mail.gmail.com>
- In-Reply-To
- <1351661875-4307-1-git-send-email-chris@rorvick.com>
Hi,
(again because the mailing list rejected it) (Gmal switched interface and HTML is the default)
On Wed, Oct 31, 2012 at 6:37 AM, Chris Rorvick <chris@rorvick.com> wrote:
Show 27 quoted lines
> > References are allowed to update from one commit-ish to another if the > former is a ancestor of the latter. This behavior is oriented to > branches which are expected to move with commits. Tag references are > expected to be static in a repository, though, thus an update to a > tag (lightweight and annotated) should be rejected unless the update is > forced. > > To enable this functionality, the following checks have been added to > set_ref_status_for_push() for updating refs (i.e, not new or deletion) > to restrict fast-forwarding in pushes: > > 1) The old and new references must be commits. If this fails, > it is not a valid update for a branch. > > 2) The reference name cannot start with "refs/tags/". This > catches lightweight tags which (usually) point to commits > and therefore would not be caught by (1). > > If either of these checks fails, then it is flagged (by default) with a > status indicating the update is being rejected due to the reference > already existing in the remote. This can be overridden by passing > --force to git push. > > The new status has the added benefit of being able to provide accurate > feedback as to why the ref update failed and what can be done. > Currently all ref update rejections are assumed to be for branches.
Makes sense to me. I've believe I've been hit by this a couple of times when tags were updated, and a colleague did 'git push' and they went all back, or something like that. To handle that case properly probably more changes are needed, but this is a change in the right direction.
Show 16 quoted lines
> +test_expect_success 'push tag requires --force to update remote tag' ' > + mk_test heads/master && > + mk_child child1 && > + mk_child child2 && > + ( > + cd child1 && > + git tag lw_tag && > + git tag -a -m "message 1" ann_tag && > + git push ../child2 lw_tag && > + git push ../child2 ann_tag && > + >file1 && > + git add file1 && > + git commit -m "file1" && > + git tag -f lw_tag && > + git tag -f -a -m "message 2" ann_tag && > + ! git push ../child2 lw_tag &&
You probably should use test_must_fail.
I don't see anything wrong with the patch, but I wonder if it might be possible to split it to ease the review.
Cheers.
-- Felipe Contreras