git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Elijah Newren <newren@gmail.com>, Junio C Hamano <gitster@pobox.com>
Cc: "Phillip Wood" <phillip.wood123@gmail.com>,
	"Alex Henrie" <alexhenrie24@gmail.com>,
	"Git Mailing List" <git@vger.kernel.org>,
	"Ævar Arnfjörð" <avarab@gmail.com>,
	"Felipe Contreras" <felipe.contreras@gmail.com>
Subject: Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible
Date: Mon, 12 Jul 2021 13:20:00 -0500	[thread overview]
Message-ID: <60ec87d03f7c5_a231f20826@natae.notmuch> (raw)
In-Reply-To: <CABPp-BERS0iiiVhSsSs6dkqzBVTQgwJUjjKaZQEzRDGRUdObcQ@mail.gmail.com>

Elijah Newren wrote:
> 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).

No:

  git -c pull.ff=only -c pull.rebase=true pull --ff

In Phillip's table that does a merge, in your rules that's a rebase.


Moreover, since when does git do something different depending if the
action was configured or specified in the command line? They are
supposed to be two ways of doing th same thing:

  git -c pull.ff=only -c pull.rebase=true pull
  git -c pull.ff=only pull --rebase

Why would those do different things?

The documentation is pretty clear:

  See `pull.rebase`, `branch.<name>.rebase` and `branch.autoSetupRebase` in
  linkgit:git-config[1] if you want to make `git pull` always use
  `--rebase` instead of merging.

If you start adding exceptions on top of exceptions the documentation
will become a mess.

-- 
Felipe Contreras

  reply	other threads:[~2021-07-12 18:20 UTC|newest]

Thread overview: 28+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-07-11  1:26 Alex Henrie
2021-07-11 17:08 ` Felipe Contreras
2021-07-11 20:00   ` Alex Henrie
2021-07-11 21:41     ` Felipe Contreras
2021-07-12 10:21 ` Phillip Wood
2021-07-12 16:04   ` Felipe Contreras
2021-07-12 16:29   ` Alex Henrie
2021-07-12 17:43     ` Felipe Contreras
2021-07-12 17:08   ` Junio C Hamano
2021-07-12 17:30     ` Felipe Contreras
2021-07-12 17:50     ` Elijah Newren
2021-07-12 18:20       ` Felipe Contreras [this message]
2021-07-12 18:20       ` Alex Henrie
2021-07-12 18:24         ` Alex Henrie
2021-07-12 19:55           ` Junio C Hamano
2021-07-12 20:19             ` Felipe Contreras
2021-07-12 20:51             ` Elijah Newren
2021-07-12 23:00               ` Junio C Hamano
2021-07-12 23:05                 ` Felipe Contreras
2021-07-12 23:24                 ` Elijah Newren
2021-07-12 20:37         ` Elijah Newren
2021-07-12 21:06           ` Felipe Contreras
2021-07-12 17:54     ` Phillip Wood
2021-07-14  8:37 ` Son Luong Ngoc
2021-07-14 15:14   ` Felipe Contreras
2021-07-14 15:22   ` Elijah Newren
2021-07-14 17:19     ` Junio C Hamano
2021-07-14 17:31     ` Felipe Contreras

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=60ec87d03f7c5_a231f20826@natae.notmuch \
    --to=felipe.contreras@gmail.com \
    --cc=alexhenrie24@gmail.com \
    --cc=avarab@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=newren@gmail.com \
    --cc=phillip.wood123@gmail.com \
    --subject='Re: [PATCH] pull: abort if --ff-only is given and fast-forwarding is impossible' \
    /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

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).