Re: git pull --rebase should use fast forward merge if possible

From: Stefan Beller <sbeller@google.com>
Date: Jun 29, 2016, 20:40 UTC
Message-ID: <CAGZ79kYS=XWBpEBJsj95FV+j+HLxg=5xqwD73618t7ERYrKUfA@mail.gmail.com>
In-Reply-To: <xmqqtwgb52py.fsf@gitster.mtv.corp.google.com>

On Wed, Jun 29, 2016 at 10:23 AM, Junio C Hamano <gitster@pobox.com> wrote:

Show 7 quoted lines

> Junio C Hamano <gitster@pobox.com> writes:
>
>>> Is it possible to change the behavior of "git pull
>>> --rebase=true|preserve|interactive" to use a fast forward merge if the
>>> remote branch is ahead and the local branch contains no new commits?
>>
>> Interesting.  I do not think of a reason why we shouldn't.

Me neither.

Show 37 quoted lines

>>
>> If we were still working in scripted Porcelain, it would have been a
>> five minute hack, perhaps like this.
>>
>>  contrib/examples/git-pull.sh | 8 ++++++++
>>  1 file changed, 8 insertions(+)
>
> ... and if we have to work with built-ins, it becomes a lot larger
> than a five-minute hack, unfortunately.
>
> Something like this may have a chance of working ;-)
>
>  builtin/pull.c | 22 ++++++++++++++++++----
>  1 file changed, 18 insertions(+), 4 deletions(-)
>
> diff --git a/builtin/pull.c b/builtin/pull.c
> index bf3fd3f..777ae56 100644
> --- a/builtin/pull.c
> +++ b/builtin/pull.c
> @@ -878,10 +878,24 @@ int cmd_pull(int argc, const char **argv, const char *prefix)
>                 if (merge_heads.nr > 1)
>                         die(_("Cannot merge multiple branches into empty head."));
>                 return pull_into_void(*merge_heads.sha1, curr_head);
> -       } else if (opt_rebase) {
> -               if (merge_heads.nr > 1)
> -                       die(_("Cannot rebase onto multiple branches."));
> +       }
> +       if (opt_rebase && merge_heads.nr > 1)
> +               die(_("Cannot rebase onto multiple branches."));
> +
> +       if (opt_rebase) {
> +               struct commit_list *list = NULL;
> +               struct commit *merge_head, *head;
> +
> +               head = lookup_commit_reference(orig_head);
> +               commit_list_insert(head, &list);
> +               merge_head = lookup_commit_reference(merge_heads.sha1[0]);

The crashes for merge_heads.nr == 0. (I did not inspect code further up if this is caught before, I think it would trigger if you and the remote are on an initial commit with no parents?)

Show 9 quoted lines

> +               if (is_descendant_of(merge_head, list)) {
> +                       /* we can fast-forward this without invoking rebase */
> +                       opt_ff = "--ff-only";
> +                       return run_merge();
> +               }
>                 return run_rebase(curr_head, *merge_heads.sha1, rebase_fork_point);
> -       } else
> +       }
> +       else

Keep the style ?

Show 6 quoted lines

>                 return run_merge();
>  }
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html