git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Richard Hansen <rhansen@bbn.com>
To: Felipe Contreras <felipe.contreras@gmail.com>,
	Junio C Hamano <gitster@pobox.com>,
	Marc Branchaud <marcnarc@xiplink.com>
Cc: Git Mailing List <git@vger.kernel.org>
Subject: Re: Pull is Mostly Evil
Date: Sun, 04 May 2014 15:09:05 -0400	[thread overview]
Message-ID: <53669051.6090204@bbn.com> (raw)
In-Reply-To: <536613bd14e24_1c89b0930cac@nysa.notmuch>

On 2014-05-04 06:17, Felipe Contreras wrote:
> Richard Hansen wrote:
>> On 2014-05-03 23:08, Felipe Contreras wrote:
>>> It is the only solution that has been proposed.
>>
>> It's not the only proposal -- I proposed a few alternatives in my
>> earlier email (though not in the form of code), and others have too.  In
>> particular:
>>
>>   * create a new 'git integrate' command/alias that behaves like 'git
>>     pull --no-ff'
> 
> Yeah but that's for a different issue altogheter. I doesn't solve the
> problems in 1. nor 2. nor 3.

'git integrate' would handle usage cases #2 (update a published branch
to its "parent" branch) and #3 (integrate a completed task into the main
line of development), making it feasible to change 'git pull' and 'git
pull $remote [$refspec]' to default to --ff-only to handle usage case #1
(update local branch to @{u}).

> 
>>   * change 'git pull' and 'git pull $remote [$refspec]' to do --ff-only
>>     by default
>>
>> Another option that I just thought of:  Instead of your proposed
>> pull.mode and branch.<name>.pullmode, add the following two sets of configs:
>>
>>   * pull.updateMode, branch.<name>.pullUpdateMode:
>>
>>     The default mode to use when running 'git pull' without naming a
>>     remote repository or when the named remote branch is @{u}.  Valid
>>     options: ff-only (default), merge-ff, merge-ff-there, merge-no-ff,
>>     merge-no-ff-there, rebase, rebase-here, rebase-here-then-merge-no-ff
> 
> Those are way too many options to be able to sensibly explain them.

Certainly this is too many options for a first patch series, but I don't
think they're unexplainable.  (I listed a bunch of options because I was
trying to envision where this might take us in the long run.)

For the first patch series, I'd expect:  merge (which uses the merge.ff
option to determine whether to ff, ff-only, or no-ff), rebase, and ff-only.

Later ff-only would be made the default.

Later some or all of the other options would be added depending on user
interest.

> 
>>   * pull.integrateMode, branch.<name>.pullIntegrateMode:
>>
>>     The default mode to use when running 'git pull $remote [$refspec]'
>>     when '$remote [$refspec]' is not @{u}.  Valid options are the same
>>     as those for pull.updateMode.  Default is merge-ff.
>>
>> This gives the default split behavior as you propose, but the user can
>> reconfigure to suit personal preference (and we can easily change the
>> default for one or the other if there's too much outcry).
> 
> If we reduce the number of options to begin with (more can be added
> later),

yup

> then it might make sense to have these two options.
> 
> However, that doesn't change the proposal you described above (1. 2.
> 3.).

Not sure what you mean.  I oulined three usage cases:
  #1 update local branch to @{u}
  #2 update a published branch to its "parent" branch
  #3 integrate a completed task into the main line of development

Having these two sets of options (updateMode and integrateMode) would
make it possible to configure plain 'git pull' to handle usage case #1
and 'git pull $remote [$refspec]' to handle usage cases #2 and #3.

Or the user could configure 'git pull' and 'git pull $remote [$refspec]'
to behave the same, in case they find the different behaviors to be too
confusing.

> There's something we can do, and let me clarify my proposal. What you
> described above is what I think should happen eventually, however, we
> can start by doing something like what my patch series is doing; issue a
> warning that the merge is not fast-forward and things might change in
> the future.

OK, let me rephrase to make sure I understand:

  1. leave the default behavior as-is for now (merge with local
     branch the first parent)
  2. add --merge argument
  3. add ff-only setting
  4. plan to eventually change the plain 'git pull' default to ff-only,
     but don't change the default yet
  5. add a warning if the plain 'git pull' is a non-ff
  6. wait and see how users react.  If they're OK with it, switch the
     default of the plain 'git pull' to ff-only.

Is that accurate?  If so, sounds OK to me.

> 
> If people find this behavior confusing they will complain in the mailing
> list.

true

> Although I suspect it would be for other reasons, not the 'git
> pull'/'git pull $there' division.

