@matheusbsbs / matheus-bagliano / commits / 0bc8d71b99

fetch: stop clobbering existing tags without --force

Change "fetch" to treat "+" in refspecs (aka --force) to mean we should clobber a local tag of the same name. This changes the long-standing behavior of "fetch" added in 853a3697dc ("[PATCH] Multi-head fetch.", 2005-08-20). Before this change, all tag fetches effectively had --force enabled. See the git-fetch-script code in fast_forward_local() with the comment: > Tags need not be pointing at commits so there is no way to > guarantee "fast-forward" anyway. That commit and the rest of the history of "fetch" shows that the "+" (--force) part of refpecs was only conceived for branch updates, while tags have accepted any changes from upstream unconditionally and clobbered the local tag object. Changing this behavior has been discussed as early as 2011[1]. The current behavior doesn't make sense to me, it easily results in local tags accidentally being clobbered. We could namespace our tags per-remote and not locally populate refs/tags/*, but as with my 97716d217c ("fetch: add a --prune-tags option and fetch.pruneTags config", 2018-02-09) it's easier to work around the current implementation than to fix the root cause. So this change implements suggestion #1 from Jeff's 2011 E-Mail[1], "fetch" now only clobbers the tag if either "+" is provided as part of the refspec, or if "--force" is provided on the command-line. This also makes it nicely symmetrical with how "tag" itself works when creating tags. I.e. we refuse to clobber any existing tags unless "--force" is supplied. Now we can refuse all such clobbering, whether it would happen by clobbering a local tag with "tag", or by fetching it from the remote with "fetch". Ref updates outside refs/{tags,heads/* are still still not symmetrical with how "git push" works, as discussed in the recently changed pull-fetch-param.txt documentation. This change brings the two divergent behaviors more into line with one another. I don't think there's any reason "fetch" couldn't fully converge with the behavior used by "push", but that's a topic for another change. One of the tests added in 31b808a032 ("clone --single: limit the fetch refspec to fetched branch", 2012-09-20) is being changed to use --force where a clone would clobber a tag. This changes nothing about the existing behavior of the test. 1. https://public-inbox.org/git/20111123221658.GA22313@sigill.intra.peff.net/ Signed-off-by: Ævar Arnfjörð Bjarmason <avarab@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Ævar Arnfjörð Bjarmason committed Aug 31, 2018 at 20:10 UTC 0bc8d71b99e91c9e90b519073b639a5066119591

4 files changed +28 -14

Documentation/pull-fetch-param.txt

+11 -4

@@ -43,10 +43,13 @@ same rules apply for fetching as when pushing, see the `<refspec>...`

43 section of linkgit:git-push[1] for what those are. Exceptions to those

44 rules particular to 'git fetch' are noted below.

45 +

46 -Unlike when pushing with linkgit:git-push[1], any updates to

47 -`refs/tags/*` will be accepted without `+` in the refspec (or

48 -`--force`). The receiving promiscuously considers all tag updates from

49 -a remote to be forced fetches.

46 +Until Git version 2.20, and unlike when pushing with

47 +linkgit:git-push[1], any updates to `refs/tags/*` would be accepted

48 +without `+` in the refspec (or `--force`). The receiving promiscuously

49 +considered all tag updates from a remote to be forced fetches. Since

50 +Git version 2.20, fetching to update `refs/tags/*` work the same way

51 +as when pushing. I.e. any updates will be rejected without `+` in the

52 +refspec (or `--force`).

53 +

54 Unlike when pushing with linkgit:git-push[1], any updates outside of

55 `refs/{tags,heads}/*` will be accepted without `+` in the refspec (or

@@ -54,6 +57,10 @@ Unlike when pushing with linkgit:git-push[1], any updates outside of

57 a commit for another commit that's doesn't have the previous commit as

58 an ancestor etc.

59 +

60 +Unlike when pushing with linkgit:git-push[1], there is no

61 +configuration which'll amend these rules, and nothing like a

62 +`pre-fetch` hook analogous to the `pre-receive` hook.

63 ++

64 As with pushing with linkgit:git-push[1], all of the rules described

65 above about what's not allowed as an update can be overridden by

66 adding an the optional leading `+` to a refspec (or using `--force`

builtin/fetch.c

+12 -6

@@ -664,12 +664,18 @@ static int update_local_ref(struct ref *ref,

664

665 if (!is_null_oid(&ref->old_oid) &&

666 starts_with(ref->name, "refs/tags/")) {

667 - int r;

668 - r = s_update_ref("updating tag", ref, 0);

669 - format_display(display, r ? '!' : 't', _("[tag update]"),

670 - r ? _("unable to update local ref") : NULL,

671 - remote, pretty_ref, summary_width);

672 - return r;

667 + if (force || ref->force) {

668 + int r;

669 + r = s_update_ref("updating tag", ref, 0);

670 + format_display(display, r ? '!' : 't', _("[tag update]"),

671 + r ? _("unable to update local ref") : NULL,

672 + remote, pretty_ref, summary_width);

673 + return r;

674 + } else {

675 + format_display(display, '!', _("[rejected]"), _("would clobber existing tag"),

676 + remote, pretty_ref, summary_width);

677 + return 1;

678 + }

679 }

680

681 current = lookup_commit_reference_gently(&ref->old_oid, 1);

t/t5516-fetch-push.sh

+3 -2

@@ -1015,7 +1015,7 @@ test_force_fetch_tag () {

1015 tag_type_description=$1

1016 tag_args=$2

1017

1018 - test_expect_success "fetch will clobber an existing $tag_type_description" "

1018 + test_expect_success "fetch will not clobber an existing $tag_type_description without --force" "

1019 mk_test testrepo heads/master &&

1020 mk_child testrepo child1 &&

1021 mk_child testrepo child2 &&

@@ -1027,7 +1027,8 @@ test_force_fetch_tag () {

1027 git add file1 &&

1028 git commit -m 'file1' &&

1029 git tag $tag_args testTag &&

1030 - git -C ../child1 fetch origin tag testTag

1030 + test_must_fail git -C ../child1 fetch origin tag testTag &&

1031 + git -C ../child1 fetch origin '+refs/tags/*:refs/tags/*'

1032 )

1033 "

1034 }

t/t5612-clone-refspec.sh

+2 -2

@@ -104,7 +104,7 @@ test_expect_success 'clone with --no-tags' '

104 test_expect_success '--single-branch while HEAD pointing at master' '

105 (

106 cd dir_master &&

107 - git fetch &&

107 + git fetch --force &&

108 git for-each-ref refs/remotes/origin |

109 sed -e "/HEAD$/d" \

110 -e "s|/remotes/origin/|/heads/|" >../actual

@@ -115,7 +115,7 @@ test_expect_success '--single-branch while HEAD pointing at master' '

115 test_cmp expect actual &&

116 (

117 cd dir_master &&

118 - git fetch --tags &&

118 + git fetch --tags --force &&

119 git for-each-ref refs/tags >../actual

120 ) &&

121 git for-each-ref refs/tags >expect &&