Re: Pull is Mostly Evil

On 2014-05-03 05:26, Felipe Contreras wrote:

Richard Hansen wrote:

quoted

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.

quoted

 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}?

quoted

    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.

What happens after a `git pull --ff-only` fails should be totally
up to the user.

I tend to agree, mostly because I want users to have an opportunity to
review incoming commits before action is taken.  Also, though rebasing
would yield the nicest history, some users aren't yet comfortable with
rebase.  If a project is OK with silly little merge commits from users
that aren't comfortable with rebase, then I don't want to force everyone
to rebase by default.

As an added bonus:  Defaulting to --ff-only makes it possible for 'git
pull --all' to fast-forward every local branch to their configured
upstream, not just the currently checked-out branch.  I think this would
be a huge usability win.

quoted

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

    In this case, the local branch (foo) is a derivative of the
    upstream branch (origin/foo) which is itself a derivative of
    another branch (origin/master).  All commits in origin/master
    should be in origin/foo, and ideally all commits unique to
    origin/foo would descend from the tip of origin/master.

I don't understand why are you tainting the example with 'origin/foo',

Originally I didn't have this case in my list, but I added it after
thinking about Peff's comment:

  On 2014-05-02 17:48, Jeff King wrote:
  > One common workflow for GitHub users is to back-merge master into a
  > topic, because they want the final "integrated" version on the topic
  > branch.

This almost but doesn't quite fit neatly into the other two cases.  It's
not case #1 because the shared nature of origin/foo means that rebasing
origin/foo onto origin/master is usually bad instead of usually good.
It's not case #3 because rebasing origin/master commits onto origin/foo
(assuming that the user would usually want to rebase the topic branch
when integrating) would definitely be bad.

'foo' and 'origin/master' are enough for this example. In fact, the
mention of 'origin/master' made it wrong: after the pull not all the
commits of origin/master would be in origin/foo, you need a push for
that.

The push of foo to origin/foo was meant to be implied.

We have enough in our plate to taint this with yet another branch
and push.

For this case `git pull origin master` already work correctly for most
projects.

Yes, it does.

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.

quoted

 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:

  * It makes commits easier to review.  For example, assume the
    following commit history:

       * merge topic-foo
       |\
       | * merge master into topic-foo
       |/|
       * | tweak the behavior of Thing
       | |
       | * refactor Thing
       | |
       | * wrap long lines; no behavior changes
       |/
       * blah
       |
       ...

    In this case, the impact the "refactor Thing" and "wrap long lines"
    commits have on master can't be fully understood without also
    examining the presumed merge conflict resolution in the "merge
    master into topic-foo" commit.  Merge commits are very hard to
    review, even if (especially if?) there are no conflicts.
    Developers can diff 'merge topic-foo' to its first parent, but then
    they'll see lots of noise caused by the "wrap long lines" commit.

    If the integrator rebases first, then the history looks like this:

       * merge topic-foo
       |\
       | * refactor Thing
       | |
       | * wrap long lines; no behavior changes
       |/
       * tweak the behavior of Thing
       |
       * blah
       |
       ...

    Now the merge conflict resolution is integrated into the "refactor
    Thing" and "wrap long lines" commits, making them easier to review.

  * Rebasing makes the commit history pretty and easier to understand.
    Instead of this:

       * merge feature.xyz
       |\
       | * xyz part 3/3
       | |
       | * merge master into feature.xyz
       |/|
       * | merge feature.foo
       |\ \
       | | * xyz part 2/3
       | * | foo part 2/2
       | * | foo part 1/2
       | | * xyz part 1/3
       |/ /
       | /
       |/
       * merge feature.bar
       |\

    you get this:

       * merge feature.xyz
       |\
       | * xyz part 3/3
       | * xyz part 2/3
       | * xyz part 1/3
       |/
       * merge feature.foo
       |\
       | * foo part 2/2
       | * foo part 1/2
       |/
       * merge feature.bar
       |\

    When there are regularly dozens of active branches at a time, this
    improved readability can be quite valuable.

quoted

     * merge into local HEAD so that the first parent (if a real merge
       and not a ff) is the previous version of the main line of
       development and the second parent is the derivative work
     * merge --no-ff so that:
        - the merge can serve as a cover letter (who reviewed it,
          which bug reports were fixed, where the changes came from,
          etc.)
        - the commits that compose the new topic are grouped together
        - the first-parent path represents a series of completed tasks

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.

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.

Maybe it's OK to require integrators to get in the habit of typing 'git
pull --no-ff'.  Presumably integrators are experienced Git users, so
they can create their own 'git integrate' alias if they don't want to
have to remember to type '--no-ff' all the time.

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

-Richard