Re: [PATCH] git-push: update remote tags only with force

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 [off-list ref] wrote:

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.

+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