git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH v5 9/9] fetch: stop clobbering existing tags without --force

From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
Date
Aug 31, 2018, 20:10 UTC
Message-ID
<20180831201004.12087-10-avarab@gmail.com>
In-Reply-To
<20180830201244.25759-1-avarab@gmail.com>

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>
---
 Documentation/pull-fetch-param.txt | 15 +++++++++++----
 builtin/fetch.c                    | 18 ++++++++++++------
 t/t5516-fetch-push.sh              |  5 +++--
 t/t5612-clone-refspec.sh           |  4 ++--
 4 files changed, 28 insertions(+), 14 deletions(-)
diff --git a/Documentation/pull-fetch-param.txt b/Documentation/pull-fetch-param.txt
index ab9617ad01..293c6b967d 100644
--- a/Documentation/pull-fetch-param.txt
+++ b/Documentation/pull-fetch-param.txt
@@ -43,10 +43,13 @@ same rules apply for fetching as when pushing, see the `<refspec>...`
 section of linkgit:git-push[1] for what those are. Exceptions to those
 rules particular to 'git fetch' are noted below.
 +
-Unlike when pushing with linkgit:git-push[1], any updates to
-`refs/tags/*` will be accepted without `+` in the refspec (or
-`--force`). The receiving promiscuously considers all tag updates from
-a remote to be forced fetches.
+Until Git version 2.20, and unlike when pushing with
+linkgit:git-push[1], any updates to `refs/tags/*` would be accepted
+without `+` in the refspec (or `--force`). The receiving promiscuously
+considered all tag updates from a remote to be forced fetches.  Since
+Git version 2.20, fetching to update `refs/tags/*` work the same way
+as when pushing. I.e. any updates will be rejected without `+` in the
+refspec (or `--force`).
 +
 Unlike when pushing with linkgit:git-push[1], any updates outside of
 `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
 a commit for another commit that's doesn't have the previous commit as
 an ancestor etc.
 +
+Unlike when pushing with linkgit:git-push[1], there is no
+configuration which'll amend these rules, and nothing like a
+`pre-fetch` hook analogous to the `pre-receive` hook.
++
 As with pushing with linkgit:git-push[1], all of the rules described
 above about what's not allowed as an update can be overridden by
 adding an the optional leading `+` to a refspec (or using `--force`
diff --git a/builtin/fetch.c b/builtin/fetch.c
index b0706b3803..ed4ed9d8c4 100644
--- a/builtin/fetch.c
+++ b/builtin/fetch.c
@@ -667,12 +667,18 @@ static int update_local_ref(struct ref *ref,
 
 	if (!is_null_oid(&ref->old_oid) &&
 	    starts_with(ref->name, "refs/tags/")) {
-		int r;
-		r = s_update_ref("updating tag", ref, 0);
-		format_display(display, r ? '!' : 't', _("[tag update]"),
-			       r ? _("unable to update local ref") : NULL,
-			       remote, pretty_ref, summary_width);
-		return r;
+		if (force || ref->force) {
+			int r;
+			r = s_update_ref("updating tag", ref, 0);
+			format_display(display, r ? '!' : 't', _("[tag update]"),
+				       r ? _("unable to update local ref") : NULL,
+				       remote, pretty_ref, summary_width);
+			return r;
+		} else {
+			format_display(display, '!', _("[rejected]"), _("would clobber existing tag"),
+				       remote, pretty_ref, summary_width);
+			return 1;
+		}
 	}
 
 	current = lookup_commit_reference_gently(the_repository,
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index 7f3d4c4965..0e758e2a43 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -1015,7 +1015,7 @@ test_force_fetch_tag () {
 	tag_type_description=$1
 	tag_args=$2
 
-	test_expect_success "fetch will clobber an existing $tag_type_description" "
+	test_expect_success "fetch will not clobber an existing $tag_type_description without --force" "
 		mk_test testrepo heads/master &&
 		mk_child testrepo child1 &&
 		mk_child testrepo child2 &&
@@ -1027,7 +1027,8 @@ test_force_fetch_tag () {
 			git add file1 &&
 			git commit -m 'file1' &&
 			git tag $tag_args testTag &&
-			git -C ../child1 fetch origin tag testTag
+			test_must_fail git -C ../child1 fetch origin tag testTag &&
+			git -C ../child1 fetch origin '+refs/tags/*:refs/tags/*'
 		)
 	"
 }
diff --git a/t/t5612-clone-refspec.sh b/t/t5612-clone-refspec.sh
index 5582b3d5fd..e36ac01661 100755
--- a/t/t5612-clone-refspec.sh
+++ b/t/t5612-clone-refspec.sh
@@ -103,7 +103,7 @@ test_expect_success 'clone with --no-tags' '
 test_expect_success '--single-branch while HEAD pointing at master' '
 	(
 		cd dir_master &&
-		git fetch &&
+		git fetch --force &&
 		git for-each-ref refs/remotes/origin |
 		sed -e "/HEAD$/d" \
 		    -e "s|/remotes/origin/|/heads/|" >../actual
@@ -114,7 +114,7 @@ test_expect_success '--single-branch while HEAD pointing at master' '
 	test_cmp expect actual &&
 	(
 		cd dir_master &&
-		git fetch --tags &&
+		git fetch --tags --force &&
 		git for-each-ref refs/tags >../actual
 	) &&
 	git for-each-ref refs/tags >expect &&

-- 
2.19.0.rc1.350.ge57e33dbd1
Previous: Ævar Arnfjörð BjarmasonNext: Ævar Arnfjörð Bjarmason
Message 64 of 101 in “Fetching tags overwrites existing tags”
  1. Wink SavilleApr 24, 2018
  2. Jacob KellerApr 24, 2018
  3. Junio C HamanoApr 25, 2018
  4. Jacob KellerApr 25, 2018
  5. Wink SavilleApr 25, 2018
  6. Wink SavilleApr 26, 2018
  7. Junio C HamanoApr 26, 2018
  8. Junio C HamanoApr 26, 2018
  9. Teach remote add the --prefix-tags optionWink Saville, Apr 27, 2018
  10. Wink SavilleApr 27, 2018
  11. Bryan TurnerApr 27, 2018
  12. Jacob KellerMay 4, 2018
  13. Jacob KellerApr 28, 2018
  14. Teach remote add the --remote-tags optionWink Saville, Apr 28, 2018
  15. Wink SavilleApr 28, 2018
  16. Wink SavilleApr 28, 2018
  17. 0/3 Optional sub hierarchy for remote tagsWink Saville, May 1, 2018
  18. 1/3 Teach remote add the --remote-tags optionWink Saville, May 1, 2018
  19. Ævar Arnfjörð BjarmasonMay 1, 2018
  20. Kaartic SivaraamMay 8, 2018
  21. 2/3 Teach tag to list remote-tagsWink Saville, May 1, 2018
  22. 3/3 Test git remote add -f --remote-tagsWink Saville, May 1, 2018
  23. Ævar Arnfjörð BjarmasonMay 1, 2018
  24. Jacob KellerMay 1, 2018
  25. Wink SavilleMay 1, 2018
  26. Junio C HamanoMay 1, 2018
  27. Jacob KellerMay 2, 2018
  28. Junio C HamanoMay 1, 2018
  29. Ævar Arnfjörð BjarmasonApr 27, 2018
  30. 0/8 "git fetch" should not clobber existing tags without --forceÆvar Arnfjörð Bjarmason, Apr 29, 2018
  31. 1/8 push tests: remove redundant 'git push' invocationÆvar Arnfjörð Bjarmason, Apr 29, 2018
  32. 2/8 push tests: fix logic error in "push" test assertionÆvar Arnfjörð Bjarmason, Apr 29, 2018
  33. 3/8 push tests: add more testing for forced tag pushingÆvar Arnfjörð Bjarmason, Apr 29, 2018
  34. Kaartic SivaraamMay 7, 2018
  35. Junio C HamanoMay 8, 2018
  36. Junio C HamanoMay 8, 2018
  37. Kaartic SivaraamMay 8, 2018
  38. Kaartic SivaraamMay 8, 2018
  39. 4/8 push tests: assert re-pushing annotated tagsÆvar Arnfjörð Bjarmason, Apr 29, 2018
  40. Junio C HamanoMay 8, 2018
  41. SZEDER GáborMay 8, 2018
  42. 6/8 fetch tests: correct a comment "remove it" -> "remove them"Ævar Arnfjörð Bjarmason, Apr 29, 2018
  43. 5/8 push doc: correct lies about how push refspecs workÆvar Arnfjörð Bjarmason, Apr 29, 2018
  44. Junio C HamanoMay 8, 2018
  45. 8/8 fetch: stop clobbering existing tags without --forceÆvar Arnfjörð Bjarmason, Apr 29, 2018
  46. Junio C HamanoMay 8, 2018
  47. 7/8 fetch tests: add a test clobbering tag behaviorÆvar Arnfjörð Bjarmason, Apr 29, 2018
  48. 00/10 "git fetch" should not clobber existing tags without --forceÆvar Arnfjörð Bjarmason, Jul 31, 2018
  49. 0/7 Prep for "git fetch" should not clobber existing tags without --forceÆvar Arnfjörð Bjarmason, Aug 13, 2018
  50. Junio C HamanoAug 13, 2018
  51. Ævar Arnfjörð BjarmasonAug 13, 2018
  52. 0/6 "git fetch" should not clobber existing tags without --forceÆvar Arnfjörð Bjarmason, Aug 30, 2018
  53. 0/9 git fetch" should not clobber existing tags without --forceÆvar Arnfjörð Bjarmason, Aug 31, 2018
  54. 1/9 fetch: change "branch" to "reference" in --force -h outputÆvar Arnfjörð Bjarmason, Aug 31, 2018
  55. 2/9 push tests: make use of unused $1 in test descriptionÆvar Arnfjörð Bjarmason, Aug 31, 2018
  56. Junio C HamanoAug 31, 2018
  57. Ævar Arnfjörð BjarmasonAug 31, 2018
  58. 3/9 push tests: use spaces in interpolated stringÆvar Arnfjörð Bjarmason, Aug 31, 2018
  59. 4/9 fetch tests: add a test for clobbering tag behaviorÆvar Arnfjörð Bjarmason, Aug 31, 2018
  60. 5/9 push doc: remove confusing mention of remote mergerÆvar Arnfjörð Bjarmason, Aug 31, 2018
  61. 6/9 push doc: move mention of "tag <tag>" later in the proseÆvar Arnfjörð Bjarmason, Aug 31, 2018
  62. 7/9 push doc: correct lies about how push refspecs workÆvar Arnfjörð Bjarmason, Aug 31, 2018
  63. 8/9 fetch: document local ref updates with/without --forceÆvar Arnfjörð Bjarmason, Aug 31, 2018
  64. 9/9 fetch: stop clobbering existing tags without --forceÆvar Arnfjörð Bjarmason, Aug 31, 2018
  65. 1/6 fetch: change "branch" to "reference" in --force -h outputÆvar Arnfjörð Bjarmason, Aug 30, 2018
  66. 2/6 push tests: correct quoting in interpolated stringÆvar Arnfjörð Bjarmason, Aug 30, 2018
  67. Junio C HamanoAug 30, 2018
  68. 3/6 fetch tests: add a test for clobbering tag behaviorÆvar Arnfjörð Bjarmason, Aug 30, 2018
  69. Junio C HamanoAug 30, 2018
  70. 4/6 push doc: correct lies about how push refspecs workÆvar Arnfjörð Bjarmason, Aug 30, 2018
  71. Junio C HamanoAug 30, 2018
  72. Ævar Arnfjörð BjarmasonAug 30, 2018
  73. Junio C HamanoAug 31, 2018
  74. Ævar Arnfjörð BjarmasonAug 31, 2018
  75. 5/6 fetch: document local ref updates with/without --forceÆvar Arnfjörð Bjarmason, Aug 30, 2018
  76. 6/6 fetch: stop clobbering existing tags without --forceÆvar Arnfjörð Bjarmason, Aug 30, 2018
  77. Junio C HamanoAug 30, 2018
  78. 2/7 push tests: remove redundant 'git push' invocationÆvar Arnfjörð Bjarmason, Aug 13, 2018
  79. 3/7 push tests: fix logic error in "push" test assertionÆvar Arnfjörð Bjarmason, Aug 13, 2018
  80. 4/7 push tests: add more testing for forced tag pushingÆvar Arnfjörð Bjarmason, Aug 13, 2018
  81. 5/7 push tests: assert re-pushing annotated tagsÆvar Arnfjörð Bjarmason, Aug 13, 2018
  82. 6/7 fetch tests: correct a comment "remove it" -> "remove them"Ævar Arnfjörð Bjarmason, Aug 13, 2018
  83. 7/7 pull doc: fix a long-standing grammar errorÆvar Arnfjörð Bjarmason, Aug 13, 2018
  84. 1/7 fetch tests: change "Tag" test tag to "testTag"Ævar Arnfjörð Bjarmason, Aug 13, 2018
  85. 01/10 fetch tests: change "Tag" test tag to "testTag"Ævar Arnfjörð Bjarmason, Jul 31, 2018
  86. 02/10 push tests: remove redundant 'git push' invocationÆvar Arnfjörð Bjarmason, Jul 31, 2018
  87. 03/10 push tests: fix logic error in "push" test assertionÆvar Arnfjörð Bjarmason, Jul 31, 2018
  88. 04/10 push tests: add more testing for forced tag pushingÆvar Arnfjörð Bjarmason, Jul 31, 2018
  89. 05/10 push tests: assert re-pushing annotated tagsÆvar Arnfjörð Bjarmason, Jul 31, 2018
  90. 06/10 push doc: correct lies about how push refspecs workÆvar Arnfjörð Bjarmason, Jul 31, 2018
  91. Junio C HamanoJul 31, 2018
  92. Ævar Arnfjörð BjarmasonAug 30, 2018
  93. Junio C HamanoAug 30, 2018
  94. Ævar Arnfjörð BjarmasonAug 30, 2018
  95. 08/10 fetch tests: add a test clobbering tag behaviorÆvar Arnfjörð Bjarmason, Jul 31, 2018
  96. Junio C HamanoJul 31, 2018
  97. 07/10 fetch tests: correct a comment "remove it" -> "remove them"Ævar Arnfjörð Bjarmason, Jul 31, 2018
  98. 09/10 pull doc: fix a long-standing grammar errorÆvar Arnfjörð Bjarmason, Jul 31, 2018
  99. 10/10 fetch: stop clobbering existing tags without --forceÆvar Arnfjörð Bjarmason, Jul 31, 2018
  100. Junio C HamanoJul 31, 2018
  101. Wink SavilleMay 1, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.