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

[PATCH v3 3/6] refactor ref status logic for pushing

From
Tay Ray Chuan <rctay89@gmail.com>
Date
Dec 24, 2009, 07:42 UTC
Message-ID
<20091224154258.08b4fe44.rctay89@gmail.com>
In-Reply-To
<20091224154158.15ba580f.rctay89@gmail.com>

Move the logic that detects up-to-date and non-fast-forward refs to a new function in remote.[ch], set_ref_status_for_push().

Make transport_push() invoke set_ref_status_for_push() before invoking the push_refs() implementation. (As a side-effect, the push_refs() implementation in transport-helper.c now knows of non-fast-forward pushes.)

Removed logic for detecting up-to-date refs from the push_refs() implementation in transport-helper.c, as transport_push() has already done so for it.

Make cmd_send_pack() invoke set_ref_status_for_push() before invoking send_pack(), as transport_push() can't do it for send_pack() here.

Mark the test on the return status of non-fast-forward push to fail. Git now exits with success, as transport.c::transport_push() does not check for refs with status REF_STATUS_REJECT_NONFASTFORWARD nor does it indicate rejected pushes with its return value.

Mark the test for ref status to succeed. As mentioned earlier, refs might be marked as non-fast-forwards, triggering the push status printing mechanism in transport.c.

Signed-off-by: Tay Ray Chuan <rctay89@gmail.com>
---

Changed nothing from v2 except tests and mentioning it in the commit message.

 builtin-send-pack.c  |   50 +++++++++++---------------------------------------
 remote.c             |   50 ++++++++++++++++++++++++++++++++++++++++++++++++++
 remote.h             |    2 ++
 t/t5541-http-push.sh |    4 ++--
 transport-helper.c   |   13 ++++++-------
 transport.c          |    4 ++++
 6 files changed, 75 insertions(+), 48 deletions(-)
