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

[PATCH v6 5/8] push: require force for refs under refs/tags/

From
Chris Rorvick <chris@rorvick.com>
Date
Nov 30, 2012, 01:41 UTC
Message-ID
<1354239700-3325-6-git-send-email-chris@rorvick.com>
In-Reply-To
<1354239700-3325-1-git-send-email-chris@rorvick.com>

References are allowed to update from one commit-ish to another if the former is an 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 something under refs/tags/ should be rejected unless the update is forced.

Signed-off-by: Chris Rorvick <chris@rorvick.com>
---
 Documentation/git-push.txt | 11 ++++++-----
 builtin/push.c             |  2 +-
 builtin/send-pack.c        |  5 +++++
 cache.h                    |  1 +
 remote.c                   | 18 ++++++++++++++----
 send-pack.c                |  1 +
 t/t5516-fetch-push.sh      | 23 ++++++++++++++++++++++-
 transport-helper.c         |  6 ++++++
 transport.c                |  8 ++++++--
 9 files changed, 62 insertions(+), 13 deletions(-)
diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt
index fe46c42..09bdec7 100644
--- a/Documentation/git-push.txt
+++ b/Documentation/git-push.txt
@@ -51,11 +51,12 @@ be named. If `:`<dst> is omitted, the same ref as <src> will be
 updated.
 +
 The object referenced by <src> is used to update the <dst> reference
-on the remote side, but by default this is only allowed if the
-update can fast-forward <dst>.  By having the optional leading `+`,
-you can tell git to update the <dst> ref even when the update is not a
-fast-forward.  This does *not* attempt to merge <src> into <dst>.  See
-EXAMPLES below for details.
+on the remote side.  By default this is only allowed if <dst> is not
+under refs/tags/, and then only if it can fast-forward <dst>.  By having
+the optional leading `+`, you can tell git to update the <dst> ref even
+if it is not allowed by default (e.g., it is not a fast-forward.)  This
+does *not* attempt to merge <src> into <dst>.  See EXAMPLES below for
+details.
 +
 `tag <tag>` means the same as `refs/tags/<tag>:refs/tags/<tag>`.
 +
