From mboxrd@z Thu Jan 1 00:00:00 1970 Return-Path: X-Spam-Checker-Version: SpamAssassin 3.4.2 (2018-09-13) on dcvr.yhbt.net X-Spam-Level: X-Spam-Status: No, score=-3.2 required=3.0 tests=AWL,BAYES_00,DKIM_SIGNED, DKIM_VALID,DKIM_VALID_AU,FREEMAIL_FORGED_FROMDOMAIN,FREEMAIL_FROM, HEADER_FROM_DIFFERENT_DOMAINS,MAILING_LIST_MULTI,SPF_HELO_PASS, SPF_PASS shortcircuit=no autolearn=ham autolearn_force=no version=3.4.2 Received: from vger.kernel.org (vger.kernel.org [23.128.96.18]) by dcvr.yhbt.net (Postfix) with ESMTP id 236641F4B4 for ; Sat, 12 Sep 2020 15:05:49 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1725875AbgILPFe (ORCPT ); Sat, 12 Sep 2020 11:05:34 -0400 Received: from lindbergh.monkeyblade.net ([23.128.96.19]:54784 "EHLO lindbergh.monkeyblade.net" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1725863AbgILPFR (ORCPT ); Sat, 12 Sep 2020 11:05:17 -0400 Received: from mail-pl1-x642.google.com (mail-pl1-x642.google.com [IPv6:2607:f8b0:4864:20::642]) by lindbergh.monkeyblade.net (Postfix) with ESMTPS id 23B60C061573 for ; Sat, 12 Sep 2020 08:05:17 -0700 (PDT) Received: by mail-pl1-x642.google.com with SMTP id d16so2317990pll.13 for ; Sat, 12 Sep 2020 08:05:17 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=gmail.com; s=20161025; h=from:to:cc:subject:date:message-id:in-reply-to:references :mime-version:content-transfer-encoding; bh=OUeg1X2gxFoptvFCVSDeHXSVkSz+4Fi/+0iaXgrfBc8=; b=CA3PbqQrWUcQO4itd60FruiGGaia/JbGRnO2dYoYNFE2NfrCeX47/JXBBD4kzon82V RHfv+7MmAIzp2EIjoH05m7zOSrZsTBMkJYSG0wm7VKBpRk1E7VBADzHnCAUNK2dueCIF pTO9EA18v/WNKs9MCGmBj7RXbeK3VlNFCc09VZNRDaxHR/zCEfPm9zLFy+mJKOlxXoqA ohNXvdeY0SDR6e+MMtnz5JrNGYIZAr/hz42iqN48lEK0nbhRF+NNNLvuKyk/F3YoVv1Y H6M5tcD5x2e1Ol/8NJaP+QsNfBcxaLeV8Fe5avk2qjNl163p87SpjwFX8ky1VLy/BDjU /dvw== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:from:to:cc:subject:date:message-id:in-reply-to :references:mime-version:content-transfer-encoding; bh=OUeg1X2gxFoptvFCVSDeHXSVkSz+4Fi/+0iaXgrfBc8=; b=nWsByRlnNRgDl+QCfC8NST87mooNWxoSI6gOni0WSPz97Vl1aJFJSHP8gwHD0hmnWo ClN2sD/k6U07ruvC2gOZY8IhMEhe8HN0QNjlMs/85mdoqzY8KP+DYdslBLTKPGVb15JR 37ANdcSgftktEK6CZcrVxQNhKfYc1YIpeueO3ANe8tL7pebwen+HFfD0TeUxOY6lUdHW 5iak3uCBlnSdwrNplx9vZge4ziNJzmggZAyDr27NpcbOTYxt7N367Y+ifofbcjYkGRi4 kmaRzn2K0g+cjgMtxJQh3+AqEFVFTEZqWNeYWiam2FHtvGYGSglXoGJWdL8YCEMORErQ eBiQ== X-Gm-Message-State: AOAM5311f1cvUmiF7ETco7oGTpV6DvpuGWu3RMjM5z02wOn+AmFRVX4l XkXs/QXkMpkZ6qLh62gAbonFSJNKZwNtYw== X-Google-Smtp-Source: ABdhPJyfpMqsQdmoxVm3WHVtLC+un5qffs1RSPI5RcGdgnlXsdK5KjHkUzAhNzMsaVUmjim6ECaPhA== X-Received: by 2002:a17:902:9b8e:b029:d0:89f3:28c8 with SMTP id y14-20020a1709029b8eb02900d089f328c8mr7000337plp.4.1599923115647; Sat, 12 Sep 2020 08:05:15 -0700 (PDT) Received: from localhost.localdomain ([124.123.104.38]) by smtp.gmail.com with ESMTPSA id q24sm5479196pfs.206.2020.09.12.08.05.13 (version=TLS1_2 cipher=ECDHE-ECDSA-AES128-GCM-SHA256 bits=128/128); Sat, 12 Sep 2020 08:05:14 -0700 (PDT) From: Srinidhi Kaushik To: git@vger.kernel.org Cc: Srinidhi Kaushik Subject: [PATCH v2 1/2] push: add "--[no-]force-if-includes" Date: Sat, 12 Sep 2020 20:34:58 +0530 Message-Id: <20200912150459.8282-2-shrinidhi.kaushik@gmail.com> X-Mailer: git-send-email 2.28.0 In-Reply-To: <20200912150459.8282-1-shrinidhi.kaushik@gmail.com> References: <20200904185147.77439-1-shrinidhi.kaushik@gmail.com> <20200912150459.8282-1-shrinidhi.kaushik@gmail.com> MIME-Version: 1.0 Content-Transfer-Encoding: 8bit Sender: git-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: git@vger.kernel.org Add a new option: `--force-if-includes` to `git-push` where forced updates are allowed only if the tip of the remote-tracking ref has been integrated locally, by verifying if the tip of the remote-tracking ref on which a local branch has based on (for a rewrite), is reachable from at least one of the `reflog` entries of the local branch about to be updated by force on the remote. This option can also be used with `--force-with-lease` in setups where the remote-tracking refs of the repository are implicitly updated in the background. If a local branch is based on a remote ref for a rewrite, and if that remote-tracking ref is updated by a push from another repository after it has been checked out locally, force updating that branch to remote with `--force-with-lease[=[:expect]]` without specifying the "" or "" values, can cause the update that happened in-between the checkout and forced push to be lost. Specifying `--force-with-includes` with `--force-with-lease` as an ancillary argument at the time of push, ensures that any new updates to the remote-tracking refs are integrated locally before allowing a forced update. This behavior can enabled by default if the configuration option `push.forceIfIncludesWithLease` is set to `true`. Signed-off-by: Srinidhi Kaushik --- Documentation/config/advice.txt | 4 + Documentation/config/push.txt | 8 ++ Documentation/git-push.txt | 22 +++++ advice.c | 3 + advice.h | 2 + builtin/push.c | 27 +++++- builtin/send-pack.c | 13 ++- remote.c | 129 ++++++++++++++++++++++++--- remote.h | 14 ++- send-pack.c | 1 + t/t5533-push-cas.sh | 53 ++++++++++++ t/t5549-push-force-if-includes.sh | 139 ++++++++++++++++++++++++++++++ transport-helper.c | 5 ++ transport.c | 24 +++++- transport.h | 12 +-- 15 files changed, 434 insertions(+), 22 deletions(-) create mode 100755 t/t5549-push-force-if-includes.sh diff --git a/Documentation/config/advice.txt b/Documentation/config/advice.txt index bdd37c3eaa..f48aed49e8 100644 --- a/Documentation/config/advice.txt +++ b/Documentation/config/advice.txt @@ -41,6 +41,10 @@ advice.*:: we can still suggest that the user push to either refs/heads/* or refs/tags/* based on the type of the source object. + pushRefNeedsUpdate:: + Shown when linkgit:git-push[1] rejects a forced update of + a branch when its remote-tracking ref has updates that we + do not have locally. statusAheadBehind:: Shown when linkgit:git-status[1] computes the ahead/behind counts for a local ref compared to its remote tracking ref, diff --git a/Documentation/config/push.txt b/Documentation/config/push.txt index f5e5b38c68..1b4948faa0 100644 --- a/Documentation/config/push.txt +++ b/Documentation/config/push.txt @@ -114,3 +114,11 @@ push.recurseSubmodules:: specifying '--recurse-submodules=check|on-demand|no'. If not set, 'no' is used by default, unless 'submodule.recurse' is set (in which case a 'true' value means 'on-demand'). + +push.forceIfIncludesWithLease:: + If set to `true`, adds `--force-if-includes` as an ancillary argument + to `--force-with-lease[=[:]]`, when "" or + "" values are unspecified at the time of push. ++ +Note: Specifying `--no-force-if-includes` to linkgit:git-push[1] as an +argument during the time of push does _not_ override this configuration. diff --git a/Documentation/git-push.txt b/Documentation/git-push.txt index 3b8053447e..199c601bd4 100644 --- a/Documentation/git-push.txt +++ b/Documentation/git-push.txt @@ -320,6 +320,15 @@ seen and are willing to overwrite, then rewrite history, and finally force push changes to `master` if the remote version is still at `base`, regardless of what your local `remotes/origin/master` has been updated to in the background. ++ +Alternatively, specifying `--force-if-includes` an an ancillary option along +with `--force-with-lease[=[:expect]]` (when "" or "" +values are unspecified) at the time of `push` will verify if updates from +the remote-tracking refs that may have been implicitly updated in the +background (via linkgit:git-fetch[1], and the like) are integrated locally +before allowing a forced update. This behavior can be enabled by default if +the configuration option `push.forceIfIncludesWithLease` to `true` +in linkgit:git-config[1]. -f:: --force:: @@ -341,6 +350,19 @@ one branch, use a `+` in front of the refspec to push (e.g `git push origin +master` to force a push to the `master` branch). See the `...` section above for details. +--[no-]force-if-includes:: + Force an update only if the tip of the remote-tracking ref + has been integrated locally. ++ +This option verifies if the tip of the remote-tracking ref on which +a local branch has based on (for a rewrite), is reachable from at +least one of the `reflog` entries of the local branch about to be +updated by force on the remote. The check ensures that any updates +from the remote have been incorporated locally by rejecting a push +if that is not the case. ++ +Specifying `--no-force-if-includes` disables this behavior. + --repo=:: This option is equivalent to the argument. If both are specified, the command-line argument takes precedence. diff --git a/advice.c b/advice.c index f0a3d32d20..164742305f 100644 --- a/advice.c +++ b/advice.c @@ -11,6 +11,7 @@ int advice_push_already_exists = 1; int advice_push_fetch_first = 1; int advice_push_needs_force = 1; int advice_push_unqualified_ref_name = 1; +int advice_push_ref_needs_update = 1; int advice_status_hints = 1; int advice_status_u_option = 1; int advice_status_ahead_behind_warning = 1; @@ -72,6 +73,7 @@ static struct { { "pushFetchFirst", &advice_push_fetch_first }, { "pushNeedsForce", &advice_push_needs_force }, { "pushUnqualifiedRefName", &advice_push_unqualified_ref_name }, + { "pushRefNeedsUpdate", &advice_push_ref_needs_update }, { "statusHints", &advice_status_hints }, { "statusUoption", &advice_status_u_option }, { "statusAheadBehindWarning", &advice_status_ahead_behind_warning }, @@ -116,6 +118,7 @@ static struct { [ADVICE_PUSH_ALREADY_EXISTS] = { "pushAlreadyExists", 1 }, [ADVICE_PUSH_FETCH_FIRST] = { "pushFetchFirst", 1 }, [ADVICE_PUSH_NEEDS_FORCE] = { "pushNeedsForce", 1 }, + [ADVICE_PUSH_REF_NEEDS_UPDATE] = { "pushRefNeedsUpdate", 1 }, /* make this an alias for backward compatibility */ [ADVICE_PUSH_UPDATE_REJECTED_ALIAS] = { "pushNonFastForward", 1 }, diff --git a/advice.h b/advice.h index 16f2c11642..bc2432980a 100644 --- a/advice.h +++ b/advice.h @@ -11,6 +11,7 @@ extern int advice_push_already_exists; extern int advice_push_fetch_first; extern int advice_push_needs_force; extern int advice_push_unqualified_ref_name; +extern int advice_push_ref_needs_update; extern int advice_status_hints; extern int advice_status_u_option; extern int advice_status_ahead_behind_warning; @@ -60,6 +61,7 @@ extern int advice_add_empty_pathspec; ADVICE_PUSH_UNQUALIFIED_REF_NAME, ADVICE_PUSH_UPDATE_REJECTED_ALIAS, ADVICE_PUSH_UPDATE_REJECTED, + ADVICE_PUSH_REF_NEEDS_UPDATE, ADVICE_RESET_QUIET_WARNING, ADVICE_RESOLVE_CONFLICT, ADVICE_RM_HINTS, diff --git a/builtin/push.c b/builtin/push.c index bc94078e72..7fb07eb38e 100644 --- a/builtin/push.c +++ b/builtin/push.c @@ -300,6 +300,12 @@ static const char message_advice_ref_needs_force[] = "or update a remote ref to make it point at a non-commit object,\n" "without using the '--force' option.\n"); +static const char message_advice_ref_needs_update[] = + N_("Updates were rejected because the tip of the remote-tracking\n" + "branch has been updated since the last checkout. You may want\n" + "to integrate those changes locally (e.g., 'git rebase ...')\n" + "before forcing an update.\n"); + static void advise_pull_before_push(void) { if (!advice_push_non_ff_current || !advice_push_update_rejected) @@ -335,6 +341,13 @@ static void advise_ref_needs_force(void) advise(_(message_advice_ref_needs_force)); } +static void advise_ref_needs_update(void) +{ + if (!advice_push_ref_needs_update || !advice_push_update_rejected) + return; + advise(_(message_advice_ref_needs_update)); +} + static int push_with_options(struct transport *transport, struct refspec *rs, int flags) { @@ -384,8 +397,9 @@ static int push_with_options(struct transport *transport, struct refspec *rs, advise_ref_fetch_first(); } else if (reject_reasons & REJECT_NEEDS_FORCE) { advise_ref_needs_force(); + } else if (reject_reasons & REJECT_REF_NEEDS_UPDATE) { + advise_ref_needs_update(); } - return 1; } @@ -520,8 +534,14 @@ static int git_push_config(const char *k, const char *v, void *cb) if (!v) return config_error_nonbool(k); return color_parse(v, push_colors[slot]); - } + } else if (!strcmp(k, "push.forceifincludeswithlease")) { + if (git_config_bool(k, v)) + *flags |= TRANSPORT_PUSH_FORCE_IF_INCLUDES; + else + *flags &= ~TRANSPORT_PUSH_FORCE_IF_INCLUDES; + return 0; + } return git_default_config(k, v, NULL); } @@ -551,6 +571,9 @@ int cmd_push(int argc, const char **argv, const char *prefix) OPT_CALLBACK_F(0, CAS_OPT_NAME, &cas, N_(":"), N_("require old value of ref to be at this value"), PARSE_OPT_OPTARG | PARSE_OPT_LITERAL_ARGHELP, parseopt_push_cas_option), + OPT_BIT(0, "force-if-includes", &flags, + N_("require remote updates to be integrated locally"), + TRANSPORT_PUSH_FORCE_IF_INCLUDES), OPT_CALLBACK(0, "recurse-submodules", &recurse_submodules, "(check|on-demand|no)", N_("control recursive pushing of submodules"), option_parse_recurse_submodules), OPT_BOOL_F( 0 , "thin", &thin, N_("use thin pack"), PARSE_OPT_NOCOMPLETE), diff --git a/builtin/send-pack.c b/builtin/send-pack.c index 2b9610f121..ee5d7af00c 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 updated since checkout"; + break; + case REF_STATUS_REJECT_ALREADY_EXISTS: res = "error"; msg = "already exists"; @@ -155,6 +160,7 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) int from_stdin = 0; struct push_cas_option cas = {0}; struct packet_reader reader; + unsigned int force_if_inc; struct option options[] = { OPT__VERBOSITY(&verbose), @@ -179,6 +185,8 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) OPT_CALLBACK_F(0, CAS_OPT_NAME, &cas, N_(":"), N_("require old value of ref to be at this value"), PARSE_OPT_OPTARG, parseopt_push_cas_option), + OPT_BOOL(0, "force-if-includes", &force_if_inc, + N_("require remote updates to be integrated locally")), OPT_END() }; @@ -278,7 +286,10 @@ int cmd_send_pack(int argc, const char **argv, const char *prefix) return -1; if (!is_empty_cas(&cas)) - apply_push_cas(&cas, remote, remote_refs); + apply_push_cas(&cas, remote, remote_refs, force_if_inc); + + if (is_empty_cas(&cas) && force_if_inc) + run_local_reflog_check(remote_refs); set_ref_status_for_push(remote_refs, args.send_mirror, args.force_update); diff --git a/remote.c b/remote.c index 420150837b..71af6d3073 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,76 @@ 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 its local + * ref's reflog entries. + */ +void check_reflog_for_ref(struct ref *r_ref) +{ + struct ref *l_ref = get_local_ref(r_ref->name); + struct object_id r_oid; + + r_ref->if_includes = 1; + if (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) + struct ref *ref, + int if_includes) { - int i; + int i, check_reflog = 0; /* Find an explicit --