diff --git a/builtin-send-pack.c b/builtin-send-pack.c
index 8fffdbf..38580c3 100644
--- a/builtin-send-pack.c
+++ b/builtin-send-pack.c
@@ -406,50 +406,19 @@ int send_pack(struct send_pack_args *args,
 	 */
 	new_refs = 0;
 	for (ref = remote_refs; ref; ref = ref->next) {
-
-		if (ref->peer_ref)
-			hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
-		else if (!args->send_mirror)
+		if (!ref->peer_ref && !args->send_mirror)
 			continue;

-		ref->deletion = is_null_sha1(ref->new_sha1);
-		if (ref->deletion && !allow_deleting_refs) {
-			ref->status = REF_STATUS_REJECT_NODELETE;
-			continue;
-		}
-		if (!ref->deletion &&
-		    !hashcmp(ref->old_sha1, ref->new_sha1)) {
-			ref->status = REF_STATUS_UPTODATE;
+		switch (ref->status) {
+		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_UPTODATE:
 			continue;
+		default:
+			; /* do nothing */
 		}

-		/* This part determines what can overwrite what.
-		 * The rules are:
-		 *
-		 * (0) you can always use --force or +A:B notation to
-		 *     selectively force individual ref pairs.
-		 *
-		 * (1) if the old thing does not exist, it is OK.
-		 *
-		 * (2) 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
-		 *     descendant of old, it is OK.
-		 *
-		 * (4) regardless of all of the above, removing :B is
-		 *     always allowed.
-		 */
-
-		ref->nonfastforward =
-		    !ref->deletion &&
-		    !is_null_sha1(ref->old_sha1) &&
-		    (!has_sha1_file(ref->old_sha1)
-		      || !ref_newer(ref->new_sha1, ref->old_sha1));
-
-		if (ref->nonfastforward && !ref->force && !args->force_update) {
-			ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+		if (ref->deletion && !allow_deleting_refs) {
+			ref->status = REF_STATUS_REJECT_NODELETE;
 			continue;
 		}

@@ -673,6 +642,9 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix)
 	if (match_refs(local_refs, &remote_refs, nr_refspecs, refspecs, flags))
 		return -1;

+	set_ref_status_for_push(remote_refs, args.send_mirror,
+		args.force_update);
+
 	ret = send_pack(&args, fd, conn, remote_refs, &extra_have);

 	if (helper_status)
diff --git a/remote.c b/remote.c
index e3afecd..c70181c 100644
--- a/remote.c
+++ b/remote.c
@@ -1247,6 +1247,56 @@ int match_refs(struct ref *src, struct ref **dst,
 	return 0;
 }

+void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
+	int force_update)
+{
+	struct ref *ref;
+
+	for (ref = remote_refs; ref; ref = ref->next) {
+		if (ref->peer_ref)
+			hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
+		else if (!send_mirror)
+			continue;
+
+		ref->deletion = is_null_sha1(ref->new_sha1);
+		if (!ref->deletion &&
+			!hashcmp(ref->old_sha1, ref->new_sha1)) {
+			ref->status = REF_STATUS_UPTODATE;
+			continue;
+		}
+
+		/* This part determines what can overwrite what.
+		 * The rules are:
+		 *
+		 * (0) you can always use --force or +A:B notation to
+		 *     selectively force individual ref pairs.
+		 *
+		 * (1) if the old thing does not exist, it is OK.
+		 *
+		 * (2) 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
+		 *     descendant of old, it is OK.
+		 *
+		 * (4) regardless of all of the above, removing :B is
+		 *     always allowed.
+		 */
+
+		ref->nonfastforward =
+			!ref->deletion &&
+			!is_null_sha1(ref->old_sha1) &&
+			(!has_sha1_file(ref->old_sha1)
+			  || !ref_newer(ref->new_sha1, ref->old_sha1));
+
+		if (ref->nonfastforward && !ref->force && !force_update) {
+			ref->status = REF_STATUS_REJECT_NONFASTFORWARD;
+			continue;
+		}
+	}
+}
+
 struct branch *branch_get(const char *name)
 {
 	struct branch *ret;
diff --git a/remote.h b/remote.h
index 8b7ecf9..6e13643 100644
--- a/remote.h
+++ b/remote.h
@@ -98,6 +98,8 @@ char *apply_refspecs(struct refspec *refspecs, int nr_refspec,

 int match_refs(struct ref *src, struct ref **dst,
 	       int nr_refspec, const char **refspec, int all);
+void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
+	int force_update);

 /*
  * Given a list of the remote refs and the specification of things to
diff --git a/t/t5541-http-push.sh b/t/t5541-http-push.sh
index 5ebe04a..86dbcb2 100755
--- a/t/t5541-http-push.sh
+++ b/t/t5541-http-push.sh
@@ -88,7 +88,7 @@ test_expect_success 'used receive-pack service' '
 	test_cmp exp act
 '

-test_expect_success 'non-fast-forward push fails' '
+test_expect_failure 'non-fast-forward push fails' '
 	cd "$ROOT_PATH"/test_repo_clone &&
 	git checkout master &&
 	echo "changed" > path2 &&
@@ -100,7 +100,7 @@ test_expect_success 'non-fast-forward push fails' '
 	 test $HEAD != $(git rev-parse --verify HEAD))
 '

-test_expect_failure 'non-fast-forward push show ref status' '
+test_expect_success 'non-fast-forward push show ref status' '
 	grep "^ ! \[rejected\][ ]*master -> master (non-fast-forward)$" output
 '

diff --git a/transport-helper.c b/transport-helper.c
index 11f3d7e..6b1f778 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -329,16 +329,15 @@ static int push_refs(struct transport *transport,
 		return 1;

 	for (ref = remote_refs; ref; ref = ref->next) {
-		if (ref->peer_ref)
-			hashcpy(ref->new_sha1, ref->peer_ref->new_sha1);
-		else if (!mirror)
+		if (!ref->peer_ref && !mirror)
 			continue;

-		ref->deletion = is_null_sha1(ref->new_sha1);
-		if (!ref->deletion &&
-			!hashcmp(ref->old_sha1, ref->new_sha1)) {
-			ref->status = REF_STATUS_UPTODATE;
+		switch (ref->status) {
+		case REF_STATUS_REJECT_NONFASTFORWARD:
+		case REF_STATUS_UPTODATE:
 			continue;
+		default:
+			; /* do nothing */
 		}

 		if (force_all)
diff --git a/transport.c b/transport.c
index 3eea836..12c4423 100644
--- a/transport.c
+++ b/transport.c
@@ -887,6 +887,10 @@ int transport_push(struct transport *transport,
 			return -1;
 		}

+		set_ref_status_for_push(remote_refs,
+			flags & TRANSPORT_PUSH_MIRROR,
+			flags & TRANSPORT_PUSH_FORCE);
+
 		ret = transport->push_refs(transport, remote_refs, flags);

 		if (!quiet || push_had_errors(remote_refs))
--
1.6.6.rc1.249.g048b3
Previous: Tay Ray ChuanNext: Tay Ray Chuan
Message 12 of 25 in “refactor ref status logic for pushing”
  1. 1/3 refactor ref status logic for pushingTay Ray Chuan, Dec 4, 2009
  2. 0/3 transport: catch non-fast-forwardsTay Ray Chuan, Dec 8, 2009
  3. 1/3 refactor ref status logic for pushingTay Ray Chuan, Dec 8, 2009
  4. Daniel BarkalowDec 8, 2009
  5. Tay Ray ChuanDec 9, 2009
  6. Daniel BarkalowDec 9, 2009
  7. 2/3 transport.c::transport_push(): make ref status affect return valueTay Ray Chuan, Dec 8, 2009
  8. 3/3 transport-helper.c::push_refs(): emit "no refs" error messageTay Ray Chuan, Dec 8, 2009
  9. 0/6 transport: catch non-fast forwardsTay Ray Chuan, Dec 24, 2009
  10. 1/6 t5541-http-push.sh: add tests for non-fast-forward pushesTay Ray Chuan, Dec 24, 2009
  11. 2/6 t5541-http-push.sh: add test for unmatched, non-fast-forwarded refsTay Ray Chuan, Dec 24, 2009
  12. 3/6 refactor ref status logic for pushingTay Ray Chuan, Dec 24, 2009
  13. 4/6 transport.c::transport_push(): make ref status affect return valueTay Ray Chuan, Dec 24, 2009
  14. 5/6 transport-helper.c::push_refs(): ignore helper-reported status if ref is not to be pushedTay Ray Chuan, Dec 24, 2009
  15. 6/6 transport-helper.c::push_refs(): emit "no refs" error messageTay Ray Chuan, Dec 24, 2009
  16. Jeff KingJan 5, 2010
  17. Tay Ray ChuanJan 5, 2010
  18. Jeff KingJan 6, 2010
  19. Tay Ray ChuanJan 6, 2010
  20. Tay Ray ChuanJan 8, 2010
  21. Jeff KingJan 5, 2010
  22. Tay Ray ChuanJan 5, 2010
  23. Jeff KingJan 6, 2010
  24. Junio C HamanoJan 6, 2010
  25. Tay Ray ChuanJan 6, 2010

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.