git@vger.kernel.org list mirror (unofficial, one of many)
 help / color / mirror / code / Atom feed
From: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
To: git@vger.kernel.org
Cc: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
Subject: [PATCH v3 1/7] remote: add reflog check for "force-if-includes"
Date: Sun, 13 Sep 2020 20:24:07 +0530	[thread overview]
Message-ID: <20200913145413.18351-2-shrinidhi.kaushik@gmail.com> (raw)
In-Reply-To: <20200913145413.18351-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; `set_ref_status_for_push`
updated to add a reject reason and disallow the forced push if the
check fails.

When a local branch that is based on a remote ref, has been rewound and
is to be force pushed on the remote, "apply_push_force_if_includes()"
runs a check that ensure 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 by rejecting the
forced update.

The struct "ref" has three new bit-fields:
  * if_includes: Set when we have to run the new check on the ref.
  * is_tracking: Set when the remote ref was marked as "use_tracking"
                 or "use_tracking_for_rest" by compare-and-swap.
  * unreachable: Set if the ref is unreachable from any of the "reflog"
                 entries of its local counterpart.

When "--force-with-includes" is used along with "--force-with-lease",
the check is run only for refs marked as "is_tracking".

The enum "status" updated to include "REF_STATUS_REJECT_REMOTE_UPDATED"
to imply that the ref failed the check.

Signed-off-by: Srinidhi Kaushik <shrinidhi.kaushik@gmail.com>
---
 remote.c | 135 ++++++++++++++++++++++++++++++++++++++++++++++++++++---
 remote.h |  14 +++++-
 2 files changed, 141 insertions(+), 8 deletions(-)

diff --git a/remote.c b/remote.c
index 420150837b..e4b2d85a6f 100644
--- a/remote.c
+++ b/remote.c
@@ -1484,6 +1484,36 @@ void set_ref_status_for_push(struct ref *remote_refs, int send_mirror,
 				force_ref_update = 1;
 		}
 
+		/*
+		 * 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.
+		 *
+		 * There is no need to check for reachability, if the
+		 * ref is marked for deletion.
+		 */
+		if (ref->if_includes && !ref->deletion) {
+			/*
+			 * If `force_ref_update' was previously set by
+			 * "compare-and-swap", and we have to run this
+			 * check, reset it back to the original value
+			 * and update it depending on the status of this
+			 * check.
+			 */
+			force_ref_update = ref->force || force_update;
+
+			if (ref->unreachable)
+				reject_reason =
+					REF_STATUS_REJECT_REMOTE_UPDATED;
+			else
+				/*
+				 * If updates from the remote-tracking ref
+				 * have been integrated locally; force the
+				 * update.
+				 */
+				force_ref_update = 1;
+		}
+
 		/*
 		 * If the update isn't already rejected then check
 		 * the usual "must fast-forward" rules.
@@ -2272,11 +2302,74 @@ static int remote_tracking(struct remote *remote, const char *refname,
 	return 0;
 }
 
+static int ref_reachable(struct object_id *o_oid, struct object_id *n_oid,
+			 const char *ident, timestamp_t timestamp, int tz,
+			 const char *message, void *cb_data)
+{
+	int ret = 0;
+	struct object_id *r_oid = cb_data;
+
+	ret = oideq(n_oid, r_oid);
+	if (!ret) {
+		struct commit *loc = lookup_commit_reference(the_repository,
+							     n_oid);
+		struct commit *rem = lookup_commit_reference(the_repository,
+							     r_oid);
+		ret = (loc && rem) ? in_merge_bases(rem, loc) : 0;
+	}
+
+	return ret;
+}
+
+/*
+ * Iterate through the reflog of a local branch and check
+ * if the tip of the remote-tracking branch is reachable
+ * from one of the entries.
+ */
+static int ref_reachable_from_reflog(const struct object_id *r_oid,
+				     const struct object_id *l_oid,
+				     const char *local_ref_name)
+{
+	int ret = 0;
+	struct commit *r_commit, *l_commit;
+
+	l_commit = lookup_commit_reference(the_repository, l_oid);
+	r_commit = lookup_commit_reference(the_repository, r_oid);
+
+	/*
+	 * If the remote-tracking ref is an ancestor of the local
+	 * ref (a merge, for instance) there is no need to iterate
+	 * through the reflog entries to ensure reachability; it
+	 * can be skipped to return early instead.
+	 */
+	ret = (r_commit && l_commit) ? in_merge_bases(r_commit, l_commit) : 0;
+	if (!ret)
+		ret = for_each_reflog_ent_reverse(local_ref_name, ref_reachable,
+						  (struct object_id *)r_oid);
+
+	return ret;
+}
+
+/*
+ * Check for reachability of a remote-tracking
+ * ref in the reflog entries of its local ref.
+ */
+void check_reflog_for_ref(struct ref *r_ref)
+{
+	struct object_id r_oid;
+	struct ref *l_ref = get_local_ref(r_ref->name);
+
+	if (r_ref->if_includes && l_ref && !read_ref(l_ref->name, &r_oid))
+		r_ref->unreachable = !ref_reachable_from_reflog(&r_ref->old_oid,
+								&r_oid,
+								l_ref->name);
+}
+
 static void apply_cas(struct push_cas_option *cas,
 		      struct remote *remote,
 		      struct ref *ref)
 {
-	int i;
+	int i, is_tracking = 0;
 
 	/* Find an explicit --<option>=<name>[:<value>] entry */
 	for (i = 0; i < cas->nr; i++) {
@@ -2288,16 +2381,26 @@ 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);
-		return;
+		else
+			is_tracking = 1;
+		break;
 	}
 
 	/* Are we using "--<option>" to cover all? */
