Re: Pull is Mostly Evil

Richard Hansen wrote:

On 2014-05-03 23:08, Felipe Contreras wrote:

quoted

Richard Hansen wrote:

quoted

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

OK.  Let me summarize to make sure I understand your full proposal:

  1. if plain 'git pull', default to --ff-only
  2. if 'git pull --merge', default to --ff.  If the local branch can't
     be fast-forwarded to the upstream branch, then create a merge
     commit where the local branch is the *second* parent, not the first
  3. if 'git pull $remote [$refspec]', default to --merge --ff.  If the
     local branch can't be fast-forwarded to the remote branch, then
     create a merge commit where the remote branch is the second parent
     (the current behavior)

Is that accurate?

Yes, that is accurate. Note that 3. is the current behavior.

quoted

quoted

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.

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.

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

  * 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), then it might make sense to have these two options.

However, that doesn't change the proposal you described above (1. 2.
3.).

quoted

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.

Yes, it might not be a problem, but I'm still nervous.  I'd need more
input (e.g., user survey, broad mailing list consensus, long beta test
period, decree by a benevolent dictator) before I'd be comfortable with it.

The user surveys are not happening any more. The results were ignored by
the developers anyway.

Mailing list consensus might be possible, but that wouldn't tell us
much.

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.

If people find this behavior confusing they will complain in the mailing
list. Although I suspect it would be for other reasons, not the 'git
pull'/'git pull $there' division. Either way we would see in the
discussion.

quoted

quoted

quoted

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:

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.
Does your proposed IntegradeMode/UpdateMode deal with this?

I will try to gather a bunch of discussions and create a new thread to
summrize what is probably the best, and Intage/Update mode is as far as
I'm willing to go into considering options.

quoted

quoted

  * It makes commits easier to review.

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

True, although even when review happens before integration there is
value in making code archeology easier.

I think I explained below why "code archeology" is better served by
preserving the history.

quoted

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.

Good point, although if I was the integrator and there was a
particularly hairy conflict I'd still rebase but ask the original
contributor to review the results before merging (or ask the contributor
to rebase).

Sure, asking the contributor to rebase is best. However, sending the
rebase results is not that useful; the contributor would like to see
what actually changed so an interdiff might be more than enough. But
then that's basically the same as reviewing the merge commit.

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.

Cheers.

-- 
Felipe Contreras