diff --git a/builtin/push.c b/builtin/push.c
index e08485d..83a3cc8 100644
--- a/builtin/push.c
+++ b/builtin/push.c
@@ -222,7 +222,7 @@ static const char message_advice_checkout_pull_push[] =
 
 static const char message_advice_ref_already_exists[] =
 	N_("Updates were rejected because the destination reference already exists\n"
-	   "in the remote and the update is not a fast-forward.");
+	   "in the remote.");
 
 static void advise_pull_before_push(void)
 {
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 9f98607..f849e0a 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -44,6 +44,11 @@ static void print_helper_status(struct ref *ref)
 			msg = "non-fast forward";
 			break;
 
+		case REF_STATUS_REJECT_ALREADY_EXISTS:
+			res = "error";
+			msg = "already exists";
+			break;
+
 		case REF_STATUS_REJECT_NODELETE:
 		case REF_STATUS_REMOTE_REJECT:
 			res = "error";
diff --git a/cache.h b/cache.h
index b7ab4ac..a32a0ea 100644
--- a/cache.h
+++ b/cache.h
@@ -1011,6 +1011,7 @@ struct ref {
 		REF_STATUS_NONE = 0,
 		REF_STATUS_OK,
 		REF_STATUS_REJECT_NONFASTFORWARD,
+		REF_STATUS_REJECT_ALREADY_EXISTS,
 		REF_STATUS_REJECT_NODELETE,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
diff --git a/remote.c b/remote.c
index 4a6f822..012b52f 100644
--- a/remote.c
+++ b/remote.c
@@ -1315,14 +1315,18 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		 *
 		 * (1) if the old thing does not exist, it is OK.
 		 *
-		 * (2) if you do not have the old thing, you are not allowed
+		 * (2) if the destination is under refs/tags/ you are
+		 *     not allowed to overwrite it; tags are expected
+		 *     to be static once created
+		 *
+		 * (3) if you do not have the old thing, you are not allowed
 		 *     to overwrite it; you would not know what you are losing
 		 *     otherwise.
 		 *
-		 * (3) if both new and old are commit-ish, and new is a
+		 * (4) if both new and old are commit-ish, and new is a
 		 *     descendant of old, it is OK.
 		 *
-		 * (4) regardless of all of the above, removing :B is
+		 * (5) regardless of all of the above, removing :B is
 		 *     always allowed.
 		 */
 
@@ -1337,7 +1341,13 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 				!has_sha1_file(ref->old_sha1)
 				  || !ref_newer(ref->new_sha1, ref->old_sha1);
 
-			if (ref->nonfastforward) {
+			if (ref->not_forwardable) {
+				ref->requires_force = 1;
+				if (!force_ref_update) {
+					ref->status = REF_STATUS_REJECT_ALREADY_EXISTS;
+					continue;
+				}
+			} else if (ref->nonfastforward) {
 				ref->requires_force = 1;
 				if (!force_ref_update) {
 					ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
diff --git a/send-pack.c b/send-pack.c
index f50dfd9..1c375f0 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -229,6 +229,7 @@ int send_pack(struct send_pack_args *args,
 		/* Check for statuses set by set_ref_status_for_push() */
 		switch (ref->status) {
 		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_UPTODATE:
 			continue;
 		default:
diff --git a/t/t5516-fetch-push.sh b/t/t5516-fetch-push.sh
index b5417cc..8f024a0 100755
--- a/t/t5516-fetch-push.sh
+++ b/t/t5516-fetch-push.sh
@@ -368,7 +368,7 @@ test_expect_success 'push with colon-less refspec (2)' '
 		git branch -D frotz
 	fi &&
 	git tag -f frotz &&
-	git push testrepo frotz &&
+	git push -f testrepo frotz &&
 	check_push_result $the_commit tags/frotz &&
 	check_push_result $the_first_commit heads/frotz
 
@@ -929,6 +929,27 @@ test_expect_success 'push into aliased refs (inconsistent)' '
 	)
 '
 
+test_expect_success 'push requires --force to update lightweight tag' '
+	mk_test heads/master &&
+	mk_child child1 &&
+	mk_child child2 &&
+	(
+		cd child1 &&
+		git tag Tag &&
+		git push ../child2 Tag &&
+		git push ../child2 Tag &&
+		>file1 &&
+		git add file1 &&
+		git commit -m "file1" &&
+		git tag -f Tag &&
+		test_must_fail git push ../child2 Tag &&
+		git push --force ../child2 Tag &&
+		git tag -f Tag &&
+		test_must_fail git push ../child2 Tag HEAD~ &&
+		git push --force ../child2 Tag
+	)
+'
+
 test_expect_success 'push --porcelain' '
 	mk_empty &&
 	echo >.git/foo  "To testrepo" &&
diff --git a/transport-helper.c b/transport-helper.c
index 4713b69..965b778 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -661,6 +661,11 @@ static void push_update_ref_status(struct strbuf *buf,
 			free(msg);
 			msg = NULL;
 		}
+		else if (!strcmp(msg, "already exists")) {
+			status = REF_STATUS_REJECT_ALREADY_EXISTS;
+			free(msg);
+			msg = NULL;
+		}
 	}
 
 	if (*ref)
@@ -720,6 +725,7 @@ static int push_refs_with_push(struct transport *transport,
 		/* Check for statuses set by set_ref_status_for_push() */
 		switch (ref->status) {
 		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_REJECT_ALREADY_EXISTS:
 		case REF_STATUS_UPTODATE:
 			continue;
 		default:
diff --git a/transport.c b/transport.c
index f3160b1..2673d27 100644
--- a/transport.c
+++ b/transport.c
@@ -695,6 +695,10 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count, i
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 						 "non-fast-forward", porcelain);
 		break;
+	case REF_STATUS_REJECT_ALREADY_EXISTS:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+						 "already exists", porcelain);
+		break;
 	case REF_STATUS_REMOTE_REJECT:
 		print_ref_status('!', "[remote rejected]", ref,
 						 ref->deletion ? NULL : ref->peer_ref,
@@ -740,12 +744,12 @@ void transport_print_push_status(const char *dest, struct ref *refs,
 		    ref->status != REF_STATUS_OK)
 			n += print_one_push_status(ref, dest, n, porcelain);
 		if (ref->status == REF_STATUS_REJECT_NONFASTFORWARD) {
-			if (ref->not_forwardable)
-				*reject_reasons |= REJECT_ALREADY_EXISTS;
 			if (!strcmp(head, ref->name))
 				*reject_reasons |= REJECT_NON_FF_HEAD;
 			else
 				*reject_reasons |= REJECT_NON_FF_OTHER;
+		} else if (ref->status == REF_STATUS_REJECT_ALREADY_EXISTS) {
+			*reject_reasons |= REJECT_ALREADY_EXISTS;
 		}
 	}
 }

-- 
1.8.0.158.g0c4328c
Previous: Chris RorvickNext: Chris Rorvick
Message 10 of 61 in “push: update remote tags only with force”
  1. 0/8 push: update remote tags only with forceChris Rorvick, Nov 30, 2012
  2. 1/8 push: return reject reasons as a bitsetChris Rorvick, Nov 30, 2012
  3. 2/8 push: add advice for rejected tag referenceChris Rorvick, Nov 30, 2012
  4. Junio C HamanoDec 2, 2012
  5. 0/2 push: honor advice.* configurationChris Rorvick, Dec 3, 2012
  6. 1/2 push: rename config variable for more general useChris Rorvick, Dec 3, 2012
  7. 2/2 push: allow already-exists advice to be disabledChris Rorvick, Dec 3, 2012
  8. 3/8 push: flag updatesChris Rorvick, Nov 30, 2012
  9. 4/8 push: flag updates that require forceChris Rorvick, Nov 30, 2012
  10. 5/8 push: require force for refs under refs/tags/Chris Rorvick, Nov 30, 2012
  11. 6/8 push: require force for annotated tagsChris Rorvick, Nov 30, 2012
  12. 7/8 push: clarify rejection of update to non-commit-ishChris Rorvick, Nov 30, 2012
  13. 8/8 push: cleanup push rules commentChris Rorvick, Nov 30, 2012
  14. remote.c: fix grammatical error in commentChris Rorvick, Dec 2, 2012
  15. Junio C HamanoDec 3, 2012
  16. Max HornJan 16, 2013
  17. Junio C HamanoJan 16, 2013
  18. Jeff KingJan 16, 2013
  19. Junio C HamanoJan 16, 2013
  20. Jeff KingJan 16, 2013
  21. Junio C HamanoJan 16, 2013
  22. Chris RorvickJan 17, 2013
  23. Jeff KingJan 17, 2013
  24. Chris RorvickJan 17, 2013
  25. Junio C HamanoJan 16, 2013
  26. Junio C HamanoJan 16, 2013
  27. Chris RorvickJan 17, 2013
  28. Junio C HamanoJan 17, 2013
  29. Chris RorvickJan 17, 2013
  30. Jeff KingJan 18, 2013
  31. Chris RorvickJan 18, 2013
  32. Jeff KingJan 21, 2013
  33. Junio C HamanoJan 21, 2013
  34. Chris RorvickJan 22, 2013
  35. Junio C HamanoJan 22, 2013
  36. 0/3 Finishing touches to "push" advisesJunio C Hamano, Jan 22, 2013
  37. 1/3 push: further clean up fields of "struct ref"Junio C Hamano, Jan 22, 2013
  38. 2/3 push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCEJunio C Hamano, Jan 22, 2013
  39. Junio C HamanoJan 22, 2013
  40. 3/3 push: further reduce "struct ref" and simplify the logicJunio C Hamano, Jan 22, 2013
  41. Junio C HamanoJan 22, 2013
  42. 0/3 Finishing touches to "push" advisesJunio C Hamano, Jan 22, 2013
  43. 1/3 push: further clean up fields of "struct ref"Junio C Hamano, Jan 22, 2013
  44. Jeff KingJan 23, 2013
  45. 2/3 push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCEJunio C Hamano, Jan 22, 2013
  46. Jeff KingJan 23, 2013
  47. Junio C HamanoJan 23, 2013
  48. Jeff KingJan 24, 2013
  49. 3/3 push: further simplify the logic to assign rejection statusJunio C Hamano, Jan 22, 2013
  50. Junio C HamanoJan 22, 2013
  51. 0/3 Finishing touches to "push" advisesJunio C Hamano, Jan 23, 2013
  52. 1/3 push: further clean up fields of "struct ref"Junio C Hamano, Jan 23, 2013
  53. Eric SunshineJan 24, 2013
  54. 2/3 push: further simplify the logic to assign rejection reasonJunio C Hamano, Jan 23, 2013
  55. 3/3 push: introduce REJECT_FETCH_FIRST and REJECT_NEEDS_FORCEJunio C Hamano, Jan 23, 2013
  56. Jeff KingJan 24, 2013
  57. Junio C HamanoJan 24, 2013
  58. Chris RorvickJan 25, 2013
  59. Junio C HamanoJan 25, 2013
  60. Chris RorvickJan 25, 2013
  61. Junio C HamanoJan 18, 2013

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.