-	if (!cas->use_tracking_for_rest)
-		return;
+	if (cas->use_tracking_for_rest) {
+		ref->expect_old_sha1 = 1;
+		if (remote_tracking(remote, ref->name, &ref->old_oid_expect))
+			oidclr(&ref->old_oid_expect);
+		else
+			is_tracking = 1;
+	}
+
+	/*
+	 * Mark this ref to be checked if "--force-if-includes" is
+	 * specified as an argument along with "compare-and-swap".
+	 */
+	ref->is_tracking = is_tracking;
 
-	ref->expect_old_sha1 = 1;
-	if (remote_tracking(remote, ref->name, &ref->old_oid_expect))
-		oidclr(&ref->old_oid_expect);
 }
 
 void apply_push_cas(struct push_cas_option *cas,
@@ -2308,3 +2411,21 @@ void apply_push_cas(struct push_cas_option *cas,
 	for (ref = remote_refs; ref; ref = ref->next)
 		apply_cas(cas, remote, ref);
 }
+
+void apply_push_force_if_includes(struct ref *remote_refs, int used_with_cas)
+{
+	struct ref *ref;
+	for (ref = remote_refs; ref; ref = ref->next) {
+		/*
+		 * If "compare-and-swap" is used along with option, run the
+		 * check on refs that have been marked to do so. Otherwise,
+		 * all refs will be checked.
+		 */
+		if (used_with_cas)
+			ref->if_includes = ref->is_tracking;
+		else
+			ref->if_includes = 1;
+
+		check_reflog_for_ref(ref);
+	}
+}
diff --git a/remote.h b/remote.h
index 5e3ea5a26d..1618ba892b 100644
--- a/remote.h
+++ b/remote.h
@@ -104,7 +104,10 @@ struct ref {
 		forced_update:1,
 		expect_old_sha1:1,
 		exact_oid:1,
-		deletion:1;
+		deletion:1,
+		if_includes:1, /* If "--force-with-includes" was specified.  */
+		is_tracking:1, /* If "use_tracking[_for_rest]" is set (CAS). */
+		unreachable:1; /* For "if_includes"; unreachable in reflog.  */
 
 	enum {
 		REF_NOT_MATCHED = 0, /* initial value */
@@ -134,6 +137,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,
@@ -346,4 +350,12 @@ int parseopt_push_cas_option(const struct option *, const char *arg, int unset);
 int is_empty_cas(const struct push_cas_option *);
 void apply_push_cas(struct push_cas_option *, struct remote *, struct ref *);
 
+/*
+ * Runs when "--force-if-includes" is specified.
+ * Checks if the remote-tracking ref was updated (since checkout)
+ * implicitly in the background and verify that changes from the
+ * updated tip have been integrated locally, before pushing.
+ */
+void apply_push_force_if_includes(struct ref*, int);
+
 #endif
-- 
2.28.0


  reply	other threads:[~2020-09-13 14:54 UTC|newest]

Thread overview: 120+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2020-09-04 18:51 [PATCH] push: make `--force-with-lease[=<ref>]` safer Srinidhi Kaushik
2020-09-07 15:23 ` Phillip Wood
2020-09-08 15:48   ` Srinidhi Kaushik
2020-09-07 16:14 ` Junio C Hamano
2020-09-08 16:00   ` Srinidhi Kaushik
2020-09-08 21:00     ` Junio C Hamano
2020-09-07 19:45 ` Johannes Schindelin
2020-09-08 15:58   ` Junio C Hamano
2020-09-09  3:40     ` Johannes Schindelin
2020-09-08 16:59   ` Srinidhi Kaushik
2020-09-16 11:55     ` Johannes Schindelin
2020-09-08 19:34   ` Junio C Hamano
2020-09-09  3:44     ` Johannes Schindelin
2020-09-10 10:22       ` Johannes Schindelin
2020-09-10 14:44         ` Srinidhi Kaushik
2020-09-11 22:16           ` Johannes Schindelin
2020-09-14 11:06             ` Srinidhi Kaushik
2020-09-14 20:08             ` Junio C Hamano
2020-09-16  5:31               ` Srinidhi Kaushik
2020-09-16 10:20                 ` Johannes Schindelin
2020-09-19 17:48                   ` Junio C Hamano
2020-09-10 14:46         ` Junio C Hamano
2020-09-11 22:17           ` Johannes Schindelin
2020-09-14 20:07             ` Junio C Hamano
2020-09-12 15:04 ` [PATCH v2 0/2] push: make "--force-with-lease" safer Srinidhi Kaushik
2020-09-12 15:04   ` [PATCH v2 1/2] push: add "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-12 18:20     ` Junio C Hamano
2020-09-12 21:25       ` Srinidhi Kaushik
2020-09-12 15:04   ` [PATCH v2 2/2] push: enable "forceIfIncludesWithLease" by default Srinidhi Kaushik
2020-09-12 18:22     ` Junio C Hamano
2020-09-12 18:15   ` [PATCH v2 0/2] push: make "--force-with-lease" safer Junio C Hamano
2020-09-12 21:03     ` Srinidhi Kaushik
2020-09-13 14:54   ` [PATCH v3 0/7] push: add "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-13 14:54     ` Srinidhi Kaushik [this message]
2020-09-14 20:17       ` [PATCH v3 1/7] remote: add reflog check for "force-if-includes" Junio C Hamano
2020-09-16 10:51         ` Srinidhi Kaushik
2020-09-14 20:31       ` Junio C Hamano
2020-09-14 21:13       ` Junio C Hamano
2020-09-16 12:35       ` Johannes Schindelin
2020-09-19 17:01         ` Srinidhi Kaushik
2020-09-13 14:54     ` [PATCH v3 2/7] transport: add flag for "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-13 14:54     ` [PATCH v3 3/7] send-pack: check ref status for "force-if-includes" Srinidhi Kaushik
2020-09-13 14:54     ` [PATCH v3 4/7] transport-helper: update " Srinidhi Kaushik
2020-09-13 14:54     ` [PATCH v3 5/7] builtin/push: add option "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-16 12:36       ` Johannes Schindelin
2020-09-13 14:54     ` [PATCH v3 6/7] doc: add reference for "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-14 21:01       ` Junio C Hamano
2020-09-16  5:35         ` Srinidhi Kaushik
2020-09-13 14:54     ` [PATCH v3 7/7] t: add tests for "force-if-includes" Srinidhi Kaushik
2020-09-16 12:47     ` [PATCH v3 0/7] push: add "--[no-]force-if-includes" Johannes Schindelin
2020-09-19 17:03   ` [PATCH v4 0/3] " Srinidhi Kaushik
2020-09-19 17:03     ` [PATCH v4 1/3] push: add reflog check for "--force-if-includes" Srinidhi Kaushik
2020-09-19 20:03       ` Junio C Hamano
2020-09-21  8:42         ` Srinidhi Kaushik
2020-09-21 18:48           ` Junio C Hamano
2020-09-23 10:22             ` Srinidhi Kaushik
2020-09-23 16:47               ` Junio C Hamano
2020-09-21 13:19         ` Phillip Wood
2020-09-21 16:12           ` Junio C Hamano
2020-09-21 18:11             ` Junio C Hamano
2020-09-23 10:27           ` Srinidhi Kaushik
2020-09-19 17:03     ` [PATCH v4 2/3] push: parse and set flag " Srinidhi Kaushik
2020-09-19 20:26       ` Junio C Hamano
2020-09-19 17:03     ` [PATCH v4 3/3] t, doc: update tests, reference " Srinidhi Kaushik
2020-09-19 20:42       ` Junio C Hamano
2020-09-23  7:30     ` [PATCH v5 0/3] push: add "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-23  7:30       ` [PATCH v5 1/3] push: add reflog check for "--force-if-includes" Srinidhi Kaushik
2020-09-23 10:18         ` Phillip Wood
2020-09-23 11:26           ` Srinidhi Kaushik
2020-09-23 16:24           ` Junio C Hamano
2020-09-23 16:29         ` Junio C Hamano
2020-09-23  7:30       ` [PATCH v5 2/3] push: parse and set flag " Srinidhi Kaushik
2020-09-23  7:30       ` [PATCH v5 3/3] t, doc: update tests, reference " Srinidhi Kaushik
2020-09-23 10:24         ` Phillip Wood
2020-09-26 10:13       ` [PATCH v6 0/3] push: add "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-26 10:13         ` [PATCH v6 1/3] push: add reflog check for "--force-if-includes" Srinidhi Kaushik
2020-09-26 10:13         ` [PATCH v6 2/3] push: parse and set flag " Srinidhi Kaushik
2020-09-26 10:13         ` [PATCH v6 3/3] t, doc: update tests, reference " Srinidhi Kaushik
2020-09-26 10:21         ` [PATCH v6 0/3] push: add "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-26 11:46         ` [PATCH v7 " Srinidhi Kaushik
2020-09-26 11:46           ` [PATCH v7 1/3] push: add reflog check for "--force-if-includes" Srinidhi Kaushik
2020-09-26 23:42             ` Junio C Hamano
2020-09-27 12:27               ` Srinidhi Kaushik
2020-09-26 11:46           ` [PATCH v7 2/3] push: parse and set flag " Srinidhi Kaushik
2020-09-26 11:46           ` [PATCH v7 3/3] t, doc: update tests, reference " Srinidhi Kaushik
2020-09-27 14:17           ` [PATCH v8 0/3] push: add "--[no-]force-if-includes" Srinidhi Kaushik
2020-09-27 14:17             ` [PATCH v8 1/3] push: add reflog check for "--force-if-includes" Srinidhi Kaushik
2020-09-27 14:17             ` [PATCH v8 2/3] push: parse and set flag " Srinidhi Kaushik
2020-09-27 14:17             ` [PATCH v8 3/3] t, doc: update tests, reference " Srinidhi Kaushik
2020-09-30 12:54               ` Philip Oakley
2020-09-30 14:27                 ` Srinidhi Kaushik
2020-09-28 17:31             ` [PATCH v8 0/3] push: add "--[no-]force-if-includes" Junio C Hamano
2020-09-28 17:46               ` SZEDER Gábor
2020-09-28 19:34                 ` Srinidhi Kaushik
2020-09-28 19:51                   ` Junio C Hamano
2020-09-28 20:00                 ` Junio C Hamano
2020-10-01  8:21             ` [PATCH v9 " Srinidhi Kaushik
2020-10-01  8:21               ` [PATCH v9 1/3] push: add reflog check for "--force-if-includes" Srinidhi Kaushik
2020-10-02 13:52                 ` Johannes Schindelin
2020-10-02 14:50                   ` Johannes Schindelin
2020-10-02 16:22                     ` Junio C Hamano
2020-10-02 15:07                   ` Srinidhi Kaushik
2020-10-02 16:41                     ` Junio C Hamano
2020-10-02 19:39                       ` Srinidhi Kaushik
2020-10-02 20:14                         ` Junio C Hamano
2020-10-02 20:58                           ` Srinidhi Kaushik
2020-10-02 21:36                             ` Junio C Hamano
2020-10-02 16:26                   ` Junio C Hamano
2020-10-01  8:21               ` [PATCH v9 2/3] push: parse and set flag " Srinidhi Kaushik
2020-10-01  8:21               ` [PATCH v9 3/3] t, doc: update tests, reference " Srinidhi Kaushik
2020-10-01 15:46               ` [PATCH v9 0/3] push: add "--[no-]force-if-includes" Junio C Hamano
2020-10-01 17:12                 ` Junio C Hamano
2020-10-01 17:54                   ` Srinidhi Kaushik
2020-10-01 18:32                     ` Junio C Hamano
2020-10-02 16:50                     ` Junio C Hamano
2020-10-02 19:42                       ` Srinidhi Kaushik
2020-10-03 12:10               ` [PATCH v10 " Srinidhi Kaushik
2020-10-03 12:10                 ` [PATCH v10 1/3] push: add reflog check for "--force-if-includes" Srinidhi Kaushik
2020-10-03 12:10                 ` [PATCH v10 2/3] push: parse and set flag " Srinidhi Kaushik
2020-10-03 12:10                 ` [PATCH v10 3/3] t, doc: update tests, reference " Srinidhi Kaushik

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: http://vger.kernel.org/majordomo-info.html

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20200913145413.18351-2-shrinidhi.kaushik@gmail.com \
    --to=shrinidhi.kaushik@gmail.com \
    --cc=git@vger.kernel.org \
    --subject='Re: [PATCH v3 1/7] remote: add reflog check for "force-if-includes"' \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link

Code repositories for project(s) associated with this inbox:

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).