probably

> Either way we would see in the discussion.

sounds good to me

> 
>>>>>>  3. integrate a more-or-less complete feature/fix back into the line
>>>>>>     of development it forked off of
>>>>>>
>>>>>>     In this case the local branch is a primary line of development and
>>>>>>     the remote branch contains the derivative work.  Think Linus
>>>>>>     pulling in contributions.  Different situations will call for
>>>>>>     different ways to handle this case, but most will probably want
>>>>>>     some or all of:
>>>>>>
>>>>>>      * rebase the remote commits onto local HEAD
>>>>>
>>>>> No. Most people will merge the remote branch as it is. There's no reason
>>>>> to rebase, specially if you are creating a merge commit.
>>>>
>>>> I disagree.  I prefer to rebase a topic branch before merging (no-ff) to
>>>> the main line of development for a couple of reasons:
>>>
>>> Well that is *your* preference. Most people would prefer to preserve the
>>> history.
>>
>> Probably.  My point is that the behavior should be configurable, and I'd
>> like that particular behavior to be one of the options (but not the
>> default -- that wouldn't be appropriate).
> 
> All right. But I'm a bit overwhelmed by all the things to keep in mind.

Sure, this would be an option to add later.

> Does your proposed IntegradeMode/UpdateMode deal with this?

mode = rebase-here-then-merge-no-ff would do what I described

> Anyway, I'll try to grab what I can from previous discussions (mainly
> about switching the merge parents) and create a new thread with a
> summary.

That would be nice, thanks.

-Richard

  reply	other threads:[~2014-05-04 19:09 UTC|newest]

Thread overview: 50+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2014-05-02 15:37 Pull is Mostly Evil Marc Branchaud
2014-05-02 15:45 ` David Kastrup
2014-05-02 16:05   ` Philip Oakley
2014-05-02 19:05     ` Felipe Contreras
2014-05-02 22:34       ` Philip Oakley
2014-05-02 22:53         ` Jonathan Nieder
2014-05-03 20:24           ` Philip Oakley
2014-05-02 23:23         ` Felipe Contreras
2014-05-03 11:24           ` Philip Oakley
2014-05-03 11:30             ` Felipe Contreras
2014-05-02 19:31   ` David Lang
2014-05-02 19:37     ` David Kastrup
2014-05-02 18:13 ` Junio C Hamano
2014-05-02 19:11   ` Felipe Contreras
2014-05-02 20:06     ` Junio C Hamano
2014-05-02 20:58       ` Felipe Contreras
2014-05-02 21:48     ` Jeff King
2014-05-02 21:55       ` Felipe Contreras
2014-05-02 22:36         ` Jeff King
2014-05-02 23:27           ` Felipe Contreras
2014-05-03  2:18       ` David Kastrup
2014-05-06 22:06       ` Junio C Hamano
2014-05-06 22:19         ` Felipe Contreras
2014-05-03  7:56   ` Richard Hansen
2014-05-03  8:17     ` David Kastrup
2014-05-03  9:04       ` Felipe Contreras
2014-05-03  9:56         ` David Kastrup
2014-05-04  4:30           ` David Lang
2014-05-04  4:38             ` Felipe Contreras
2014-05-04  6:13               ` David Kastrup
2014-05-04  6:50               ` James Denholm
2014-05-04  7:48                 ` David Kastrup
2014-05-04  9:51                 ` Felipe Contreras
2014-05-04 10:37                   ` James Denholm
2014-05-04 11:02                     ` David Kastrup
2014-05-03  9:26     ` Felipe Contreras
2014-05-03 22:09       ` Richard Hansen
2014-05-04  3:08         ` Felipe Contreras
2014-05-04  7:49           ` Richard Hansen
2014-05-04 10:17             ` Felipe Contreras
2014-05-04 19:09               ` Richard Hansen [this message]
2014-05-04 21:13                 ` Felipe Contreras
2014-05-05  5:44                   ` Richard Hansen
2014-05-05  5:47                     ` Felipe Contreras
2014-05-07 22:37     ` Max Kirillov
2014-05-03 10:00   ` John Szakmeister
2014-05-05 15:39     ` Richard Hansen
2014-05-05 18:15       ` Felipe Contreras
2014-05-02 22:12 ` Philip Oakley
2014-05-09 19:49 ` Marc Branchaud

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=53669051.6090204@bbn.com \
    --to=rhansen@bbn.com \
    --cc=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=marcnarc@xiplink.com \
    /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
Be sure your reply has a Subject: header at the top and a blank line before the message body.
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).