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

[PATCH v5 1/3] push: add reflog check for "--force-if-includes"

From
Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
Date
Sep 23, 2020, 07:30 UTC
Message-ID
<20200923073022.61293-2-shrinidhi.kaushik@gmail.com>
In-Reply-To
<20200923073022.61293-1-shrinidhi.kaushik@gmail.com>

Add a check to verify if the remote-tracking ref of the local branch is reachable from one of its "reflog" entries.

When a local branch that is based on a remote ref, has been rewound and is to be force pushed on the remote, "--force-if-includes" runs a check that ensures any updates to remote-tracking refs that may have happened (by push from another repository) in-between the time of the last checkout, and right before the time of push, have been integrated locally before allowing a forced updated.

A new field "use_force_if_includes" has been added to "push_cas_option", which is set to "1" when "--force-if-includes" is specified as an option in the command line or as a configuration setting.

The struct "ref" has two new bit-fields:
  - check_reachable:
    Set when we have to run the new check on the ref, and the remote
    ref was marked as "use_tracking" or "use_tracking_for_rest" by
    compare-and-swap (if the "the remote tip must be at the expected
    commit" condition is not specified); "apply_push_cas()" has been
    updated to check if this field is set and run the check.
  - unreachable:
    Set if the ref is unreachable from any of the "reflog" entries of
    its local counterpart.

"REF_STATUS_REJECT_REMOTE_UPDATED" has been added to the "status" enum to imply that the ref failed the check; "case" statements in "send-pack", "transport" and "transport-helper" have been updated accordingly to catch this status when set.

When "--force-is-includes" is used along with "--force-with-lease", the check runs only for refs marked as "if_includes". If the option is passed without specifying "--force-with-lease", or specified along with "--force-with-lease=<refname>:<expect>" it is a "no-op".

Signed-off-by: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
---
 builtin/send-pack.c |   5 ++
 remote.c            | 140 +++++++++++++++++++++++++++++++++++++++++++-
 remote.h            |   8 ++-
 send-pack.c         |   1 +
 transport-helper.c  |   5 ++
 transport.c         |   6 ++
 6 files changed, 162 insertions(+), 3 deletions(-)
diff --git a/builtin/send-pack.c b/builtin/send-pack.c
index 2b9610f121..4d76727edb 100644
--- a/builtin/send-pack.c
+++ b/builtin/send-pack.c
@@ -69,6 +69,11 @@ static void print_helper_status(struct ref *ref)
 			msg = "stale info";
 			break;
 
+		case REF_STATUS_REJECT_REMOTE_UPDATED:
+			res = "error";
+			msg = "remote ref updated since checkout";
+			break;
+
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
 			res = "error";
 			msg = "already exists";
diff --git a/remote.c b/remote.c
index eafc14cbe7..0dcac4ab8e 100644
--- a/remote.c
+++ b/remote.c
@@ -1471,12 +1471,23 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 		 * with the remote-tracking branch to find the value
 		 * to expect, but we did not have such a tracking
 		 * branch.
+		 *
+		 * If the tip of the remote-tracking ref is unreachable
+		 * from any reflog entry of its local ref indicating a
+		 * possible update since checkout; reject the push.
 		 */
 		if (ref->expect_old_sha1) {
 			if (!oideq(&ref->old_oid, &ref->old_oid_expect))
 				reject_reason = REF_STATUS_REJECT_STALE;
+			else if (ref->check_reachable && ref->unreachable)
+				reject_reason =
+					REF_STATUS_REJECT_REMOTE_UPDATED;
 			else
-				/* If the ref isn't stale then force the update. */
+				/*
+				 * If the ref isn't stale, and is reachable
+				 * from from one of the reflog entries of
+				 * the local branch, force the update.
+				 */
 				force_ref_update = 1;
 		}
 
@@ -2268,6 +2279,118 @@ static int remote_tracking(struct remote *remote, const char *refname,
 	return 0;
 }
 
+/*
+ * The struct "reflog_commit_list" and related helper functions
+ * for list manipulation are used for collecting commits into a
+ * list during reflog traversals in "if_exists_or_grab_until()".
+ */
+struct reflog_commit_list {
+	struct commit **items;
+	size_t nr, alloc;
+};
+
+/* Adds a commit to list. */
+static void add_commit(struct reflog_commit_list *list, struct commit *commit)
+{
+	ALLOC_GROW(list->items, list->nr + 1, list->alloc);
+	list->items[list->nr++] = commit;
+}
+
+/* Free and reset the list. */
+static void free_reflog_commit_list(struct reflog_commit_list *list)
+{
+	FREE_AND_NULL(list->items);
+	list->nr = list->alloc = 0;
+}
+
+struct check_and_collect_until_cb_data {
+	struct commit *remote_commit;
+	struct reflog_commit_list *local_commits;
+};
+
+
+static int check_and_collect_until(struct object_id *o_oid,
+				   struct object_id *n_oid,
+				   const char *ident, timestamp_t timestamp,
+				   int tz, const char *message, void *cb_data)
+{
+	struct commit *commit;
+	struct check_and_collect_until_cb_data *cb = cb_data;
+
+	/*
+	 * If the reflog entry timestamp is older than the
+	 * remote commit date, there is no need to check or
+	 * collect entries older than this one.
+	 */
+	if (timestamp < cb->remote_commit->date)
+		return -1;
+
+	/* An entry was found. */
+	if (oideq(n_oid, &cb->remote_commit->object.oid))
+		return 1;
+
+	/* Lookup the commit and append it to the list. */
+	if ((commit = lookup_commit_reference(the_repository, n_oid)))
+		add_commit(cb->local_commits, commit);
+
+	return 0;
+}
+
+/*
+ * Iterate through the reflog of a local ref to check if there is an entry for
+ * the given remote-tracking ref (i.e., if it was checked out); runs until the
+ * timestamp of an entry is older than the commit date of the remote-tracking
+ * ref. Any commits that seen along the way are collected into a list to check
+ * if the remote-tracking ref is reachable from any of them.
+ */
+static int is_reachable_in_reflog(const char *local_ref_name,
+				  const struct object_id *remote_oid)
+{
+	struct commit *remote_commit;
+	struct reflog_commit_list list = { NULL, 0, 0 };
+	struct check_and_collect_until_cb_data cb;
+	int ret = 0;
+
+	remote_commit = lookup_commit_reference(the_repository, remote_oid);
+	if (!remote_commit)
+		goto cleanup_return;
+
+	cb.remote_commit = remote_commit;
+	cb.local_commits = &list;
+	ret = for_each_reflog_ent_reverse(local_ref_name,
+					  check_and_collect_until, &cb);
+
+	/* We found an entry in the reflog. */
+	if (ret > 0)
+		goto cleanup_return;
+
+	/*
+	 * Check if "remote_commit" is reachable from
+	 * any of the commits in the collected list.
+	 */
+	if (list.nr > 0)
+		ret = in_merge_bases_many(remote_commit, list.nr, list.items);
+
+cleanup_return:
+	free_reflog_commit_list(&list);
+	return ret;
+}
+
+/*
+ * Check for reachability of a remote-tracking
+ * ref in the reflog entries of its local ref.
+ */
+static void check_if_includes_upstream(struct ref *remote_ref)
+{
+	struct ref *local_ref = get_local_ref(remote_ref->name);
+
+	if (!local_ref)
+		return;
+
+	if (!is_reachable_in_reflog(local_ref->name, &remote_ref->old_oid))
+		remote_ref->unreachable = 1;
+}
+
 static void apply_cas(struct push_cas_option *cas,
 		      struct remote *remote,
 		      struct ref *ref)
@@ -2284,6 +2407,8 @@ static void apply_cas(struct push_cas_option *cas,
 			oidcpy(&ref->old_oid_expect, &entry->expect);
 		else if (remote_tracking(remote, ref->name, &ref->old_oid_expect))
 			oidclr(&ref->old_oid_expect);
+		else
+			ref->check_reachable = cas->use_force_if_includes;
 		return;
 	}
 
@@ -2294,6 +2419,8 @@ static void apply_cas(struct push_cas_option *cas,
 	ref->expect_old_sha1 = 1;
 	if (remote_tracking(remote, ref->name, &ref->old_oid_expect))
 		oidclr(&ref->old_oid_expect);
+	else
+		ref->check_reachable = cas->use_force_if_includes;
 }
 
 void apply_push_cas(struct push_cas_option *cas,
@@ -2301,6 +2428,15 @@ void apply_push_cas(struct push_cas_option *cas,
 		    struct ref *remote_refs)
 {
 	struct ref *ref;
-	for (ref = remote_refs; ref; ref = ref->next)
+	for (ref = remote_refs; ref; ref = ref->next) {
 		apply_cas(cas, remote, ref);
+
+		/*
+		 * If "compare-and-swap" is in "use_tracking[_for_rest]"
+		 * mode, and if "--force-if-includes" was specified, run
+		 * the check.
+		 */
+		if (ref->check_reachable)
+			check_if_includes_upstream(ref);
+	}
 }
diff --git a/remote.h b/remote.h
index 5e3ea5a26d..7c5e59770e 100644
--- a/remote.h
+++ b/remote.h
@@ -104,7 +104,11 @@ struct ref {
 		forced_update:1,
 		expect_old_sha1:1,
 		exact_oid:1,
-		deletion:1;
+		deletion:1,
+		/* Need to check if local reflog reaches the remote tip. */
+		check_reachable:1,
+		/* The local reflog does not reach the remote tip. */
+		unreachable:1;
 
 	enum {
 		REF_NOT_MATCHED = 0, /* initial value */
@@ -134,6 +138,7 @@ struct ref {
 		REF_STATUS_REJECT_NEEDS_FORCE,
 		REF_STATUS_REJECT_STALE,
 		REF_STATUS_REJECT_SHALLOW,
+		REF_STATUS_REJECT_REMOTE_UPDATED,
 		REF_STATUS_UPTODATE,
 		REF_STATUS_REMOTE_REJECT,
 		REF_STATUS_EXPECTING_REPORT,
@@ -332,6 +337,7 @@ struct ref *get_stale_heads(struct refspec *rs, struct ref *fetch_map);
 
 struct push_cas_option {
 	unsigned use_tracking_for_rest:1;
+	unsigned use_force_if_includes:1;
 	struct push_cas {
 		struct object_id expect;
 		unsigned use_tracking:1;
diff --git a/send-pack.c b/send-pack.c
index 632f1580ca..956306e8e8 100644
--- a/send-pack.c
+++ b/send-pack.c
@@ -240,6 +240,7 @@ static int check_to_send_update(const struct ref *ref, const struct send_pack_ar
 	case REF_STATUS_REJECT_FETCH_FIRST:
 	case REF_STATUS_REJECT_NEEDS_FORCE:
 	case REF_STATUS_REJECT_STALE:
+	case REF_STATUS_REJECT_REMOTE_UPDATED:
 	case REF_STATUS_REJECT_NODELETE:
 		return CHECK_REF_STATUS_REJECTED;
 	case REF_STATUS_UPTODATE:
diff --git a/transport-helper.c b/transport-helper.c
index c52c99d829..e547e21199 100644
--- a/transport-helper.c
+++ b/transport-helper.c
@@ -779,6 +779,10 @@ static int push_update_ref_status(struct strbuf *buf,
 			status = REF_STATUS_REJECT_STALE;
 			FREE_AND_NULL(msg);
 		}
+		else if (!strcmp(msg, "remote ref updated since checkout")) {
+			status = REF_STATUS_REJECT_REMOTE_UPDATED;
+			FREE_AND_NULL(msg);
+		}
 		else if (!strcmp(msg, "forced update")) {
 			forced = 1;
 			FREE_AND_NULL(msg);
@@ -897,6 +901,7 @@ static int push_refs_with_push(struct transport *transport,
 		case REF_STATUS_REJECT_NONFASTFORWARD:
 		case REF_STATUS_REJECT_STALE:
 		case REF_STATUS_REJECT_ALREADY_EXISTS:
+		case REF_STATUS_REJECT_REMOTE_UPDATED:
 			if (atomic) {
 				reject_atomic_push(remote_refs, mirror);
 				string_list_clear(&cas_options, 0);
diff --git a/transport.c b/transport.c
index 43e24bf1e5..99fe6233a3 100644
--- a/transport.c
+++ b/transport.c
@@ -567,6 +567,11 @@ static int print_one_push_status(struct ref *ref, const char *dest, int count,
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 				 "stale info", porcelain, summary_width);
 		break;
+	case REF_STATUS_REJECT_REMOTE_UPDATED:
+		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
+				 "remote ref updated since checkout",
+				 porcelain, summary_width);
+		break;
 	case REF_STATUS_REJECT_SHALLOW:
 		print_ref_status('!', "[rejected]", ref, ref->peer_ref,
 				 "new shallow roots not allowed",
@@ -1101,6 +1106,7 @@ static int run_pre_push_hook(struct transport *transport,
 		if (!r->peer_ref) continue;
 		if (r->status == REF_STATUS_REJECT_NONFASTFORWARD) continue;
 		if (r->status == REF_STATUS_REJECT_STALE) continue;
+		if (r->status == REF_STATUS_REJECT_REMOTE_UPDATED) continue;
 		if (r->status == REF_STATUS_UPTODATE) continue;
 
 		strbuf_reset(&buf);
-- 
2.28.0
Previous: Srinidhi KaushikNext: Phillip Wood
Message 67 of 120 in “push: make `--force-with-lease[=<ref>]` safer”
  1. push: make `--force-with-lease[=<ref>]` saferSrinidhi Kaushik, Sep 4, 2020
  2. Phillip WoodSep 7, 2020
  3. Srinidhi KaushikSep 8, 2020
  4. Junio C HamanoSep 7, 2020
  5. Srinidhi KaushikSep 8, 2020
  6. Junio C HamanoSep 8, 2020
  7. Johannes SchindelinSep 7, 2020
  8. Srinidhi KaushikSep 8, 2020
  9. Johannes SchindelinSep 16, 2020
  10. Junio C HamanoSep 8, 2020
  11. Johannes SchindelinSep 9, 2020
  12. Junio C HamanoSep 8, 2020
  13. Johannes SchindelinSep 9, 2020
  14. Johannes SchindelinSep 10, 2020
  15. Junio C HamanoSep 10, 2020
  16. Johannes SchindelinSep 11, 2020
  17. Junio C HamanoSep 14, 2020
  18. Srinidhi KaushikSep 10, 2020
  19. Johannes SchindelinSep 11, 2020
  20. Srinidhi KaushikSep 14, 2020
  21. Junio C HamanoSep 14, 2020
  22. Srinidhi KaushikSep 16, 2020
  23. Johannes SchindelinSep 16, 2020
  24. Junio C HamanoSep 19, 2020
  25. 0/2 push: make "--force-with-lease" saferSrinidhi Kaushik, Sep 12, 2020
  26. 1/2 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Sep 12, 2020
  27. Junio C HamanoSep 12, 2020
  28. Srinidhi KaushikSep 12, 2020
  29. 2/2 push: enable "forceIfIncludesWithLease" by defaultSrinidhi Kaushik, Sep 12, 2020
  30. Junio C HamanoSep 12, 2020
  31. Junio C HamanoSep 12, 2020
  32. Srinidhi KaushikSep 12, 2020
  33. 0/7 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Sep 13, 2020
  34. 1/7 remote: add reflog check for "force-if-includes"Srinidhi Kaushik, Sep 13, 2020
  35. Junio C HamanoSep 14, 2020
  36. Junio C HamanoSep 14, 2020
  37. Srinidhi KaushikSep 16, 2020
  38. Junio C HamanoSep 14, 2020
  39. Johannes SchindelinSep 16, 2020
  40. Srinidhi KaushikSep 19, 2020
  41. 3/7 send-pack: check ref status for "force-if-includes"Srinidhi Kaushik, Sep 13, 2020
  42. 2/7 transport: add flag for "--[no-]force-if-includes"Srinidhi Kaushik, Sep 13, 2020
  43. 4/7 transport-helper: update ref status for "force-if-includes"Srinidhi Kaushik, Sep 13, 2020
  44. 5/7 builtin/push: add option "--[no-]force-if-includes"Srinidhi Kaushik, Sep 13, 2020
  45. Johannes SchindelinSep 16, 2020
  46. 6/7 doc: add reference for "--[no-]force-if-includes"Srinidhi Kaushik, Sep 13, 2020
  47. Junio C HamanoSep 14, 2020
  48. Srinidhi KaushikSep 16, 2020
  49. 7/7 t: add tests for "force-if-includes"Srinidhi Kaushik, Sep 13, 2020
  50. Johannes SchindelinSep 16, 2020
  51. 0/3 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Sep 19, 2020
  52. 1/3 push: add reflog check for "--force-if-includes"Srinidhi Kaushik, Sep 19, 2020
  53. Junio C HamanoSep 19, 2020
  54. Srinidhi KaushikSep 21, 2020
  55. Junio C HamanoSep 21, 2020
  56. Srinidhi KaushikSep 23, 2020
  57. Junio C HamanoSep 23, 2020
  58. Phillip WoodSep 21, 2020
  59. Junio C HamanoSep 21, 2020
  60. Junio C HamanoSep 21, 2020
  61. Srinidhi KaushikSep 23, 2020
  62. 2/3 push: parse and set flag for "--force-if-includes"Srinidhi Kaushik, Sep 19, 2020
  63. Junio C HamanoSep 19, 2020
  64. 3/3 t, doc: update tests, reference for "--force-if-includes"Srinidhi Kaushik, Sep 19, 2020
  65. Junio C HamanoSep 19, 2020
  66. 0/3 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Sep 23, 2020
  67. 1/3 push: add reflog check for "--force-if-includes"Srinidhi Kaushik, Sep 23, 2020
  68. Phillip WoodSep 23, 2020
  69. Srinidhi KaushikSep 23, 2020
  70. Junio C HamanoSep 23, 2020
  71. Junio C HamanoSep 23, 2020
  72. 2/3 push: parse and set flag for "--force-if-includes"Srinidhi Kaushik, Sep 23, 2020
  73. 3/3 t, doc: update tests, reference for "--force-if-includes"Srinidhi Kaushik, Sep 23, 2020
  74. Phillip WoodSep 23, 2020
  75. 0/3 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Sep 26, 2020
  76. 1/3 push: add reflog check for "--force-if-includes"Srinidhi Kaushik, Sep 26, 2020
  77. 3/3 t, doc: update tests, reference for "--force-if-includes"Srinidhi Kaushik, Sep 26, 2020
  78. 2/3 push: parse and set flag for "--force-if-includes"Srinidhi Kaushik, Sep 26, 2020
  79. Srinidhi KaushikSep 26, 2020
  80. 0/3 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Sep 26, 2020
  81. 1/3 push: add reflog check for "--force-if-includes"Srinidhi Kaushik, Sep 26, 2020
  82. Junio C HamanoSep 26, 2020
  83. Srinidhi KaushikSep 27, 2020
  84. 2/3 push: parse and set flag for "--force-if-includes"Srinidhi Kaushik, Sep 26, 2020
  85. 3/3 t, doc: update tests, reference for "--force-if-includes"Srinidhi Kaushik, Sep 26, 2020
  86. 0/3 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Sep 27, 2020
  87. 1/3 push: add reflog check for "--force-if-includes"Srinidhi Kaushik, Sep 27, 2020
  88. 2/3 push: parse and set flag for "--force-if-includes"Srinidhi Kaushik, Sep 27, 2020
  89. 3/3 t, doc: update tests, reference for "--force-if-includes"Srinidhi Kaushik, Sep 27, 2020
  90. Philip OakleySep 30, 2020
  91. Srinidhi KaushikSep 30, 2020
  92. Junio C HamanoSep 28, 2020
  93. SZEDER GáborSep 28, 2020
  94. Srinidhi KaushikSep 28, 2020
  95. Junio C HamanoSep 28, 2020
  96. Junio C HamanoSep 28, 2020
  97. 0/3 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Oct 1, 2020
  98. 1/3 push: add reflog check for "--force-if-includes"Srinidhi Kaushik, Oct 1, 2020
  99. Johannes SchindelinOct 2, 2020
  100. Johannes SchindelinOct 2, 2020
  101. Junio C HamanoOct 2, 2020
  102. Srinidhi KaushikOct 2, 2020
  103. Junio C HamanoOct 2, 2020
  104. Srinidhi KaushikOct 2, 2020
  105. Junio C HamanoOct 2, 2020
  106. Srinidhi KaushikOct 2, 2020
  107. Junio C HamanoOct 2, 2020
  108. Junio C HamanoOct 2, 2020
  109. 2/3 push: parse and set flag for "--force-if-includes"Srinidhi Kaushik, Oct 1, 2020
  110. 3/3 t, doc: update tests, reference for "--force-if-includes"Srinidhi Kaushik, Oct 1, 2020
  111. Junio C HamanoOct 1, 2020
  112. Junio C HamanoOct 1, 2020
  113. Srinidhi KaushikOct 1, 2020
  114. Junio C HamanoOct 1, 2020
  115. Junio C HamanoOct 2, 2020
  116. Srinidhi KaushikOct 2, 2020
  117. 0/3 push: add "--[no-]force-if-includes"Srinidhi Kaushik, Oct 3, 2020
  118. 2/3 push: parse and set flag for "--force-if-includes"Srinidhi Kaushik, Oct 3, 2020
  119. 1/3 push: add reflog check for "--force-if-includes"Srinidhi Kaushik, Oct 3, 2020
  120. 3/3 t, doc: update tests, reference for "--force-if-includes"Srinidhi Kaushik, Oct 3, 2020

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.