All of lore.kernel.org
 help / color / mirror / 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

* 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 \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.