Re: [PATCH RFC] rebase: respect --ff-only option

From: Felipe Contreras <felipe.contreras@gmail.com>
Date: Jul 7, 2021, 01:13 UTC
Message-ID: <60e4ffd4d2a93_1c5ca52081a@natae.notmuch>
In-Reply-To: <xmqqpmvwn1qp.fsf@gitster.g>

Junio C Hamano wrote:

Show 11 quoted lines

> Phillip Wood <phillip.wood123@gmail.com> writes:
> 
> > Looking at origin/seen:builtin/pull.c we already check if we can
> > fast-forward and unconditionally merge in that case irrespective of
> > any '--rebase' option or pull.rebase config. It should be simple for
> > pull to error out if '--ff-only' is given and we cannot fast-forward.
> 
> Excellent.
> 
> Even though teaching even more special case on the "git pull" side
> makes me feel somewhat dirty,

It makes you feeld dirty for a reason.

Why don't you just actually try the proposal?

--- a/builtin/pull.c
+++ b/builtin/pull.c
@@ -1052,6 +1052,9 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
                        show_advice_pull_non_ff();
        }
 
+       if (!can_ff && (!opt_ff || !strcmp(opt_ff, "--ff-only")))
+               die("unable to fast-forward");
+
        if (opt_rebase) {
                int ret = 0;
                int ran_ff = 0;

Then:

  ./git pull . topic
  ./git pull --no-rebase . topic
  ./git pull --rebase . topic

From 1 to 10 how broken would you say that is?

-------------------
t4013-diff-various.sh                            (Wstat: 256 Tests: 208 Failed: 91)
  Failed tests:  1, 49-63, 67-69, 71-91, 101-116, 120-124
                127-130, 138-141, 144-145, 147-150, 152-153
                171-175, 189-193, 201-204
  Non-zero exit status: 1
t5521-pull-options.sh                            (Wstat: 256 Tests: 20 Failed: 2)
  Failed tests:  12, 16
  Non-zero exit status: 1
t5524-pull-msg.sh                                (Wstat: 256 Tests: 3 Failed: 2)
  Failed tests:  2-3
  Non-zero exit status: 1
t5520-pull.sh                                    (Wstat: 256 Tests: 72 Failed: 39)
  Failed tests:  16, 19, 23-24, 26-29, 33-41, 43-46, 48-53
                55-57, 59-64, 69, 71-72
  Non-zero exit status: 1
t5533-push-cas.sh                                (Wstat: 256 Tests: 23 Failed: 2)
  Failed tests:  21-22
  Non-zero exit status: 1
t5553-set-upstream.sh                            (Wstat: 256 Tests: 19 Failed: 6)
  Failed tests:  11-14, 16-17
  Non-zero exit status: 1
t5604-clone-reference.sh                         (Wstat: 256 Tests: 33 Failed: 4)
  Failed tests:  13-16
  Non-zero exit status: 1
t5572-pull-submodule.sh                          (Wstat: 256 Tests: 64 Failed: 3)
  Failed tests:  62-64
  Non-zero exit status: 1
t6409-merge-subtree.sh                           (Wstat: 256 Tests: 12 Failed: 3)
  Failed tests:  9, 11-12
  Non-zero exit status: 1
t6402-merge-rename.sh                            (Wstat: 256 Tests: 46 Failed: 8)
  Failed tests:  2-8, 11
  Non-zero exit status: 1
t6417-merge-ours-theirs.sh                       (Wstat: 256 Tests: 7 Failed: 1)
  Failed test:  6
  Non-zero exit status: 1
t7601-merge-pull-config.sh                       (Wstat: 256 Tests: 32 Failed: 3)
  Failed tests:  11, 15-16
  Non-zero exit status: 1
t7603-merge-reduce-heads.sh                      (Wstat: 256 Tests: 13 Failed: 1)
  Failed test:  3
  Non-zero exit status: 1

-- 
Felipe Contreras