git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Felipe Contreras <felipe.contreras@gmail.com>
To: Richard Hansen <rhansen@bbn.com>,
	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: Sat, 03 May 2014 22:08:35 -0500	[thread overview]
Message-ID: <5365af33825c3_520db2b308bf@nysa.notmuch> (raw)
In-Reply-To: <5365691C.1010208@bbn.com>

Richard Hansen wrote:
> On 2014-05-03 05:26, Felipe Contreras wrote:
> > Richard Hansen wrote:
> > 
> >> I think the fundamental difference is in the relationship between the
> >> local and the remote branch (which branch derives from the other).
> >> The relationship between the branches determines what the user wants
> >> from 'git pull'.
> >>
> >> In my experience 'git pull' is mostly (only?) used for the following
> >> three tasks:
> > 
> > I agree.
> > 
> >>  1. update a local branch to incorporate the latest upstream changes
> >>
> >>     In this case, the local branch (master) is a derivative of the
> >>     upstream branch (origin/master).  The user wants all of the
> >>     commits in the remote branch to be in the local branch.  And the
> >>     user would like the local changes, if any, to descend from the tip
> >>     of the remote branch.
> > 
> > My current propsal of making `git pull` by default do --ff-only would
> > solve this.
> 
> It would go a long way toward improving the situation, yes.
> 
> > In addition I think by default 'master' should be merged to
> > 'origin/master', if say --merge is given.
> 
> This would break cases #2 and #3.  (With cases #2 and #3 you want the
> fetched branch to be the second parent, not the first.)
> 
> Or are you proposing that pull --merge should reverse the parents if and
> only if the remote ref is @{u}?

Only if no remote or branch are specified `git pull --merge`.

> > 
> >>     For this case, 'git pull --ff-only' followed by 'git rebase -p'
> >>     works well, as does 'git pull --rebase=preserve' if the user is
> >>     comfortable rebasing without reviewing the incoming commits first.
> > 
> > I suppose you mean a `git rebase -p` if the `git pull --ff-only` failed.
> 
> Yes.
> 
> > This might be OK on most projects, but not all.
> 
> The rebase only affects the local repository (the commits haven't been
> pushed yet or else they'd be in @{u} already), so I'd say it's more of
> an individual developer decision than a project decision.
> 
> In my opinion rebase would be the best option here, but if the project
> is OK with developers pushing merge or merge-there commits and the
> developer isn't yet comfortable with rebasing, then merge is also an
> acceptable option.

Precisely for that reason.

> >>  2. update a published feature branch with the latest changes from its
> >>     parent branch

> > We probably shouldn't change that.
> 
> If we change 'git pull' to default to --ff-only but let 'git pull
> $remote [$refspec]' continue to default to --ff then we have two
> different behaviors depending on how 'git pull' is invoked.  I'm worried
> that this would trip up users.  I'm not convinced that having two
> different behaviors would be bad, but I'm not convinced that it would be
> good either.

It is the only solution that has been proposed.

Moreover, while it's a bit worrisome, it wouldn't create any actual
problems. Since `git pull $what` remains the same, there's no problems
there. The only change would be on `git pull`.

Since most users are not going to do `git pull $what` therefore it would
only be a small subset of users that would notice the discrepancy
between running with $what, or not. And the only discrepancy they could
notice is that when they run `git pull $what` they expect it to be
--ff-only, or when the run `git pull` they don't. Only the former could
be an issue, but even then, it's highly unlikely that `git pull $what`
would ever be a fast-forward.

So althought conceptually it doesn't look clean, in reality there
wouldn't be any problems.

> >>  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.

>   * It makes commits easier to review.

The review in the vast majority of cases happens *before* the
integration.

And the problem comes when the integrator makes a mistake, which they
inevitable do (we all do), then there's no history about how the
conflict was resolved, and what whas the original patch.

That's why most people don't do this.

>   * Rebasing makes the commit history pretty and easier to understand.

It is more important to be able to track integration errors than to have
a pretty history. That is for most people.

I like to have a pretty history for my own local branches, but once
something gets integrated it's important to see who did exactly what
(the integrator did the merge).
> > It is very rare that an integrator is even able to do a fast-forward
> > merge anyway.
> 
> It depends on the level of project activity.  A project as active as the
> Linux kernel or Git will almost never have fast-forwards.  But
> occasional contributions by random users to a small, simple project will
> likely be fast-forwards.

And small simple projects don't care about such issues.

> > So being explicit about --no-ff might better, but it would
> > hardly make a difference. Either way, a good integrator would configure
> > pull.ff = false.
> 
> Configuring pull.ff = false is OK if the integrator only integrates and
> only uses one machine.  But if the integrator also wants to develop in
> the same repository, or if the integrator uses multiple machines to do
> the integration work (e.g., office desktop and laptop), then setting
> pull.ff may be less convenient, not more.

Any good integrator would find solutions for those problems easily.

Either way I don't see any proposed solutions.

> > I'd say `git pull origin master` already works fine for this case.
> 
> It does, but again preserving the current behavior would cause the
> behavior of 'git pull origin master' to be inconsistent with the
> proposed ff-only default for a plain 'git pull'.

Yes, it doesn't look clean. But I don't see any proposed alternatives.

-- 
Felipe Contreras

  reply	other threads:[~2014-05-04  3:19 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 [this message]
2014-05-04  7:49           ` Richard Hansen
2014-05-04 10:17             ` Felipe Contreras
2014-05-04 19:09               ` Richard Hansen
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=5365af33825c3_520db2b308bf@nysa.notmuch \
    --to=felipe.contreras@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=gitster@pobox.com \
    --cc=marcnarc@xiplink.com \
    --cc=rhansen@bbn.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).