git/list[1] front-page[2] threads[3] people[4] search[5] about
 

Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible

From
Alex Henrie <alexhenrie24@gmail.com>
Date
Jul 12, 2021, 18:20 UTC
Message-ID
<CAMMLpeRX3iMwT9NJ+ULHgAhS3A=nAybgDYFHomkY3sif-H+F4g@mail.gmail.com>
In-Reply-To
<CABPp-BERS0iiiVhSsSs6dkqzBVTQgwJUjjKaZQEzRDGRUdObcQ@mail.gmail.com>
On Mon, Jul 12, 2021 at 11:51 AM Elijah Newren <newren@gmail.com> wrote:
Show 55 quoted lines
>
> On Mon, Jul 12, 2021 at 10:08 AM Junio C Hamano <gitster@pobox.com> wrote:
> >
> > Phillip Wood <phillip.wood123@gmail.com> writes:
> >
> > > Thanks for revising this patch, I like this approach much better. I do
> > > however have some concerns about the interaction of pull.ff with the
> > > rebase config and command line options. I'd naively expect the
> > > following behavior (where rebase can fast-forward if possible)
> > >
> > >   pull.ff  pull.rebase  commandline  action
> > >    only     not false                rebase
> > >    only     not false   --no-rebase  fast-forward only
> > >     *       not false    --ff-only   fast-forward only
> > >    only     not false    --ff        merge --ff
> > >    only     not false    --no-ff     merge --no-ff
> > >    only       false                  fast-forward only
> > >    only       false      --rebase    rebase
> > >    only       false      --ff        merge --ff
> > >    only       false      --no-ff     merge --no-ff
> >
> > Do you mean by "not false" something other than "true"?  Are you
> > trying to capture what should happen when these configuration
> > options are unspecified as well (and your "not false" is "either set
> > to true or unspecified")?  I ask because the first row does not make
> > any sense to me.  It seems to say
> >
> >     "If pull.ff is set to 'only', pull.rebase is not set to 'false',
> >     and the command line does not say anything, we will rebase".
>
> I think Phillip is trying to answer what to do when pull.ff and
> pull.rebase conflict.  If I read his "not false" means "is set to
> something other than false", then I agree with his table, but I think
> he missed covering some cases.
>
> I think his table says that pull.rebase=false cannot conflict with
> pull.ff settings, but any other value for pull.rebase can.  That makes
> sense to me.
>
> I'd similarly say that pull.ff=true cannot conflict with any
> pull.rebase settings...but that both pull.ff=only AND pull.ff=false
> conflict with pull.rebase={true,merges}.
>
> My opinion would be:
>   * conflicting command line flags results in the last one winning.
>   * --no-rebase makes pull.ff determine the action.
>   * --ff makes pull.rebase determine the action.
>   * any other command line flag (-r|--rebase|--no-ff|--ff-only)
> overrides both pull.ff and pull.rebase
>   * If no command line option is given, and pull.ff and pull.rebase
> conflict, then error out.
>
> I believe my recommendation above is consistent with every entry in
> Phillip's table except the first line (where I suggest erroring out
> instead).

I'm not sure that --no-ff should imply --no-rebase because `git rebase` actually has a --no-ff option to rewrite commits even when fast-forwarding is possible. And it's not really necessary to make --ff-only imply --no-rebase because we're going to make `git pull` handle --ff-only itself without invoking `git merge`. However, the rest of this proposal could be implemented in a straightforward manner by making --rebase on the command line imply --ff, and I think that would be a fine solution.

-Alex
Previous: Felipe ContrerasNext: Alex Henrie
Message 13 of 28 in “pull: abort if --ff-only is given and fast-forwarding is impossible”
  1. pull: abort if --ff-only is given and fast-forwarding is impossibleAlex Henrie, Jul 11, 2021
  2. Felipe ContrerasJul 11, 2021
  3. Alex HenrieJul 11, 2021
  4. Felipe ContrerasJul 11, 2021
  5. Phillip WoodJul 12, 2021
  6. Felipe ContrerasJul 12, 2021
  7. Alex HenrieJul 12, 2021
  8. Felipe ContrerasJul 12, 2021
  9. Junio C HamanoJul 12, 2021
  10. Felipe ContrerasJul 12, 2021
  11. Elijah NewrenJul 12, 2021
  12. Felipe ContrerasJul 12, 2021
  13. Alex HenrieJul 12, 2021
  14. Alex HenrieJul 12, 2021
  15. Junio C HamanoJul 12, 2021
  16. Felipe ContrerasJul 12, 2021
  17. Elijah NewrenJul 12, 2021
  18. Junio C HamanoJul 12, 2021
  19. Felipe ContrerasJul 12, 2021
  20. Elijah NewrenJul 12, 2021
  21. Elijah NewrenJul 12, 2021
  22. Felipe ContrerasJul 12, 2021
  23. Phillip WoodJul 12, 2021
  24. Son Luong NgocJul 14, 2021
  25. Felipe ContrerasJul 14, 2021
  26. Elijah NewrenJul 14, 2021
  27. Junio C HamanoJul 14, 2021
  28. Felipe ContrerasJul 14, 2021

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.