first parent, commit graph layout, and pull merge direction

first parent, commit graph layout, and pull merge direction

[Andreas Krey]

Hi everyone,

I'm just looking into better displays of the commit graph (as
displayed with gitk, smartgit, fisheye) - they tend to quickly
dissolve into a heap of spaghetti.

We had the idea that treating the first parent specially would
have some advantage here - including graphically indicating which
one of the parents of a commit is the first parent. (For instance,
by letting that line leave the commit node at the top/bottom,
and the other(s) to the side.)

A short trial showed that representing first parent chains as
straight lines in the graph does actually improve understandability,
as feature branches clearly stand out as separate lines even when
they no longer carry a branch name.

Does any GUI already do that (treat first parent specially),
or does anybody think of doing such? I don't quite dare to
jump into the gitk code yet.

Also, there is an implication with 'git pull': You'd expect the
master branch to be a first parent line, but when I do a small
thing directly on master and need to pull before pushing back,
then origin/master is merged into my branch, and thus my side
branch becomes the first parent line.

So, feature discussion request: Invert the parent ordering
when doing git pull from upstream? Configurably so?

We actually thought about putting a restriction into our blessed
repo that it not only restricts to fast-forward pushed, but further
to only allow pushing new things that have the old branch head in
the first parent chain.

What do you think?

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

Andreas Krey <a.krey@gmx.de> writes:

> A short trial showed that representing first parent chains as
> straight lines in the graph does actually improve understandability,
> as feature branches clearly stand out as separate lines even when
> they no longer carry a branch name.

If you have a four-commit segment in your commit ancestry graph
(time flows from left to right; turn your head 90-degrees to the
right if you want a gitk representation):

    ---A--X
        \/
        /\
    ---B--Y

where X and Y are both merges between A and B, having A as their
first parent, how would you express such a graph with first-parent
chain going a straight line?

> Also, there is an implication with 'git pull': You'd expect the
> master branch to be a first parent line, but when I do a small
> thing directly on master and need to pull before pushing back,
> then origin/master is merged into my branch, and thus my side
> branch becomes the first parent line.

Don't do that, then.

Re: first parent, commit graph layout, and pull merge direction

[Andreas Krey]

On Wed, 22 May 2013 11:07:07 +0000, Junio C Hamano wrote:
...
> If you have a four-commit segment in your commit ancestry graph

I never had yet. :-(

> (time flows from left to right; turn your head 90-degrees to the
> right if you want a gitk representation):
> 
>     ---A--X
>         \/
>         /\
>     ---B--Y
> 
> where X and Y are both merges between A and B, having A as their
> first parent, how would you express such a graph with first-parent
> chain going a straight line?

Of course there are multiple possible straight lines and how it looks
depends on the order I use the existing heads to fish them out. (That
is, when the straight lines join, I need to bend one of them.) Assuming
I take the one where X is on, I expect a look like

-----A-------X-----
      \      |
       +- Y--------
          |  |
-----B----+--+

Branch heads that are reachable from other head are picked after those
that aren't reachable.

The point is to get the feature branches being displayed on separate
lanes (and thus visibly sticking out) and not being intermingled with
the longer-living branches.

...
> Don't do that, then.

:-) Problem is, in this case 'I' expands to about
    1<<7 people I need to educate on this.

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: first parent, commit graph layout, and pull merge direction

[John Szakmeister]

On Thu, May 23, 2013 at 5:06 AM, Andreas Krey <a.krey@gmx.de> wrote:
[snip]
> ...
>> Don't do that, then.
>
> :-) Problem is, in this case 'I' expands to about
>     1<<7 people I need to educate on this.

This is a feature of `git pull` that I really despise.  I really wish
`git pull` treated the remote as the first parent in its merge
operation.

-John

Re: first parent, commit graph layout, and pull merge direction

[Jeremy Rosen]

----- Mail original -----
> On Thu, May 23, 2013 at 5:06 AM, Andreas Krey <a.krey@gmx.de> wrote:
> [snip]
> > ...
> >> Don't do that, then.
> >
> > :-) Problem is, in this case 'I' expands to about
> >     1<<7 people I need to educate on this.
> 
> This is a feature of `git pull` that I really despise.  I really wish
> `git pull` treated the remote as the first parent in its merge
> operation.
> 

seconded...

github's network pages (which display the commit graph of projects) seems to follow the "first parent at the top" rule and the pull merges are standing out as "wrong" because of that...

Re: first parent, commit graph layout, and pull merge direction

[Andreas Krey]

On Thu, 23 May 2013 05:48:38 +0000, John Szakmeister wrote:
...
> This is a feature of `git pull` that I really despise.  I really wish
> `git pull` treated the remote as the first parent in its merge
> operation.

I'd actually only like it that way when pulling from
the tracking branch, not for any pull.

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: first parent, commit graph layout, and pull merge direction

[John Keeping]

On Thu, May 23, 2013 at 12:29:59PM +0200, Andreas Krey wrote:
> On Thu, 23 May 2013 05:48:38 +0000, John Szakmeister wrote:
> ...
> > This is a feature of `git pull` that I really despise.  I really wish
> > `git pull` treated the remote as the first parent in its merge
> > operation.
> 
> I'd actually only like it that way when pulling from
> the tracking branch, not for any pull.

I'll add my voice to the "annoyed by this" pile ;-)

I've been annoyed by this at $DAYJOB recently.  A lot of people seem to
blindly "git pull" without much thought about how the history is ending
up and what they actually want to do.

I wonder if it would make sense for "git pull" (with no arguments) to
pass "--ff-only" to git-merge, allowing this to be overridden with
--rebase and --merge (which doesn't currently exist).  With some
suitable advice output we could hopefully educate users about how to
shape their history.

Re: first parent, commit graph layout, and pull merge direction

[Felipe Contreras]

On Wed, May 22, 2013 at 6:50 AM, Andreas Krey <a.krey@gmx.de> wrote:
> Hi everyone,
>
> I'm just looking into better displays of the commit graph (as
> displayed with gitk, smartgit, fisheye) - they tend to quickly
> dissolve into a heap of spaghetti.
>
> We had the idea that treating the first parent specially would
> have some advantage here - including graphically indicating which
> one of the parents of a commit is the first parent. (For instance,
> by letting that line leave the commit node at the top/bottom,
> and the other(s) to the side.)

I don't understand; gitk already shows the first parent starting from
the bottom, and the merge commits arrive from the right side. What am
I missing?

-- 
Felipe Contreras

Re: first parent, commit graph layout, and pull merge direction

[Andreas Krey]

On Thu, 23 May 2013 06:34:38 +0000, Felipe Contreras wrote:
...
> I don't understand; gitk already shows the first parent starting from
> the bottom, and the merge commits arrive from the right side. What am
> I missing?

That this isn't (consistently) the case in complicated situations.
I'll need to make a picture (as in png).

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> I've been annoyed by this at $DAYJOB recently.  A lot of people seem to
> blindly "git pull" without much thought about how the history is ending
> up and what they actually want to do.

I think these two are essentially the same thing, and having an
option to flip the heads of a merge only solves a half of the
problem.

A merge that shows everybody else's work merged into your history
means you are the integrator, the keeper of the main history.  And
the first-parent view of the history is useful only when the keeper
of the main history takes good care of the main history.

When you are using a "central shared repository" workflow, if you
had and used an option to flip the heads of a merge to record what
you have done so far as a side branch of what everybody else did to
do the merge, or if you rebased your work on top of what everybody
else did, the first-parent view would make a bit more sense than
what you currently get.  At least, everybody else's work will not
appear as a side branch that does 47 unrelated things, and your work
will appear as a side branch.  That is a big plus.

But the other half of the problem still remains, i.e. "what they
actually want to do".  People tend to do too many "pull" when their
work is not ready, only to "catch up", and that is the real problem.

Instead of having a nice "these six commits marked as 'x' were done
on a branch forked some time ago, to address only this one issue and
to address it fully" history that explains how these commits were
related and these commits are the full solution to a single issue:

      x---x---x---x---x---x
     /                     \
 ---o---o---o---o---o---o---M---o---o---...

they end up with something like this, even with the "flip the heads
of a merge" option, by pulling too often:

      x---x   x---x---x   x
     /     \ /         \ / \
 ---o---o---M---o---o---M---M---o---o---...

The result fragments otherwise a logical and clean "single strand of
pearls to fully address the issue, consisting of 6 commits", into
separate and seemingly unrelated pieces.

Imagine that other people are working the same way, and the commits
marked with 'o' are merges of side branches they add their half-way
work to the main history similar to what happened in the second
illustration above.  You would get this history:

      x---x   x---x---x   x
     /     \ /         \ / \
 ---o---o---M---o---M---M---M---o---o---...
             \     /
              y---y 

Nothing, other than the labels I used in the picture, ties these
'x's together while differentiating them from 'y's, so you lost an
important information.  Unless people stop doing that too many
"pull"s that are used only to "catch up", even with the "flip the
heads of a merge" option, you will not get a history that yields a
good first-parent view.

That gets back to what I said in the second paragraph of this
message.  When you "pull" from the central shared repository, with
the "flip the heads of a merge" option, you are acting as the keeper
of the main history at that point, and you are responsible for
taking good care of it.  If you make a 2+3+1=6 mess as depicted in
the last illustration above, you are failing to do so.

One obvious way to solve it is to use a topic branch workflow (the
first picture above; 'x's are built not on local 'master'), and you
do a "git pull" from the shared repository while you are on your
'master', which is free of your 'x's until that 6-commit series is
complete and ready.  Then you locally merge that topic branch and
push it back for everybody to see, which will give you the first
picture in this message.  Incidentally, this does not need the "flip
the heads" option.

Solving half a problem is better than solving no problem, and
especially because not all changes need to be multi-commit series
but can be done directly, perfectly and fully on the local 'master'
(i.e. 2+3+1=6 split would not happen for such changes).  For these
reasons, I personally am not strongly opposed to a "flip the heads"
option, if implemented cleanly.

But people need to realize that it is not solving the other half, a
more fundamental problem some people have in their workflow.

Re: first parent, commit graph layout, and pull merge direction

[John Keeping]

On Thu, May 23, 2013 at 09:01:15AM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > I've been annoyed by this at $DAYJOB recently.  A lot of people seem to
> > blindly "git pull" without much thought about how the history is ending
> > up and what they actually want to do.
> 
> I think these two are essentially the same thing, and having an
> option to flip the heads of a merge only solves a half of the
> problem.
> 
> A merge that shows everybody else's work merged into your history
> means you are the integrator, the keeper of the main history.  And
> the first-parent view of the history is useful only when the keeper
> of the main history takes good care of the main history.
> 
> When you are using a "central shared repository" workflow, if you
> had and used an option to flip the heads of a merge to record what
> you have done so far as a side branch of what everybody else did to
> do the merge, or if you rebased your work on top of what everybody
> else did, the first-parent view would make a bit more sense than
> what you currently get.  At least, everybody else's work will not
> appear as a side branch that does 47 unrelated things, and your work
> will appear as a side branch.  That is a big plus.
> 
> But the other half of the problem still remains, i.e. "what they
> actually want to do".  People tend to do too many "pull" when their
> work is not ready, only to "catch up", and that is the real problem.
...
> One obvious way to solve it is to use a topic branch workflow (the
> first picture above; 'x's are built not on local 'master'), and you
> do a "git pull" from the shared repository while you are on your
> 'master', which is free of your 'x's until that 6-commit series is
> complete and ready.  Then you locally merge that topic branch and
> push it back for everybody to see, which will give you the first
> picture in this message.  Incidentally, this does not need the "flip
> the heads" option.

Yes, I don't think this is as much of a problem when using a topic
branch workflow, because it's clear what the history should look like
and users are expected to get it right.

Where I see this is when people are aiming for a linear history but
don't get that because with "git pull" to catch up they end up with
these backwards merges.  In these cases, I think what users really want
is "git pull --rebase".

I have to wonder how often "git pull" with no arguments actually does
what users really want (even if they don't know it!) when it doesn't
result in a fast-forward (and pull.rebase isn't configured).

Hence my suggestion to error when "git pull" doesn't result in a
fast-forward and no branch name is specified.  We could give some advice
like:

    Your local changes are not included in the local branch and you
    haven't told Git how to preserve them.

    If you want to rebase your changes onto the modified upstream
    branch, run:

        git pull --rebase

> Solving half a problem is better than solving no problem, and
> especially because not all changes need to be multi-commit series
> but can be done directly, perfectly and fully on the local 'master'
> (i.e. 2+3+1=6 split would not happen for such changes).  For these
> reasons, I personally am not strongly opposed to a "flip the heads"
> option, if implemented cleanly.
> 
> But people need to realize that it is not solving the other half, a
> more fundamental problem some people have in their workflow.

Yes, but some users don't realise that their workflow is broken, and
perhaps we can nudge them in the right direction.

Re: first parent, commit graph layout, and pull merge direction

[Andreas Krey]

On Thu, 23 May 2013 11:06:57 +0000, Andreas Krey wrote:
...
> ...
> > Don't do that, then.

Ouch, you're right. The problem is not actually in the
pull; only the *last* pull into a feature branch that
then get pushed back ff to master needs to be reversed.

And at that time you don't know it's the last one
-> swap parents before the push if necessary.

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> I have to wonder how often "git pull" with no arguments actually does
> what users really want (even if they don't know it!) when it doesn't
> result in a fast-forward (and pull.rebase isn't configured).

If you are in a totally centralized shared repository mindset
without using topic branch workflow, --first-parent would not help
you.  In your history the second parent is more likely to be the
mainline.

So for them "git pull" that either fast-forward when it can, or
makes a merge that records the then-current state of the central
shared repository, is perfectly sensible.  They will view gitk and
see all the changes, "git shortlog" and "git log --no-merges" will
give them what they expect.

> Hence my suggestion to error when "git pull" doesn't result in a
> fast-forward and no branch name is specified.  We could give some advice
> like:
>
>     Your local changes are not included in the local branch and you
>     haven't told Git how to preserve them.
>
>     If you want to rebase your changes onto the modified upstream
>     branch, run:
>
>         git pull --rebase

I can parse the first paragraph above, but cannot make much sense
out of it.  Unless you are talking about local changes that are not
committed yet, that is.  But in that case I fail to see what it has
to do with the current discussion, or suggestion to use rebase.

>> But people need to realize that it is not solving the other half, a
>> more fundamental problem some people have in their workflow.
>
> Yes, but some users don't realise that their workflow is broken, and
> perhaps we can nudge them in the right direction.

I actually avoided mentioning that deliberately, because I think the
"flip the head when merging" encourages people to (1) work directly
on 'master' and (2) pull too often when they shouldn't.

That is detrimental if your goal is to nudge them in the right
direction.

Re: first parent, commit graph layout, and pull merge direction

[John Keeping]

On Thu, May 23, 2013 at 02:01:39PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> > I have to wonder how often "git pull" with no arguments actually does
> > what users really want (even if they don't know it!) when it doesn't
> > result in a fast-forward (and pull.rebase isn't configured).
> 
> If you are in a totally centralized shared repository mindset
> without using topic branch workflow, --first-parent would not help
> you.  In your history the second parent is more likely to be the
> mainline.
> 
> So for them "git pull" that either fast-forward when it can, or
> makes a merge that records the then-current state of the central
> shared repository, is perfectly sensible.  They will view gitk and
> see all the changes, "git shortlog" and "git log --no-merges" will
> give them what they expect.

Yes, but for people used to a cleaner history it's confusing to see the
mainline branch and one small change the wrong way round.  When I see
people doing this, it's normally something like:

    ... do some work for several hours...
    git commit -a
    git push
    # fails because it's not a fast forward
    git pull
    git push

In this scenario, just adding --rebase to "git pull" actually results in
a much more sensible history.

> > Hence my suggestion to error when "git pull" doesn't result in a
> > fast-forward and no branch name is specified.  We could give some advice
> > like:
> >
> >     Your local changes are not included in the local branch and you
> >     haven't told Git how to preserve them.
> >
> >     If you want to rebase your changes onto the modified upstream
> >     branch, run:
> >
> >         git pull --rebase
> 
> I can parse the first paragraph above, but cannot make much sense
> out of it.  Unless you are talking about local changes that are not
> committed yet, that is.  But in that case I fail to see what it has
> to do with the current discussion, or suggestion to use rebase.

This isn't about "swap parents", it's about helping people realise that
just "git pull" isn't necessarily the best thing for them to do, and
that they may want --rebase.

So I was asking if it would be sensible (possibly in Git 2.0) to make
git-pull pass --ff-only to git-merge by default.

Re: first parent, commit graph layout, and pull merge direction

[Felipe Contreras]

On Thu, May 23, 2013 at 4:55 PM, John Keeping <john@keeping.me.uk> wrote:

> So I was asking if it would be sensible (possibly in Git 2.0) to make
> git-pull pass --ff-only to git-merge by default.

Definitely yes.

-- 
Felipe Contreras

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> This isn't about "swap parents", it's about helping people realise that
> just "git pull" isn't necessarily the best thing for them to do, and
> that they may want --rebase.
>
> So I was asking if it would be sensible (possibly in Git 2.0) to make
> git-pull pass --ff-only to git-merge by default.

Unless your primary user base is those who use Git as a deployment
tool to always follow along the tip of some external repository
without doing anything on your own on the branch you run your "git
pull" on, defaulting it to --ff-only does not make much sense to me.

If the proposal were to make pull.rebase the default at a major
version bump and force all integrators and other people who are
happy with how "pull = fetch + merge" (not "fetch + rebase") works
to say "pull.rebase = false" in their configuration, I think I can
see why some people may think it makes sense, though.

But neither is an easy sell, I would imagine.  It is not about
passing me, but about not hurting users like kernel folks we
accumulated over 7-8 years.

Also "rebase" of the branch you attempted to push out is sometimes a
good solution (fixing "just a small change on 'master'" that was
beaten by somebody else pushing first), but is a bad workaround (you
had many changes on that branch, which would have been better if
they were done on a topic branch, but you do not want to merge with
the upstream because you worked on 'master') some other times, so I
have this suspicion that 'pull.rebase' is not necessarily a good
thing to encourage in the first place.

Re: first parent, commit graph layout, and pull merge direction

[Felipe Contreras]

On Thu, May 23, 2013 at 5:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
> John Keeping <john@keeping.me.uk> writes:
>
>> This isn't about "swap parents", it's about helping people realise that
>> just "git pull" isn't necessarily the best thing for them to do, and
>> that they may want --rebase.
>>
>> So I was asking if it would be sensible (possibly in Git 2.0) to make
>> git-pull pass --ff-only to git-merge by default.
>
> Unless your primary user base is those who use Git as a deployment
> tool to always follow along the tip of some external repository
> without doing anything on your own on the branch you run your "git
> pull" on, defaulting it to --ff-only does not make much sense to me.

A lot of people do stuff, but the rebase it.

> If the proposal were to make pull.rebase the default at a major
> version bump and force all integrators and other people who are
> happy with how "pull = fetch + merge" (not "fetch + rebase") works
> to say "pull.rebase = false" in their configuration, I think I can
> see why some people may think it makes sense, though.

That makes perfect sense, because the people that are not familiar
with Git more often than not end up making merges by mistake, and the
ones that are familiar with it can easily configure it to do what they
want, or just 'git pull --merge', or 'git fetch'+'git merge' (we
should make merge.defaulttoupstream=true as well).

> But neither is an easy sell, I would imagine.  It is not about
> passing me, but about not hurting users like kernel folks we
> accumulated over 7-8 years.

I've worked in the Linux kernel, and in my experience the vast vast
majority of kernel developers don't do merges; they send patches. It's
only the lieutenants that might do that, and although there are a lot,
they don't surpass the 200, and they most definitely know how to
configure Git to do what they need. And even then, most of them don't
do merges, but create a linear history for Linus to merge.

So the only one who does really rely on merges is Linus, I think he
would have no problems configuring Git.

It is also my experience that most people don't do 'git pull', because
it rarely does what one wants; 'upstream' is still too cumbersome for
most people.

> Also "rebase" of the branch you attempted to push out is sometimes a
> good solution (fixing "just a small change on 'master'" that was
> beaten by somebody else pushing first), but is a bad workaround (you
> had many changes on that branch, which would have been better if
> they were done on a topic branch, but you do not want to merge with
> the upstream because you worked on 'master') some other times, so I
> have this suspicion that 'pull.rebase' is not necessarily a good
> thing to encourage in the first place.

Too bad, that's what most people recommend; 'git fetch'+'git rebase'.
That's the only way newcomers can avoid the ugliness of 'upstream',
and avoid making atrocious merges.

It's silly that the people familiar with Git has to explain this to
each and every newcomer.

-- 
Felipe Contreras

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> On Thu, May 23, 2013 at 5:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> John Keeping <john@keeping.me.uk> writes:
>>
>>> This isn't about "swap parents", it's about helping people realise that
>>> just "git pull" isn't necessarily the best thing for them to do, and
>>> that they may want --rebase.
>>>
>>> So I was asking if it would be sensible (possibly in Git 2.0) to make
>>> git-pull pass --ff-only to git-merge by default.
>>
>> Unless your primary user base is those who use Git as a deployment
>> tool to always follow along the tip of some external repository
>> without doing anything on your own on the branch you run your "git
>> pull" on, defaulting it to --ff-only does not make much sense to me.
>
> A lot of people do stuff, but the rebase it.

If I am parsing the above properly, I think that is only saying that
"pull --rebase" makes sense for people who do real work, which I am
not disagreeing.

>> If the proposal were to make pull.rebase the default at a major
>> version bump and force all integrators and other people who are
>> happy with how "pull = fetch + merge" (not "fetch + rebase") works
>> to say "pull.rebase = false" in their configuration, I think I can
>> see why some people may think it makes sense, though.
>
> That makes perfect sense, because the people that are not familiar
> with Git more often than not end up making merges by mistake, and the
> ones that are familiar with it can easily configure it to do what they
> want

Yes, in theory.  The transition needs a major version bump, but it
is doable (with unknown level of resistance).

>> But neither is an easy sell, I would imagine.  It is not about
>> passing me, but about not hurting users like kernel folks we
>> accumulated over 7-8 years.
>
> I've worked in the Linux kernel, and in my experience the vast vast
> majority of kernel developers don't do merges; they send patches. It's
> only the lieutenants that might do that, and although there are a lot,
> they don't surpass the 200, and they most definitely know how to
> configure Git to do what they need. And even then, most of them don't
> do merges, but create a linear history for Linus to merge.
>
> So the only one who does really rely on merges is Linus, I think he
> would have no problems configuring Git.

That is not something I can agree or disagree without looping
somebody whose judgement I can trust from the kernel circle ;-).

Re: first parent, commit graph layout, and pull merge direction

[Felipe Contreras]

On Thu, May 23, 2013 at 5:54 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> On Thu, May 23, 2013 at 5:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
>>> John Keeping <john@keeping.me.uk> writes:
>>>
>>>> This isn't about "swap parents", it's about helping people realise that
>>>> just "git pull" isn't necessarily the best thing for them to do, and
>>>> that they may want --rebase.
>>>>
>>>> So I was asking if it would be sensible (possibly in Git 2.0) to make
>>>> git-pull pass --ff-only to git-merge by default.
>>>
>>> Unless your primary user base is those who use Git as a deployment
>>> tool to always follow along the tip of some external repository
>>> without doing anything on your own on the branch you run your "git
>>> pull" on, defaulting it to --ff-only does not make much sense to me.
>>
>> A lot of people do stuff, but the rebase it.
>
> If I am parsing the above properly, I think that is only saying that
> "pull --rebase" makes sense for people who do real work, which I am
> not disagreeing.

You claimed that 'git pull' (--ff-only) only makes sense if the
primary user-base doesn't do any work on it, but that's not true; they
can do a 'git rebase' after such pull (or a merge).

We don't have to assume our primary user-base wants to do full fledged
merges, in fact, such assumption would be wrong.

>>> If the proposal were to make pull.rebase the default at a major
>>> version bump and force all integrators and other people who are
>>> happy with how "pull = fetch + merge" (not "fetch + rebase") works
>>> to say "pull.rebase = false" in their configuration, I think I can
>>> see why some people may think it makes sense, though.
>>
>> That makes perfect sense, because the people that are not familiar
>> with Git more often than not end up making merges by mistake, and the
>> ones that are familiar with it can easily configure it to do what they
>> want
>
> Yes, in theory.  The transition needs a major version bump, but it
> is doable (with unknown level of resistance).

Isn't that what wer are discussing here?

>>> But neither is an easy sell, I would imagine.  It is not about
>>> passing me, but about not hurting users like kernel folks we
>>> accumulated over 7-8 years.
>>
>> I've worked in the Linux kernel, and in my experience the vast vast
>> majority of kernel developers don't do merges; they send patches. It's
>> only the lieutenants that might do that, and although there are a lot,
>> they don't surpass the 200, and they most definitely know how to
>> configure Git to do what they need. And even then, most of them don't
>> do merges, but create a linear history for Linus to merge.
>>
>> So the only one who does really rely on merges is Linus, I think he
>> would have no problems configuring Git.
>
> That is not something I can agree or disagree without looping
> somebody whose judgement I can trust from the kernel circle ;-).

See section 16) in Documentation/SubmittingPatches, notice how the
whole section is written with Linus in mind. Some maintainers do have
sub-maintainers that send pull requests to them, not Linus, but they
are the minority. But most definitely pull requests are not for the
general population (except in a few very rare exceptions maybe).

-- 
Felipe Contreras

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

>>>> Unless your primary user base is those who use Git as a deployment
>>>> tool to always follow along the tip of some external repository
>>>> without doing anything on your own on the branch you run your "git
>>>> pull" on, defaulting it to --ff-only does not make much sense to me.
>>>
>>> A lot of people do stuff, but the rebase it.
>>
>> If I am parsing the above properly, I think that is only saying that
>> "pull --rebase" makes sense for people who do real work, which I am
>> not disagreeing.
>
> You claimed that 'git pull' (--ff-only) only makes sense if the
> primary user-base doesn't do any work on it, but that's not true; they
> can do a 'git rebase' after such pull (or a merge).

Either you misread what I wrote or I was unclear.  I really meant
"anything on your own *ON* THE BRANCH YOU RUN your "git pull" on".
With

	git checkout frotz ; git pull --ff-only

you do not do anything "on frotz" other than following along.  You
can of course commit, rebase and all others on other branches like
xyzzy and push them out directly.  But you cannot even do this once

	git checkout frotz; git merge xyzzy

if you expect the next "git checkout frotz; git pull --ff-only" will
keep working usefuly.


> We don't have to assume our primary user-base wants to do full fledged
> merges, in fact, such assumption would be wrong.

I think we are in agreement on that point already.

An assumption that people who do merges are somehow more well versed
in Git and are more capable than others to configure their
repository or they will not be annoyed if you asked them a single
configuration change is also wrong, though.

Re: first parent, commit graph layout, and pull merge direction

[Felipe Contreras]

On Thu, May 23, 2013 at 6:26 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>>>>> Unless your primary user base is those who use Git as a deployment
>>>>> tool to always follow along the tip of some external repository
>>>>> without doing anything on your own on the branch you run your "git
>>>>> pull" on, defaulting it to --ff-only does not make much sense to me.
>>>>
>>>> A lot of people do stuff, but the rebase it.
>>>
>>> If I am parsing the above properly, I think that is only saying that
>>> "pull --rebase" makes sense for people who do real work, which I am
>>> not disagreeing.
>>
>> You claimed that 'git pull' (--ff-only) only makes sense if the
>> primary user-base doesn't do any work on it, but that's not true; they
>> can do a 'git rebase' after such pull (or a merge).
>
> Either you misread what I wrote or I was unclear.  I really meant
> "anything on your own *ON* THE BRANCH YOU RUN your "git pull" on".
> With
>
>         git checkout frotz ; git pull --ff-only
>
> you do not do anything "on frotz" other than following along.  You
> can of course commit, rebase and all others on other branches like
> xyzzy and push them out directly.  But you cannot even do this once
>
>         git checkout frotz; git merge xyzzy
>
> if you expect the next "git checkout frotz; git pull --ff-only" will
> keep working usefuly.

Unless you rebase. We could of course have a
'branch.<name>.allow_merge' configuration that gets automatically
turned on the first time a 'git merge' is executed, but I feel that
creates more inconsistency.

>> We don't have to assume our primary user-base wants to do full fledged
>> merges, in fact, such assumption would be wrong.
>
> I think we are in agreement on that point already.
>
> An assumption that people who do merges are somehow more well versed
> in Git and are more capable than others to configure their
> repository or they will not be annoyed if you asked them a single
> configuration change is also wrong, though.

s/people who do merges/people who should do merges/

And no, it's not wrong. People who do merges should know what they are doing.

The alternatives are these:

a) you annoy the vast majority of the user-base by making 'git pull' a
dangerous operation that should be avoided, and replaced with 'git
fetch'+'git rebase'.

b) you annoy a minority of the user-base by making 'git pull' not do
the merge the expected, so they have to do +'git merge' (which is
already less of a change than a)), or configure the default (which
they most likely are able to do, if they did intent to do a merge).

b) is clearly superior.

-- 
Felipe Contreras

Re: first parent, commit graph layout, and pull merge direction

[Linus Torvalds]

On Thu, May 23, 2013 at 3:11 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> If the proposal were to make pull.rebase the default at a major
> version bump and force all integrators and other people who are
> happy with how "pull = fetch + merge" (not "fetch + rebase") works
> to say "pull.rebase = false" in their configuration, I think I can
> see why some people may think it makes sense, though.
>
> But neither is an easy sell, I would imagine.  It is not about
> passing me, but about not hurting users like kernel folks we
> accumulated over 7-8 years.

It would be a *horrible* mistake to make "rebase" the default, because
it's so much easier to screw things up that way.

That said, making "no-ff" the default, and then if that fails, saying

   The pull was not a fast-forward pull, please say if you want to
merge or rebase.
   Use either

        git pull --rebase
        git pull --merge

   You can also use "git config pull.merge true" or "git config
pull.rebase true"
   to set this once for this project and forget about it.

That way, people who want the existing behavior could just do that

    git config pull.merge true

once, and they'd not even notice.

Hmm? Better yet, make it per-branch.

                   Linus

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

Linus Torvalds <torvalds@linux-foundation.org> writes:

> It would be a *horrible* mistake to make "rebase" the default, because
> it's so much easier to screw things up that way.
>
> That said, making "no-ff" the default, and then if that fails, saying
>
>    The pull was not a fast-forward pull, please say if you want to
> merge or rebase.
>    Use either
>
>         git pull --rebase
>         git pull --merge
>
>    You can also use "git config pull.merge true" or "git config
> pull.rebase true"
>    to set this once for this project and forget about it.
>
> That way, people who want the existing behavior could just do that
>
>     git config pull.merge true
>
> once, and they'd not even notice.
>
> Hmm? Better yet, make it per-branch.

I would assume that "no-ff" above was meant to be "--ff-only" from
the first part of the message.

I also would assume that I can rephrase that setting pull.merge
(which does not exist) as setting pull.rebase explicitly to false
instead (i.e. missing pull.rebase and pull.rebase that is explicitly
set to false would mean two different things).

I have to think about this a bit to convince myself that the message
is clear enough and useful for those this updated behaviour is
trying to help.  After reading the above message three times, I
still cannot shake the impression that we are just covering our
backside to be able to say "we told you already and you chose
poorly", in case things go wrong for them later.

Re: first parent, commit graph layout, and pull merge direction

[Linus Torvalds]

On Thu, May 23, 2013 at 5:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
>
> I would assume that "no-ff" above was meant to be "--ff-only" from
> the first part of the message.

Yeah, I may need more coffee..

> I also would assume that I can rephrase that setting pull.merge
> (which does not exist) as setting pull.rebase explicitly to false
> instead (i.e. missing pull.rebase and pull.rebase that is explicitly
> set to false would mean two different things).

Yeah, sounds good to me, and doesn't really sound like it would
confuse/annoy anybody as long as it was clearly documented.

              Linus

Re: first parent, commit graph layout, and pull merge direction

[Felipe Contreras]

On Thu, May 23, 2013 at 7:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Linus Torvalds <torvalds@linux-foundation.org> writes:
>
>> It would be a *horrible* mistake to make "rebase" the default, because
>> it's so much easier to screw things up that way.
>>
>> That said, making "no-ff" the default, and then if that fails, saying
>>
>>    The pull was not a fast-forward pull, please say if you want to
>> merge or rebase.
>>    Use either
>>
>>         git pull --rebase
>>         git pull --merge
>>
>>    You can also use "git config pull.merge true" or "git config
>> pull.rebase true"
>>    to set this once for this project and forget about it.
>>
>> That way, people who want the existing behavior could just do that
>>
>>     git config pull.merge true
>>
>> once, and they'd not even notice.
>>
>> Hmm? Better yet, make it per-branch.
>
> I would assume that "no-ff" above was meant to be "--ff-only" from
> the first part of the message.
>
> I also would assume that I can rephrase that setting pull.merge
> (which does not exist) as setting pull.rebase explicitly to false
> instead (i.e. missing pull.rebase and pull.rebase that is explicitly
> set to false would mean two different things).
>
> I have to think about this a bit to convince myself that the message
> is clear enough and useful for those this updated behaviour is
> trying to help.  After reading the above message three times, I
> still cannot shake the impression that we are just covering our
> backside to be able to say "we told you already and you chose
> poorly", in case things go wrong for them later.

FWIW this is the message Mercurial users get (and they often say
Mercurial's UI makes more sense):

pushing to /tmp/foo
searching for changes
abort: push creates new remote head 77eafc4313d5!
(you should pull and merge or use push -f to force)

-- 
Felipe Contreras

Re: first parent, commit graph layout, and pull merge direction

[Felipe Contreras]

On Thu, May 23, 2013 at 7:25 PM, Felipe Contreras
<felipe.contreras@gmail.com> wrote:
> On Thu, May 23, 2013 at 7:21 PM, Junio C Hamano <gitster@pobox.com> wrote:
>> Linus Torvalds <torvalds@linux-foundation.org> writes:
>>
>>> It would be a *horrible* mistake to make "rebase" the default, because
>>> it's so much easier to screw things up that way.
>>>
>>> That said, making "no-ff" the default, and then if that fails, saying
>>>
>>>    The pull was not a fast-forward pull, please say if you want to
>>> merge or rebase.
>>>    Use either
>>>
>>>         git pull --rebase
>>>         git pull --merge
>>>
>>>    You can also use "git config pull.merge true" or "git config
>>> pull.rebase true"
>>>    to set this once for this project and forget about it.
>>>
>>> That way, people who want the existing behavior could just do that
>>>
>>>     git config pull.merge true
>>>
>>> once, and they'd not even notice.
>>>
>>> Hmm? Better yet, make it per-branch.
>>
>> I would assume that "no-ff" above was meant to be "--ff-only" from
>> the first part of the message.
>>
>> I also would assume that I can rephrase that setting pull.merge
>> (which does not exist) as setting pull.rebase explicitly to false
>> instead (i.e. missing pull.rebase and pull.rebase that is explicitly
>> set to false would mean two different things).
>>
>> I have to think about this a bit to convince myself that the message
>> is clear enough and useful for those this updated behaviour is
>> trying to help.  After reading the above message three times, I
>> still cannot shake the impression that we are just covering our
>> backside to be able to say "we told you already and you chose
>> poorly", in case things go wrong for them later.
>
> FWIW this is the message Mercurial users get (and they often say
> Mercurial's UI makes more sense):
>
> pushing to /tmp/foo
> searching for changes
> abort: push creates new remote head 77eafc4313d5!
> (you should pull and merge or use push -f to force)

Er, that's for push, but I don't see why something small like that
wouldn't make sense:

The pull was not fast-forward, please either merge or rebase.

-- 
Felipe Contreras

Re: first parent, commit graph layout, and pull merge direction

[John Keeping]

On Thu, May 23, 2013 at 06:53:36PM -0500, Felipe Contreras wrote:
> The alternatives are these:
> 
> a) you annoy the vast majority of the user-base by making 'git pull' a
> dangerous operation that should be avoided, and replaced with 'git
> fetch'+'git rebase'.
> 
> b) you annoy a minority of the user-base by making 'git pull' not do
> the merge the expected, so they have to do +'git merge' (which is
> already less of a change than a)), or configure the default (which
> they most likely are able to do, if they did intent to do a merge).

Note that in my email that started this, I tried to be clear that I was
talking about "git pull" *without a branch name*.  If this user
explicitly says "git pull remote branch" then I consider that a clear
indication that they really do mean to perform a merge; I would not
recommend changing the current behaviour in that case.

If the user just says "git pull" then it is more likely that they are
just trying to synchronise with the upstream branch, in which case they
probably don't actually want a merge.

Re: first parent, commit graph layout, and pull merge direction

[Holger Hellmuth (IKS)]

Am 23.05.2013 21:25, schrieb Andreas Krey:
> On Thu, 23 May 2013 11:06:57 +0000, Andreas Krey wrote:
> ...
>> ...
>>> Don't do that, then.
>
> Ouch, you're right. The problem is not actually in the
> pull; only the *last* pull into a feature branch that
> then get pushed back ff to master needs to be reversed.
>
> And at that time you don't know it's the last one
> -> swap parents before the push if necessary.

if you have to be so careful to ensure the correct ordering of parents 
it almost defeats the initial objective to make commit graphs in gitk 
look nice without re-educating/restricting other users. A solution that 
works for everyone should work without users having to think about it.

Here is an idea (probably already discussed in the long history of git):
1) the branch name is recorded in a commit (for merges the branch that 
is updated)
2) unique identifier of repository is recorded in commit (optional)
3) simple configurable ordering and/or coloring scheme in gitk based on 
committer,branch name and repo (with wildcards).

With this users could pull and push as often as they like, the main 
branches would always be ordered and straight lines. If instead you 
already do the work to keep your history clean you could just use the 
coloring scheme and see committers color coded in gitk. Further benefit: 
the history of really old commits could be more easily remembered if you 
knew in what branch they originated

Is this a bad idea or just no one did it yet?

Re: first parent, commit graph layout, and pull merge direction

[John Szakmeister]

On Fri, May 24, 2013 at 4:29 AM, John Keeping <john@keeping.me.uk> wrote:
[snip]
> Note that in my email that started this, I tried to be clear that I was
> talking about "git pull" *without a branch name*.  If this user
> explicitly says "git pull remote branch" then I consider that a clear
> indication that they really do mean to perform a merge; I would not
> recommend changing the current behaviour in that case.
>
> If the user just says "git pull" then it is more likely that they are
> just trying to synchronise with the upstream branch, in which case they
> probably don't actually want a merge.

This makes a lot of sense to me.  I was going to write earlier that I
almost wish there was a separate command for getting your local branch
"in sync" with the remote one.

BTW, it also doesn't help that `git pull` is suggested as the answer
anytime a push cannot succeed.  I've warned my users about using `git
pull`, and--unfortunately--they sometimes forget because the advice is
right there in front of them.

I agree with John here: it's a bare `git pull` that is often the
culprit.  Of course, the asymmetry between `git pull` and `git pull
remote branch` is a little bothersome too, but the team does that
*far* less often.

-John

Re: first parent, commit graph layout, and pull merge direction

[Andreas Krey]

On Fri, 24 May 2013 11:29:00 +0000, Holger Hellmuth (IKS) wrote:
...
> Here is an idea (probably already discussed in the long history of git):
> 1) the branch name is recorded in a commit (for merges the branch that 
> is updated)

The branch name is almost completely meaningless. I could just
do my feature in my local master and never have a different name.

Or commit something onto tmp that I then fast-forward into my
(properly named) feature branch.

> 2) unique identifier of repository is recorded in commit (optional)

That is pure noise (in my workflow).

> 3) simple configurable ordering and/or coloring scheme in gitk based on 
> committer,branch name and repo (with wildcards).

Ok, gitk could use some features. :-)

...
> Is this a bad idea or just no one did it yet?

Possibly not bad (hg does parts of it), but un-git-ish?

(I'm not sure that it was *intended* that the parents
of a merge commit have an order, except that they need
to for deterministic hashes.)

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: first parent, commit graph layout, and pull merge direction

[Holger Hellmuth (IKS)]

Am 24.05.2013 15:42, schrieb Andreas Krey:
> On Fri, 24 May 2013 11:29:00 +0000, Holger Hellmuth (IKS) wrote:
> ...
>> Here is an idea (probably already discussed in the long history of git):
>> 1) the branch name is recorded in a commit (for merges the branch that
>> is updated)
>
> The branch name is almost completely meaningless. I could just
> do my feature in my local master and never have a different name.

In which case parent switching in the commit wouldn't help you either.

But even you could keep your master always on the left side of gitk if 
you deem it special. And you could keep longer running cooperative 
branches (the main develop and the release branch of your project for 
example) in a seperate lane.

Depending on your use of branches many branches won't get any ordering, 
but at a minimum important branches can easily be "highlighted".

> Or commit something onto tmp that I then fast-forward into my
> (properly named) feature branch.

Yes, but then you would see a feature branch in its expected column in 
gitk and you would also see (even years later) that it didn't start as a 
feature but later was made into one. Cues like this help to remember 
what happened even if you forgot to mention them in the commit message

>> 2) unique identifier of repository is recorded in commit (optional)
>
> That is pure noise (in my workflow).

It is important to differentiate between branches of the same name in 
different repositories. For example if your project has a central 
repository with master getting all the release stuff you want to sort 
that master differently than your own master.

The unique identifier might be just a random number or string created at 
init time of the repo.

>> 3) simple configurable ordering and/or coloring scheme in gitk based on
>> committer,branch name and repo (with wildcards).
>
> Ok, gitk could use some features. :-)

Without additional information about the commit history gitk can do 
exactly what it does now.

> ...
>> Is this a bad idea or just no one did it yet?
>
> Possibly not bad (hg does parts of it), but un-git-ish?

Don't know. No CVS does branches as good as git. But then it drops that 
information which depending on development style could be useful or not.
Not that useful for people who keep their history clean, a lot for 
people who don't.

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

Felipe Contreras <felipe.contreras@gmail.com> writes:

> ... but I don't see why something small like that
> wouldn't make sense:
>
> The pull was not fast-forward, please either merge or rebase.

OK, I think I got what John was getting at and this single liner
message is a good summary of it.

Instead of telling them "you cannot push this thing without losing
history from the location you are pushing to; you need to become up
to date with respect to them before pushing" upon seeing a non ff
push failure, we can tell them "you cannot update your history to
what the place you get new changes from has without losing your
history; you need to integrate the two".

Initially I said limiting "git pull" to "--ff-only" by default did
not make sense, but when we view it that way, I now see how such a
default makes sense.

In another subthread, John Szakmeister mentioned that the "please
'git pull' first" message that a "push" gives when it stops due to
non-ff nudges the users in a wrong direction, because they often
take that 'git pull' too literally (e.g. 'pull --rebase' may be
necessary in their project, not 'git pull<ENTER>').

The original message deliberately avoided mentioning 'git pull' for
that exact reason, but in mid 2010 we made it worse.  The log of
that change says that it attempted to

    ... remains fuzzy to include "git pull", "git pull --rebase" and
    others, but directs the user to the simplest solution in the
    vast majority of cases.

but this thread shows that it did not work; the simplest solution
was a wrong one.  The message also may need to be rethought to
complement this direction being proposed for "pull".

Re: first parent, commit graph layout, and pull merge direction

[Andreas Krey]

On Thu, 23 May 2013 09:01:15 +0000, Junio C Hamano wrote:
...
> Instead of having a nice "these six commits marked as 'x' were done
> on a branch forked some time ago, to address only this one issue and
> to address it fully" history that explains how these commits were
> related and these commits are the full solution to a single issue:
> 
>       x---x---x---x---x---x
>      /                     \
>  ---o---o---o---o---o---o---M---o---o---...
> 
> they end up with something like this, even with the "flip the heads
> of a merge" option, by pulling too often:
> 
>       x---x   x---x---x   x
>      /     \ /         \ / \
>  ---o---o---M---o---o---M---M---o---o---...

Wouldn't that be (you don't want to put your work back into master before
it's done) the following?

       x---x---M---x---x---M--x
      /       /           /    \
  ---o---o---M---o---o---M--o---M---o---o---...

With a bit of luck the first-parent strands will also run like this.

I know that rebasing topic branches is better than updating, but my
monetary upstream is busy letting go a clearcase-minted mindset.
Teaching them rebasing will take a while, and as long as tthat we
will have the picture above.

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: first parent, commit graph layout, and pull merge direction

[Andreas Krey]

On Fri, 24 May 2013 17:05:26 +0000, Holger Hellmuth (IKS) wrote:
> Am 24.05.2013 15:42, schrieb Andreas Krey:
...
> >The branch name is almost completely meaningless. I could just
> >do my feature in my local master and never have a different name.
> 
> In which case parent switching in the commit wouldn't help you either.

Oh, it does; I tried. Names are meaningless, the parent ordering
isn't. ( [And at least, it's already in there.]

> But even you could keep your master always on the left side of gitk if 
> you deem it special. And you could keep longer running cooperative 
> branches (the main develop and the release branch of your project for 
> example) in a seperate lane.

I need gitk (or similar) to do it. Will take some time to understand
the code (and triggers the 'I can write it (the interesting part) faster
than I can grok gitk').

...
> Without additional information about the commit history gitk can do 
> exactly what it does now.

Most definitely not. There are quite some situations where the graph
deteriorates pretty heavily, even when not expecting it to pay attention
to first parent. When you have two branches, of which one regularly
gets merge into the other, it sometimes manages to display first the
one, then the other branch, with a log of merge edges going upwards
in parallel, for example.

Andreas

-- 
"Totally trivial. Famous last words."
From: Linus Torvalds <torvalds@*.org>
Date: Fri, 22 Jan 2010 07:29:21 -0800

Re: first parent, commit graph layout, and pull merge direction

[Junio C Hamano]

Andreas Krey <a.krey@gmx.de> writes:

> On Thu, 23 May 2013 09:01:15 +0000, Junio C Hamano wrote:
> ...
>> Instead of having a nice "these six commits marked as 'x' were done
>> on a branch forked some time ago, to address only this one issue and
>> to address it fully" history that explains how these commits were
>> related and these commits are the full solution to a single issue:
>> 
>>       x---x---x---x---x---x
>>      /                     \
>>  ---o---o---o---o---o---o---M---o---o---...
>> 
>> they end up with something like this, even with the "flip the heads
>> of a merge" option, by pulling too often:
>> 
>>       x---x   x---x---x   x
>>      /     \ /         \ / \
>>  ---o---o---M---o---o---M---M---o---o---...
>
> Wouldn't that be (you don't want to put your work back into master before
> it's done) the following?
>
>        x---x---M---x---x---M--x
>       /       /           /    \
>   ---o---o---M---o---o---M--o---M---o---o---...

That is what you would get if you "pull from my upstream" with the
current software.

And that is what triggered this discussion thread in which some
people said that they do not want that shape of the history.

At the leftmost merge M you drew on the upper line (i.e. your
topic), the merge pulls in other's commits that are unrelated to
each other as if it were a meaningful group of commits on a side
branch.  They want to see the merge going other way around, pulling
your work done on a side branch, integrating into the mainline.

The second illustration you are commenting on were done to explain
why such a "when pulling from my upstream, I want the order of
parents swapped, so that mainline appears as the first parent" is
not solving the whole issue.  The time series would go more like
this:

(1) While you were working on two 'x's, others have worked to
    advance the mainline:

       x---x  Your 'master'
      /
  ---o---o  Mainline


(2) You cannot push without losing others work, so you pull, but in
    order to avoid the "others work on mixed on a single side
    branch" issue, you use the fictional "flip heads of a merge"
    option, and push the result out.  That becomes the tip of the
    mainline:

       x---x
      /     \
  ---o---o---M

(3) Then you keep working to build more commits on top. 

       x---x   x---x---x
      /     \ /
  ---o---o---M

(4) And others also keep working.

       x---x   x---x---x
      /     \ /
  ---o---o---M---o---o

Re: first parent, commit graph layout, and pull merge direction

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
Sent: Friday, May 24, 2013 5:26 PM
> Felipe Contreras <felipe.contreras@gmail.com> writes:
>
>> ... but I don't see why something small like that
>> wouldn't make sense:
>>
>> The pull was not fast-forward, please either merge or rebase.
>
> OK, I think I got what John was getting at and this single liner
> message is a good summary of it.
>
> Instead of telling them "you cannot push this thing without losing
> history from the location you are pushing to; you need to become up
> to date with respect to them before pushing" upon seeing a non ff
> push failure, we can tell them "you cannot update your history to
> what the place you get new changes from has without losing your
> history; you need to integrate the two".
>
> Initially I said limiting "git pull" to "--ff-only" by default did
> not make sense, but when we view it that way, I now see how such a
> default makes sense.
>
> In another subthread, John Szakmeister mentioned that the "please
> 'git pull' first" message that a "push" gives when it stops due to
> non-ff nudges the users in a wrong direction, because they often
> take that 'git pull' too literally (e.g. 'pull --rebase' may be
> necessary in their project, not 'git pull<ENTER>').
>
> The original message deliberately avoided mentioning 'git pull' for
> that exact reason, but in mid 2010 we made it worse.  The log of
> that change says that it attempted to
>
>    ... remains fuzzy to include "git pull", "git pull --rebase" and
>    others, but directs the user to the simplest solution in the
>    vast majority of cases.
>
> but this thread shows that it did not work; the simplest solution
> was a wrong one.  The message also may need to be rethought to
> complement this direction being proposed for "pull".
> --

Perhaps offer "git pull ....", which suggests that the user should 
consider what pull parameters to provide and if taken literally should 
barf with the four dots.

Philip

[PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

Because letting a trivial merge automatically handled by Git is so
easy with "git pull", a person who is new to Git may not realize
that the project s/he is interacting with may prefer "rebase"
workflow.  Add a safety valve to fail "git pull" that is not a
fast-forward until/unless the user expressed her preference between
the two.

Those who want the existing behaviour could just do

    git config --global pull.rebase false

once, and they'd not even notice.

    http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225326

for a full discussion.

The fallout from this change to test suite is not very pretty, though.

Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This is not a serious inclusion proposal yet, but to see if
   people are still interested in possibly helping new users.

 git-pull.sh                            | 36 +++++++++++++++++++++++++++++++++-
 t/annotate-tests.sh                    |  2 +-
 t/t4013-diff-various.sh                |  2 ++
 t/t4200-rerere.sh                      |  2 ++
 t/t5500-fetch-pack.sh                  |  6 +++++-
 t/t5521-pull-options.sh                |  2 ++
 t/t5524-pull-msg.sh                    |  2 +-
 t/t5700-clone-reference.sh             |  4 ++--
 t/t6022-merge-rename.sh                |  2 ++
 t/t6026-merge-attr.sh                  |  2 +-
 t/t6029-merge-subtree.sh               |  1 +
 t/t6037-merge-ours-theirs.sh           |  2 ++
 t/t9114-git-svn-dcommit-merge.sh       |  2 +-
 t/t9400-git-cvsserver-server.sh        |  2 +-
 t/t9500-gitweb-standalone-no-errors.sh |  2 +-
 15 files changed, 59 insertions(+), 10 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 638aabb..4a6a863 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -41,13 +41,21 @@ test -f "$GIT_DIR/MERGE_HEAD" && die_merge
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
 log_arg= verbosity= progress= recurse_submodules= verify_signatures=
 merge_args= edit=
+
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
+
+# See if we are configured to rebase by default.
+# The value $rebase is, throughout the main part of the code:
+#    (empty) - the user did not have any preference
+#    true    - the user told us to integrate by rebasing
+#    flase   - the user told us to integrate by merging
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
 if test -z "$rebase"
 then
 	rebase=$(git config --bool pull.rebase)
 fi
+
 dry_run=
 while :
 do
@@ -113,7 +121,8 @@ do
 	-r|--r|--re|--reb|--reba|--rebas|--rebase)
 		rebase=true
 		;;
-	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
+	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase|\
+	-m|--m|--me|--mer|--merg|--merge)
 		rebase=false
 		;;
 	--recurse-submodules)
@@ -219,6 +228,7 @@ test true = "$rebase" && {
 		fi
 	done
 }
+
 orig_head=$(git rev-parse -q --verify HEAD)
 git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1
 test -z "$dry_run" || exit 0
@@ -264,6 +274,30 @@ case "$merge_head" in
 		die "$(gettext "Cannot rebase onto multiple branches")"
 	fi
 	;;
+*)
+	# integrating with a single other history
+	merge_head=${merge_head% }
+	if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" &&
+		test -n "$orig_head" &&
+		! $(git merge-base --is-ancestor "$orig_head" "$merge_head")
+	then
+echo >&2 "orig-head was $orig_head"
+echo >&2 "merge-head is $merge_head"
+git show >&2 --oneline -s "$orig_head" "$merge_head"
+
+		die "The pull does not fast-forward; please specify
+if you want to merge or rebase.
+
+Use either
+
+    git pull --rebase
+    git pull --merge
+
+You can also use 'git config pull.rebase true' (if you want --rebase) or
+'git config pull.rebase false' (if you want --merge) to set this once for
+this project and forget about it."
+	fi
+	;;
 esac
 
 if test -z "$orig_head"
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index c56a77d..af02c6d 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -79,7 +79,7 @@ test_expect_success \
 
 test_expect_success \
     'merge-setup part 3' \
-    'git pull . branch1'
+    'git pull --merge . branch1'
 
 test_expect_success \
     'Two lines blamed on A, one on B, two on B1, one on B2' \
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index e77c09c..1ee2198 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -12,6 +12,8 @@ LF='
 
 test_expect_success setup '
 
+	git config pull.rebase false &&
+
 	GIT_AUTHOR_DATE="2006-06-26 00:00:00 +0000" &&
 	GIT_COMMITTER_DATE="2006-06-26 00:00:00 +0000" &&
 	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 7f6666f..0563357 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -25,6 +25,8 @@ test_description='git rerere
 . ./test-lib.sh
 
 test_expect_success 'setup' '
+	git config pull.rebase false &&
+
 	cat >a1 <<-\EOF &&
 	Some title
 	==========
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index fd2598e..4be8877 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -143,7 +143,11 @@ test_expect_success 'clone shallow depth 1 with fsck' '
 '
 
 test_expect_success 'clone shallow' '
-	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow
+	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow &&
+	(
+		cd shallow &&
+		git config pull.rebase false
+	)
 '
 
 test_expect_success 'clone shallow depth count' '
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index 453aba5..d821fab 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -91,6 +91,8 @@ test_expect_success 'git pull --force' '
 	[branch "master"]
 		remote = two
 		merge = refs/heads/master
+	[pull]
+		rebase = false
 	EOF
 	git pull two &&
 	test_commit A &&
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index 8cccecc..660714b 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 test_expect_success pull '
 (
 	cd cloned &&
-	git pull --log &&
+	git pull --log --merge &&
 	git log -2 &&
 	git cat-file commit HEAD >result &&
 	grep Dollar result
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 6537911..306badf 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -94,7 +94,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling changes from origin' \
 'cd C &&
-git pull origin'
+git pull --merge origin'
 
 cd "$base_dir"
 
@@ -109,7 +109,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling changes from origin' \
 'cd D &&
-git pull origin'
+git pull --merge origin'
 
 cd "$base_dir"
 
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index c680f78..e12d90b 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -10,6 +10,8 @@ modify () {
 
 test_expect_success setup \
 '
+git config pull.rebase false &&
+
 cat >A <<\EOF &&
 a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 5e43997..5428f19 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -172,7 +172,7 @@ test_expect_success 'up-to-date merge without common ancestor' '
 	test_tick &&
 	(
 		cd repo1 &&
-		git pull ../repo2 master
+		git pull --merge ../repo2 master
 	)
 '
 
diff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh
index 73fc240..3ca29c4 100755
--- a/t/t6029-merge-subtree.sh
+++ b/t/t6029-merge-subtree.sh
@@ -41,6 +41,7 @@ test_expect_success 'setup' '
 	mkdir git &&
 	cd git &&
 	git init &&
+	git config pull.rebase false &&
 	echo git >git.c &&
 	o2=$(git hash-object git.c) &&
 	git add git.c &&
diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh
index 3889eca..41bf060 100755
--- a/t/t6037-merge-ours-theirs.sh
+++ b/t/t6037-merge-ours-theirs.sh
@@ -66,6 +66,8 @@ test_expect_success 'binary file with -Xours/-Xtheirs' '
 '
 
 test_expect_success 'pull passes -X to underlying merge' '
+	git config pull.rebase false &&
+
 	git reset --hard master && git pull -s recursive -Xours . side &&
 	git reset --hard master && git pull -s recursive -X ours . side &&
 	git reset --hard master && git pull -s recursive -Xtheirs . side &&
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index f524d2f..dfce024 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' '
 	echo friend > README &&
 	cat tmp >> README &&
 	git commit -a -m "friend" &&
-	git pull . merge
+	git pull --merge . merge
 	'
 
 test_debug 'gitk --all & sleep 1'
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 0431386..76b8640 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -46,7 +46,7 @@ test_expect_success 'setup' '
   touch secondrootfile &&
   git add secondrootfile &&
   git commit -m "second root") &&
-  git pull secondroot master &&
+  git pull --merge secondroot master &&
   git clone -q --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
   GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true &&
   GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" &&
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 6fca193..787c6cc 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -328,7 +328,7 @@ test_expect_success \
 	 git add b &&
 	 git commit -a -m "On branch" &&
 	 git checkout master &&
-	 git pull . b &&
+	 git pull --merge . b &&
 	 git tag merge_commit'
 
 test_expect_success \
-- 
1.8.3.1-794-ga13ccd6

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
> Because letting a trivial merge automatically handled by Git is so
> easy with "git pull", a person who is new to Git may not realize
> that the project s/he is interacting with may prefer "rebase"
> workflow.

Or they may not even realize that they've just merged an unrelated
branch at all, dragging in a thousand unrelated commits which they
accidentally push to a central repository without looking,
contaminating future branches based on the central repostitory without
drastic rebase surgery ;).  I just saw one of these earlier this week.

>  * This is not a serious inclusion proposal yet, but to see if
>    people are still interested in possibly helping new users.

What needs to happen to make it serious?  I'm happy to help.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Fredrik Gustafsson]

On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
<snip>
> +# See if we are configured to rebase by default.
> +# The value $rebase is, throughout the main part of the code:
> +#    (empty) - the user did not have any preference
> +#    true    - the user told us to integrate by rebasing
> +#    flase   - the user told us to integrate by merging

s/flase/false

And isn't all config settings documented somewhere?

-- 
Med vänliga hälsningar
Fredrik Gustafsson

tel: 0733-608274
e-post: iveqy@iveqy.com

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 602 bytes --]

Assorted minor edits:

On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
> Because letting a trivial merge automatically handled by Git is so

Maybe:

  Because letting Git handle a trivial merge automatically is so…

> that the project s/he is interacting with may prefer "rebase"
> workflow.  Add a safety valve to fail "git pull" that is not a

Maybe (adding an "a"):

  a "rebase" workflow.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

Fredrik Gustafsson <iveqy@iveqy.com> writes:

> On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
> <snip>
>> +# See if we are configured to rebase by default.
>> +# The value $rebase is, throughout the main part of the code:
>> +#    (empty) - the user did not have any preference
>> +#    true    - the user told us to integrate by rebasing
>> +#    flase   - the user told us to integrate by merging
>
> s/flase/false

Thanks.

> And isn't all config settings documented somewhere?

Yes, but the above does not have anything to do with it.  It is how
the variable in this script gets used---it may come from the config,
but it will be overridden by command line option.

If you do "git config pull.rebase false", it is an explicit show of
preference to do "pull --merge".

If you do not have any pull.rebase in your configuration, *and* if
your command line does not say "pull --merge" nor "pull --rebase",
then $rebase will be empty, and that is how we detect that you
haven't given us any explicit preference (yet) and fail the command.

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

"W. Trevor King" <wking@tremily.us> writes:

> Assorted minor edits:
>
> On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
>> Because letting a trivial merge automatically handled by Git is so
>
> Maybe:
>
>   Because letting Git handle a trivial merge automatically is so…
>
>> that the project s/he is interacting with may prefer "rebase"
>> workflow.  Add a safety valve to fail "git pull" that is not a
>
> Maybe (adding an "a"):
>
>   a "rebase" workflow.

Thanks.

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

"W. Trevor King" <wking@tremily.us> writes:

> On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
>> Because letting a trivial merge automatically handled by Git is so
>> easy with "git pull", a person who is new to Git may not realize
>> that the project s/he is interacting with may prefer "rebase"
>> workflow.
>
> Or they may not even realize that they've just merged an unrelated
> branch at all, dragging in a thousand unrelated commits which they
> accidentally push to a central repository without looking,
> contaminating future branches based on the central repostitory without
> drastic rebase surgery ;).  I just saw one of these earlier this week.

I am not sure "running pull and integrate other's work in random
branches" is something the proposed (not by me) change would help to
prevent from happening.

Your "accident user" could have just been on a 'maint' branch,
pulled the 'master' branch which would fast-forward and then pushed
the result back to 'maint', contaminating the shared 'maint' branch
with commits that do not match the purpose of it, which is to hold
only fixes without enhancements.

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Matthieu Moy]

Junio C Hamano <gitster@pobox.com> writes:

> Because letting a trivial merge automatically handled by Git is so
> easy with "git pull", a person who is new to Git may not realize
> that the project s/he is interacting with may prefer "rebase"
> workflow.  Add a safety valve to fail "git pull" that is not a
> fast-forward until/unless the user expressed her preference between
> the two.

IMHO, that would be terrible for beginners.

My experience with many beginners/students is: they run "git pull" to
get changes from their co-workers, don't read the messages. When there's
no conflict, it's OK, Git creates the merge commit and they continue
working. When there are conflicts, they fix it (or not), and forget to
commit, continue working, and commit when they really need to, later.
That's bad: mixing merges with actual changes is terrible. But that
works. And that's a very common mistake in my experience :-(.

Now, give the same user as above "git pull --rebase". rebase may stop
because of conflicts, the user may fix it, but then if the user
continues working, he's on a detached HEAD with a rebase ongoing. Some
of the changes went away, they may come back one day if the user runs
"git rebase --continue".

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 503 bytes --]

On Thu, Jun 27, 2013 at 02:20:42PM -0700, Junio C Hamano wrote:
> Your "accident user" could have just been on a 'maint' branch,
> [snip]

By the time I talk people into using a 'maint' branch, we'll probably
have already passed the 'accidental pull and push' stage ;).  This
will certainly reduce the risk in any case.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1121 bytes --]

On Fri, Jun 28, 2013 at 12:16:53AM +0200, Matthieu Moy wrote:
> IMHO, that would be terrible for beginners.
> 
> My experience with many beginners/students is: they run "git pull" to
> get changes from their co-workers, don't read the messages.

I admit that I'd be happy with a config option that just disabled pull
entirely (forcing people to fetch/merge explicitly) to avoid this type
of beginner mistake.  With an unconfigured pull.rebase, this patch
does that for merge/rebase cases, while still letting folks pull when
it's a clean fast forward (usually ok).

I'd also be happy with an opt-in disable.  The real solution would be
to talk my group out of using a central shared repository or into
using pull-free feature branches, but I don't see either on my
horizon.  Git doesn't need to change to mitigate sloppy-shared-repo
problems, but having some sort of anti-pull configuration option would
certainly help me out.

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Matthieu Moy]

"W. Trevor King" <wking@tremily.us> writes:

> On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
>> Because letting a trivial merge automatically handled by Git is so
>> easy with "git pull", a person who is new to Git may not realize
>> that the project s/he is interacting with may prefer "rebase"
>> workflow.
>
> Or they may not even realize that they've just merged an unrelated
> branch at all, dragging in a thousand unrelated commits which they
> accidentally push to a central repository without looking,
> contaminating future branches based on the central repostitory without
> drastic rebase surgery ;).  I just saw one of these earlier this week.

I don't understand how the change would solve this. If "pull" would drag
a lot of commits in the current branch, the "rebase" will rebase the
current branch on a totally different history, and pushing the result
would be equally bad.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[John Keeping]

On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
> Because letting a trivial merge automatically handled by Git is so
> easy with "git pull", a person who is new to Git may not realize
> that the project s/he is interacting with may prefer "rebase"
> workflow.  Add a safety valve to fail "git pull" that is not a
> fast-forward until/unless the user expressed her preference between
> the two.
> 
> Those who want the existing behaviour could just do
> 
>     git config --global pull.rebase false
> 
> once, and they'd not even notice.
> 
>     http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225326
> 
> for a full discussion.
> 
> The fallout from this change to test suite is not very pretty, though.
> 
> Signed-off-by: Junio C Hamano <gitster@pobox.com>
> ---
[snip]
> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
> index c56a77d..af02c6d 100644
> --- a/t/annotate-tests.sh
> +++ b/t/annotate-tests.sh
> @@ -79,7 +79,7 @@ test_expect_success \
>  
>  test_expect_success \
>      'merge-setup part 3' \
> -    'git pull . branch1'
> +    'git pull --merge . branch1'

I think the "--merge" should be implied here because the suer has
specified an explicit remote and branch.  Similarly, if "--ff",
"--no-ff" or "--ff-only" are given then we can infer "--merge" in the
absence of any other configuration.

However, when I looked at doing this I decided that it would be
difficult to get that ideal behaviour without rewriting git-pull as a
builtin.

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1557 bytes --]

On Fri, Jun 28, 2013 at 08:34:53AM +0200, Matthieu Moy wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> > On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
> >> Because letting a trivial merge automatically handled by Git is so
> >> easy with "git pull", a person who is new to Git may not realize
> >> that the project s/he is interacting with may prefer "rebase"
> >> workflow.
> >
> > Or they may not even realize that they've just merged an unrelated
> > branch at all, dragging in a thousand unrelated commits which they
> > accidentally push to a central repository without looking,
> > contaminating future branches based on the central repostitory without
> > drastic rebase surgery ;).  I just saw one of these earlier this week.
> 
> I don't understand how the change would solve this. If "pull" would drag
> a lot of commits in the current branch, the "rebase" will rebase the
> current branch on a totally different history, and pushing the result
> would be equally bad.

I want the warning that they had not made the required config choice
between rebase/merge needed to handle a non-ff case, not the default
merge (or rebase) behavior.  The warning gives them a chance to
realize that this was not an appropriate time for a `svn update`
analog, and that the project may not to want to have the branches
joined at all ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Matthieu Moy]

"W. Trevor King" <wking@tremily.us> writes:

> On Fri, Jun 28, 2013 at 08:34:53AM +0200, Matthieu Moy wrote:
>> "W. Trevor King" <wking@tremily.us> writes:
>>
>> > Or they may not even realize that they've just merged an unrelated
>> > branch at all, dragging in a thousand unrelated commits which they
>> > accidentally push to a central repository without looking,
>> > contaminating future branches based on the central repostitory without
>> > drastic rebase surgery ;).  I just saw one of these earlier this week.
>> 
>> I don't understand how the change would solve this. If "pull" would drag
>> a lot of commits in the current branch, the "rebase" will rebase the
>> current branch on a totally different history, and pushing the result
>> would be equally bad.
>
> I want the warning that they had not made the required config choice
> between rebase/merge needed to handle a non-ff case, not the default
> merge (or rebase) behavior.  The warning gives them a chance to
> realize that this was not an appropriate time for a `svn update`
> analog, and that the project may not to want to have the branches
> joined at all ;).

You're assuming that the config is not made, but this is supposed to
happen once initially. Then, the user will chose either merge or rebase,
and whatever is chosen, the result will be bad.

-- 
Matthieu Moy
http://www-verimag.imag.fr/~moy/

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[W. Trevor King]

[-- Attachment #1: Type: text/plain, Size: 1869 bytes --]

On Fri, Jun 28, 2013 at 01:52:38PM +0200, Matthieu Moy wrote:
> "W. Trevor King" <wking@tremily.us> writes:
> > I want the warning that they had not made the required config choice
> > between rebase/merge needed to handle a non-ff case, not the default
> > merge (or rebase) behavior.  The warning gives them a chance to
> > realize that this was not an appropriate time for a `svn update`
> > analog, and that the project may not to want to have the branches
> > joined at all ;).
> 
> You're assuming that the config is not made, but this is supposed to
> happen once initially. Then, the user will chose either merge or rebase,
> and whatever is chosen, the result will be bad.

I'm hoping that reading the error message reminds them that these
cross-branch pulls are not recommended (for us), and that they skip
the configuration step (so they'll get the same warning after their
next subconcious pull).  Of course, there are no guarantees.  But if
they do configure their rebase/merge preference and make and push bad
merge, at least I'll have something I can suggest as a finger-breaker.

Of course, they should already be seeing their editor with a merge
commit message that they are ok-ing.  If that's not enough to make
them think twice, a warning that:

  The pull does not fast-forward; …

may fall on deaf ears (blind eyes?).  However, for folks used to only
having a single branch, this may be enough of a jolt to wake them up.

I'm not making a very strong case, and this whole line of reasoning is
getting off topic for this PR.  Unless we adapt it to:

  pull.non-ff = {merge,rebase,never}

which is, I think, even less likely to land ;).

Cheers,
Trevor

-- 
This email may be signed or encrypted with GnuPG (http://www.gnupg.org).
For more information, see http://en.wikipedia.org/wiki/Pretty_Good_Privacy

[-- Attachment #2: OpenPGP digital signature --]
[-- Type: application/pgp-signature, Size: 836 bytes --]

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

>>  test_expect_success \
>>      'merge-setup part 3' \
>> -    'git pull . branch1'
>> +    'git pull --merge . branch1'
>
> I think the "--merge" should be implied here because the suer has
> specified an explicit remote and branch.

The whole point of the topic is "It used to be that when you said
'git pull' and did not tell us your preferred way to integrate your
work and work by the others', we default to merging, but we no
longer do so---you have to choose."

Here, "git pull . branch1" is merely saying "I want to integrate
the work on my current branch with that of branch1" without saying
how that integration wants to happen.

Even though, as an old timer, I find it mildly irritating that we
now have to be explicit in these tests, this change is in line with
the spirit of the topic.  If we didn't have to change this example
and the pull silently succeeded without complaining, we achieved
nothing.

>  Similarly, if "--ff",
> "--no-ff" or "--ff-only" are given then we can infer "--merge" in the
> absence of any other configuration.

Isn't that already there in the patch to git-merge?

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[John Keeping]

On Fri, Jun 28, 2013 at 10:22:57AM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> >>  test_expect_success \
> >>      'merge-setup part 3' \
> >> -    'git pull . branch1'
> >> +    'git pull --merge . branch1'
> >
> > I think the "--merge" should be implied here because the suer has
> > specified an explicit remote and branch.
> 
> The whole point of the topic is "It used to be that when you said
> 'git pull' and did not tell us your preferred way to integrate your
> work and work by the others', we default to merging, but we no
> longer do so---you have to choose."
> 
> Here, "git pull . branch1" is merely saying "I want to integrate
> the work on my current branch with that of branch1" without saying
> how that integration wants to happen.

The change that I think is important is that the "bring my branch
up-to-date" operation should force the user to choose what to do if the
branch does not fast-forward to its upstream.  If that was spelled "git
update" then having "git pull" perform a merge would be fine, but we
spell this operation as "git pull" so the change needs to happen there.

I don't think "git pull remote branch" falls into the same category as
plain "git pull" so I'm not convinced that defaulting to merge there is
unreasonable.  The original message about this [1] did talk about only
"git pull" with no arguments.

> Even though, as an old timer, I find it mildly irritating that we
> now have to be explicit in these tests, this change is in line with
> the spirit of the topic.  If we didn't have to change this example
> and the pull silently succeeded without complaining, we achieved
> nothing.

I disagree that we would have achieved nothing.  New users will not be
using explicit arguments to git-pull when just trying to bring a branch
up-to-date.

[1] http://article.gmane.org/gmane.comp.version-control.git/225240

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

>> Here, "git pull . branch1" is merely saying "I want to integrate
>> the work on my current branch with that of branch1" without saying
>> how that integration wants to happen.
>
> The change that I think is important is that the "bring my branch
> up-to-date" operation should force the user to choose what to do if the
> branch does not fast-forward to its upstream.  If that was spelled "git
> update" then having "git pull" perform a merge would be fine, but we
> spell this operation as "git pull" so the change needs to happen there.

I am not sure I quite get what you want to say with "git update",
and I am not sure if I necessarily want to know---I do not think we
would want to add yet another command that DWIMs for certain _I_,
that may not match newbie expectations.

> I don't think "git pull remote branch" falls into the same category as
> plain "git pull" so I'm not convinced that defaulting to merge there is
> unreasonable.  The original message about this [1] did talk about only
> "git pull" with no arguments.

If you want to limit the scope to only "git pull" (without any
command line argument), I actually do not have strong preference for
or against it either way.  Perhaps a follow-up patch to be squashed?

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[John Keeping]

On Fri, Jun 28, 2013 at 03:41:34PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> 
> >> Here, "git pull . branch1" is merely saying "I want to integrate
> >> the work on my current branch with that of branch1" without saying
> >> how that integration wants to happen.
> >
> > The change that I think is important is that the "bring my branch
> > up-to-date" operation should force the user to choose what to do if the
> > branch does not fast-forward to its upstream.  If that was spelled "git
> > update" then having "git pull" perform a merge would be fine, but we
> > spell this operation as "git pull" so the change needs to happen there.
> 
> I am not sure I quite get what you want to say with "git update",
> and I am not sure if I necessarily want to know---I do not think we
> would want to add yet another command that DWIMs for certain _I_,
> that may not match newbie expectations.

I wasn't proposing any new command, I was trying to express the
operation that users coming from non-distributed VCSs want to perform
(which is called "update" in svn).  The problem is that a DVCS operates
in a completely different way and a lot of users do not seem to want to
learn the difference but simply try to map the existing commands that
they know onto Git commands ([1] is the top result for "svn commands to
git" on Google and maps "svn update" straight to "git pull").

[1] http://git.or.cz/course/svn.html

> > I don't think "git pull remote branch" falls into the same category as
> > plain "git pull" so I'm not convinced that defaulting to merge there is
> > unreasonable.  The original message about this [1] did talk about only
> > "git pull" with no arguments.
> 
> If you want to limit the scope to only "git pull" (without any
> command line argument), I actually do not have strong preference for
> or against it either way.  Perhaps a follow-up patch to be squashed?

I remember looking at this a few weeks ago and being concerned that it's
impossible to tell what options you actually have in git-pull because it
just invokes 'git fetch "$@"' and git-pull(1) does advertise a number of
fetch options.  It may be that "test $# = 0" is good enough, but ideally
I want to test for non-option arguments.

I can't see a way of doing this without putting knowledge of all of the
fetch options in git-pull so that we can handle options with arguments
correctly.

[PATCH] fixup! pull: require choice between rebase/merge on non-fast-forward pull

[John Keeping]

---
On Fri, Jun 28, 2013 at 03:41:34PM -0700, Junio C Hamano wrote:
> John Keeping <john@keeping.me.uk> writes:
> > I don't think "git pull remote branch" falls into the same category as
> > plain "git pull" so I'm not convinced that defaulting to merge there is
> > unreasonable.  The original message about this [1] did talk about only
> > "git pull" with no arguments.
> 
> If you want to limit the scope to only "git pull" (without any
> command line argument), I actually do not have strong preference for
> or against it either way.  Perhaps a follow-up patch to be squashed?

Here is that patch.  The test changes here are all reverting changes in
ae2dab2 (pull: require choice between rebase/merge on non-fast-forward
pull, 2013-06-27) - with this change to git-pull.sh the only change
needed in the tests is in t5524-pull-msg:

    $ git diff ae2dab2^ -- t
    diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
    index 8cccecc..660714b 100755
    --- a/t/t5524-pull-msg.sh
    +++ b/t/t5524-pull-msg.sh
    @@ -25,7 +25,7 @@ test_expect_success setup '
     test_expect_success pull '
     (
            cd cloned &&
    -       git pull --log &&
    +       git pull --log --merge &&
            git log -2 &&
            git cat-file commit HEAD >result &&
            grep Dollar result

 git-pull.sh                            | 1 +
 t/annotate-tests.sh                    | 2 +-
 t/t4013-diff-various.sh                | 2 --
 t/t4200-rerere.sh                      | 2 --
 t/t5500-fetch-pack.sh                  | 6 +-----
 t/t5521-pull-options.sh                | 2 --
 t/t5700-clone-reference.sh             | 4 ++--
 t/t6022-merge-rename.sh                | 2 --
 t/t6026-merge-attr.sh                  | 2 +-
 t/t6029-merge-subtree.sh               | 1 -
 t/t6037-merge-ours-theirs.sh           | 2 --
 t/t9114-git-svn-dcommit-merge.sh       | 2 +-
 t/t9400-git-cvsserver-server.sh        | 2 +-
 t/t9500-gitweb-standalone-no-errors.sh | 2 +-
 14 files changed, 9 insertions(+), 23 deletions(-)

diff --git a/git-pull.sh b/git-pull.sh
index 5ce67f9..0ff4a98 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -279,6 +279,7 @@ case "$merge_head" in
 	merge_head=${merge_head% }
 	if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" &&
 		test -n "$orig_head" &&
+		test $# = 0 &&
 		! $(git merge-base --is-ancestor "$orig_head" "$merge_head")
 	then
 echo >&2 "orig-head was $orig_head"
diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
index af02c6d..c56a77d 100644
--- a/t/annotate-tests.sh
+++ b/t/annotate-tests.sh
@@ -79,7 +79,7 @@ test_expect_success \
 
 test_expect_success \
     'merge-setup part 3' \
-    'git pull --merge . branch1'
+    'git pull . branch1'
 
 test_expect_success \
     'Two lines blamed on A, one on B, two on B1, one on B2' \
diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
index 1ee2198..e77c09c 100755
--- a/t/t4013-diff-various.sh
+++ b/t/t4013-diff-various.sh
@@ -12,8 +12,6 @@ LF='
 
 test_expect_success setup '
 
-	git config pull.rebase false &&
-
 	GIT_AUTHOR_DATE="2006-06-26 00:00:00 +0000" &&
 	GIT_COMMITTER_DATE="2006-06-26 00:00:00 +0000" &&
 	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&
diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
index 0563357..7f6666f 100755
--- a/t/t4200-rerere.sh
+++ b/t/t4200-rerere.sh
@@ -25,8 +25,6 @@ test_description='git rerere
 . ./test-lib.sh
 
 test_expect_success 'setup' '
-	git config pull.rebase false &&
-
 	cat >a1 <<-\EOF &&
 	Some title
 	==========
diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
index 4be8877..fd2598e 100755
--- a/t/t5500-fetch-pack.sh
+++ b/t/t5500-fetch-pack.sh
@@ -143,11 +143,7 @@ test_expect_success 'clone shallow depth 1 with fsck' '
 '
 
 test_expect_success 'clone shallow' '
-	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow &&
-	(
-		cd shallow &&
-		git config pull.rebase false
-	)
+	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow
 '
 
 test_expect_success 'clone shallow depth count' '
diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
index d821fab..453aba5 100755
--- a/t/t5521-pull-options.sh
+++ b/t/t5521-pull-options.sh
@@ -91,8 +91,6 @@ test_expect_success 'git pull --force' '
 	[branch "master"]
 		remote = two
 		merge = refs/heads/master
-	[pull]
-		rebase = false
 	EOF
 	git pull two &&
 	test_commit A &&
diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
index 306badf..6537911 100755
--- a/t/t5700-clone-reference.sh
+++ b/t/t5700-clone-reference.sh
@@ -94,7 +94,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling changes from origin' \
 'cd C &&
-git pull --merge origin'
+git pull origin'
 
 cd "$base_dir"
 
@@ -109,7 +109,7 @@ cd "$base_dir"
 
 test_expect_success 'pulling changes from origin' \
 'cd D &&
-git pull --merge origin'
+git pull origin'
 
 cd "$base_dir"
 
diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
index e12d90b..c680f78 100755
--- a/t/t6022-merge-rename.sh
+++ b/t/t6022-merge-rename.sh
@@ -10,8 +10,6 @@ modify () {
 
 test_expect_success setup \
 '
-git config pull.rebase false &&
-
 cat >A <<\EOF &&
 a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
 b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
index 5428f19..5e43997 100755
--- a/t/t6026-merge-attr.sh
+++ b/t/t6026-merge-attr.sh
@@ -172,7 +172,7 @@ test_expect_success 'up-to-date merge without common ancestor' '
 	test_tick &&
 	(
 		cd repo1 &&
-		git pull --merge ../repo2 master
+		git pull ../repo2 master
 	)
 '
 
diff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh
index 3ca29c4..73fc240 100755
--- a/t/t6029-merge-subtree.sh
+++ b/t/t6029-merge-subtree.sh
@@ -41,7 +41,6 @@ test_expect_success 'setup' '
 	mkdir git &&
 	cd git &&
 	git init &&
-	git config pull.rebase false &&
 	echo git >git.c &&
 	o2=$(git hash-object git.c) &&
 	git add git.c &&
diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh
index 41bf060..3889eca 100755
--- a/t/t6037-merge-ours-theirs.sh
+++ b/t/t6037-merge-ours-theirs.sh
@@ -66,8 +66,6 @@ test_expect_success 'binary file with -Xours/-Xtheirs' '
 '
 
 test_expect_success 'pull passes -X to underlying merge' '
-	git config pull.rebase false &&
-
 	git reset --hard master && git pull -s recursive -Xours . side &&
 	git reset --hard master && git pull -s recursive -X ours . side &&
 	git reset --hard master && git pull -s recursive -Xtheirs . side &&
diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
index dfce024..f524d2f 100755
--- a/t/t9114-git-svn-dcommit-merge.sh
+++ b/t/t9114-git-svn-dcommit-merge.sh
@@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' '
 	echo friend > README &&
 	cat tmp >> README &&
 	git commit -a -m "friend" &&
-	git pull --merge . merge
+	git pull . merge
 	'
 
 test_debug 'gitk --all & sleep 1'
diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
index 76b8640..0431386 100755
--- a/t/t9400-git-cvsserver-server.sh
+++ b/t/t9400-git-cvsserver-server.sh
@@ -46,7 +46,7 @@ test_expect_success 'setup' '
   touch secondrootfile &&
   git add secondrootfile &&
   git commit -m "second root") &&
-  git pull --merge secondroot master &&
+  git pull secondroot master &&
   git clone -q --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
   GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true &&
   GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" &&
diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
index 787c6cc..6fca193 100755
--- a/t/t9500-gitweb-standalone-no-errors.sh
+++ b/t/t9500-gitweb-standalone-no-errors.sh
@@ -328,7 +328,7 @@ test_expect_success \
 	 git add b &&
 	 git commit -a -m "On branch" &&
 	 git checkout master &&
-	 git pull --merge . b &&
+	 git pull . b &&
 	 git tag merge_commit'
 
 test_expect_success \
-- 
1.8.3.2.922.gc9d4734

Re: [PATCH] fixup! pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

John Keeping <john@keeping.me.uk> writes:

> Here is that patch.  The test changes here are all reverting changes in
> ae2dab2 (pull: require choice between rebase/merge on non-fast-forward
> pull, 2013-06-27) - with this change to git-pull.sh the only change
> needed in the tests is in t5524-pull-msg:
>
>     $ git diff ae2dab2^ -- t
>     diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
>     index 8cccecc..660714b 100755
>     --- a/t/t5524-pull-msg.sh
>     +++ b/t/t5524-pull-msg.sh
>     @@ -25,7 +25,7 @@ test_expect_success setup '
>      test_expect_success pull '
>      (
>             cd cloned &&
>     -       git pull --log &&
>     +       git pull --log --merge &&

Nice.  "--log" is not a pass-thru option to "fetch" and here it does
not count as part of "$@", so we do require the user to say how the
integration should happen, between --merge and --rebase.

Thanks, will queue.

>             git log -2 &&
>             git cat-file commit HEAD >result &&
>             grep Dollar result
>
>  git-pull.sh                            | 1 +
>  t/annotate-tests.sh                    | 2 +-
>  t/t4013-diff-various.sh                | 2 --
>  t/t4200-rerere.sh                      | 2 --
>  t/t5500-fetch-pack.sh                  | 6 +-----
>  t/t5521-pull-options.sh                | 2 --
>  t/t5700-clone-reference.sh             | 4 ++--
>  t/t6022-merge-rename.sh                | 2 --
>  t/t6026-merge-attr.sh                  | 2 +-
>  t/t6029-merge-subtree.sh               | 1 -
>  t/t6037-merge-ours-theirs.sh           | 2 --
>  t/t9114-git-svn-dcommit-merge.sh       | 2 +-
>  t/t9400-git-cvsserver-server.sh        | 2 +-
>  t/t9500-gitweb-standalone-no-errors.sh | 2 +-
>  14 files changed, 9 insertions(+), 23 deletions(-)
>
> diff --git a/git-pull.sh b/git-pull.sh
> index 5ce67f9..0ff4a98 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -279,6 +279,7 @@ case "$merge_head" in
>  	merge_head=${merge_head% }
>  	if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" &&
>  		test -n "$orig_head" &&
> +		test $# = 0 &&
>  		! $(git merge-base --is-ancestor "$orig_head" "$merge_head")
>  	then
>  echo >&2 "orig-head was $orig_head"
> diff --git a/t/annotate-tests.sh b/t/annotate-tests.sh
> index af02c6d..c56a77d 100644
> --- a/t/annotate-tests.sh
> +++ b/t/annotate-tests.sh
> @@ -79,7 +79,7 @@ test_expect_success \
>  
>  test_expect_success \
>      'merge-setup part 3' \
> -    'git pull --merge . branch1'
> +    'git pull . branch1'
>  
>  test_expect_success \
>      'Two lines blamed on A, one on B, two on B1, one on B2' \
> diff --git a/t/t4013-diff-various.sh b/t/t4013-diff-various.sh
> index 1ee2198..e77c09c 100755
> --- a/t/t4013-diff-various.sh
> +++ b/t/t4013-diff-various.sh
> @@ -12,8 +12,6 @@ LF='
>  
>  test_expect_success setup '
>  
> -	git config pull.rebase false &&
> -
>  	GIT_AUTHOR_DATE="2006-06-26 00:00:00 +0000" &&
>  	GIT_COMMITTER_DATE="2006-06-26 00:00:00 +0000" &&
>  	export GIT_AUTHOR_DATE GIT_COMMITTER_DATE &&
> diff --git a/t/t4200-rerere.sh b/t/t4200-rerere.sh
> index 0563357..7f6666f 100755
> --- a/t/t4200-rerere.sh
> +++ b/t/t4200-rerere.sh
> @@ -25,8 +25,6 @@ test_description='git rerere
>  . ./test-lib.sh
>  
>  test_expect_success 'setup' '
> -	git config pull.rebase false &&
> -
>  	cat >a1 <<-\EOF &&
>  	Some title
>  	==========
> diff --git a/t/t5500-fetch-pack.sh b/t/t5500-fetch-pack.sh
> index 4be8877..fd2598e 100755
> --- a/t/t5500-fetch-pack.sh
> +++ b/t/t5500-fetch-pack.sh
> @@ -143,11 +143,7 @@ test_expect_success 'clone shallow depth 1 with fsck' '
>  '
>  
>  test_expect_success 'clone shallow' '
> -	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow &&
> -	(
> -		cd shallow &&
> -		git config pull.rebase false
> -	)
> +	git clone --no-single-branch --depth 2 "file://$(pwd)/." shallow
>  '
>  
>  test_expect_success 'clone shallow depth count' '
> diff --git a/t/t5521-pull-options.sh b/t/t5521-pull-options.sh
> index d821fab..453aba5 100755
> --- a/t/t5521-pull-options.sh
> +++ b/t/t5521-pull-options.sh
> @@ -91,8 +91,6 @@ test_expect_success 'git pull --force' '
>  	[branch "master"]
>  		remote = two
>  		merge = refs/heads/master
> -	[pull]
> -		rebase = false
>  	EOF
>  	git pull two &&
>  	test_commit A &&
> diff --git a/t/t5700-clone-reference.sh b/t/t5700-clone-reference.sh
> index 306badf..6537911 100755
> --- a/t/t5700-clone-reference.sh
> +++ b/t/t5700-clone-reference.sh
> @@ -94,7 +94,7 @@ cd "$base_dir"
>  
>  test_expect_success 'pulling changes from origin' \
>  'cd C &&
> -git pull --merge origin'
> +git pull origin'
>  
>  cd "$base_dir"
>  
> @@ -109,7 +109,7 @@ cd "$base_dir"
>  
>  test_expect_success 'pulling changes from origin' \
>  'cd D &&
> -git pull --merge origin'
> +git pull origin'
>  
>  cd "$base_dir"
>  
> diff --git a/t/t6022-merge-rename.sh b/t/t6022-merge-rename.sh
> index e12d90b..c680f78 100755
> --- a/t/t6022-merge-rename.sh
> +++ b/t/t6022-merge-rename.sh
> @@ -10,8 +10,6 @@ modify () {
>  
>  test_expect_success setup \
>  '
> -git config pull.rebase false &&
> -
>  cat >A <<\EOF &&
>  a aaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaaa
>  b bbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbbb
> diff --git a/t/t6026-merge-attr.sh b/t/t6026-merge-attr.sh
> index 5428f19..5e43997 100755
> --- a/t/t6026-merge-attr.sh
> +++ b/t/t6026-merge-attr.sh
> @@ -172,7 +172,7 @@ test_expect_success 'up-to-date merge without common ancestor' '
>  	test_tick &&
>  	(
>  		cd repo1 &&
> -		git pull --merge ../repo2 master
> +		git pull ../repo2 master
>  	)
>  '
>  
> diff --git a/t/t6029-merge-subtree.sh b/t/t6029-merge-subtree.sh
> index 3ca29c4..73fc240 100755
> --- a/t/t6029-merge-subtree.sh
> +++ b/t/t6029-merge-subtree.sh
> @@ -41,7 +41,6 @@ test_expect_success 'setup' '
>  	mkdir git &&
>  	cd git &&
>  	git init &&
> -	git config pull.rebase false &&
>  	echo git >git.c &&
>  	o2=$(git hash-object git.c) &&
>  	git add git.c &&
> diff --git a/t/t6037-merge-ours-theirs.sh b/t/t6037-merge-ours-theirs.sh
> index 41bf060..3889eca 100755
> --- a/t/t6037-merge-ours-theirs.sh
> +++ b/t/t6037-merge-ours-theirs.sh
> @@ -66,8 +66,6 @@ test_expect_success 'binary file with -Xours/-Xtheirs' '
>  '
>  
>  test_expect_success 'pull passes -X to underlying merge' '
> -	git config pull.rebase false &&
> -
>  	git reset --hard master && git pull -s recursive -Xours . side &&
>  	git reset --hard master && git pull -s recursive -X ours . side &&
>  	git reset --hard master && git pull -s recursive -Xtheirs . side &&
> diff --git a/t/t9114-git-svn-dcommit-merge.sh b/t/t9114-git-svn-dcommit-merge.sh
> index dfce024..f524d2f 100755
> --- a/t/t9114-git-svn-dcommit-merge.sh
> +++ b/t/t9114-git-svn-dcommit-merge.sh
> @@ -62,7 +62,7 @@ test_expect_success 'setup git mirror and merge' '
>  	echo friend > README &&
>  	cat tmp >> README &&
>  	git commit -a -m "friend" &&
> -	git pull --merge . merge
> +	git pull . merge
>  	'
>  
>  test_debug 'gitk --all & sleep 1'
> diff --git a/t/t9400-git-cvsserver-server.sh b/t/t9400-git-cvsserver-server.sh
> index 76b8640..0431386 100755
> --- a/t/t9400-git-cvsserver-server.sh
> +++ b/t/t9400-git-cvsserver-server.sh
> @@ -46,7 +46,7 @@ test_expect_success 'setup' '
>    touch secondrootfile &&
>    git add secondrootfile &&
>    git commit -m "second root") &&
> -  git pull --merge secondroot master &&
> +  git pull secondroot master &&
>    git clone -q --bare "$WORKDIR/.git" "$SERVERDIR" >/dev/null 2>&1 &&
>    GIT_DIR="$SERVERDIR" git config --bool gitcvs.enabled true &&
>    GIT_DIR="$SERVERDIR" git config gitcvs.logfile "$SERVERDIR/gitcvs.log" &&
> diff --git a/t/t9500-gitweb-standalone-no-errors.sh b/t/t9500-gitweb-standalone-no-errors.sh
> index 787c6cc..6fca193 100755
> --- a/t/t9500-gitweb-standalone-no-errors.sh
> +++ b/t/t9500-gitweb-standalone-no-errors.sh
> @@ -328,7 +328,7 @@ test_expect_success \
>  	 git add b &&
>  	 git commit -a -m "On branch" &&
>  	 git checkout master &&
> -	 git pull --merge . b &&
> +	 git pull . b &&
>  	 git tag merge_commit'
>  
>  test_expect_success \

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[John Keeping]

On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
> diff --git a/git-pull.sh b/git-pull.sh
> index 638aabb..4a6a863 100755
> --- a/git-pull.sh
> +++ b/git-pull.sh
> @@ -264,6 +274,30 @@ case "$merge_head" in
>  		die "$(gettext "Cannot rebase onto multiple branches")"
>  	fi
>  	;;
> +*)
> +	# integrating with a single other history
> +	merge_head=${merge_head% }
> +	if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" &&
> +		test -n "$orig_head" &&
> +		! $(git merge-base --is-ancestor "$orig_head" "$merge_head")

I think this needs to be:

	! $(git merge-base --is-ancestor "$orig_head" "$merge_head" ||
	    git merge-base --is-ancestor "$merge_head" "$orig_head")

in order to avoid printing the message when "git pull" does not fetch
any new changes and the user has some new commits.

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Andreas Schwab]

John Keeping <john@keeping.me.uk> writes:

> On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
>> diff --git a/git-pull.sh b/git-pull.sh
>> index 638aabb..4a6a863 100755
>> --- a/git-pull.sh
>> +++ b/git-pull.sh
>> @@ -264,6 +274,30 @@ case "$merge_head" in
>>  		die "$(gettext "Cannot rebase onto multiple branches")"
>>  	fi
>>  	;;
>> +*)
>> +	# integrating with a single other history
>> +	merge_head=${merge_head% }
>> +	if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" &&
>> +		test -n "$orig_head" &&
>> +		! $(git merge-base --is-ancestor "$orig_head" "$merge_head")
>
> I think this needs to be:
>
> 	! $(git merge-base --is-ancestor "$orig_head" "$merge_head" ||
> 	    git merge-base --is-ancestor "$merge_head" "$orig_head")

Neither makes sense.  You want to check the exit status of git
merge-base --is-ancestor, not execute its (empty) output as a command.

Andreas.

-- 
Andreas Schwab, schwab@linux-m68k.org
GPG Key fingerprint = 58CA 54C7 6D53 942B 1756  01D3 44D5 214B 8276 4ED5
"And now for something completely different."

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

Andreas Schwab <schwab@linux-m68k.org> writes:

> John Keeping <john@keeping.me.uk> writes:
>
>> On Thu, Jun 27, 2013 at 12:48:52PM -0700, Junio C Hamano wrote:
>>> diff --git a/git-pull.sh b/git-pull.sh
>>> index 638aabb..4a6a863 100755
>>> --- a/git-pull.sh
>>> +++ b/git-pull.sh
>>> @@ -264,6 +274,30 @@ case "$merge_head" in
>>>  		die "$(gettext "Cannot rebase onto multiple branches")"
>>>  	fi
>>>  	;;
>>> +*)
>>> +	# integrating with a single other history
>>> +	merge_head=${merge_head% }
>>> +	if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" &&
>>> +		test -n "$orig_head" &&
>>> +		! $(git merge-base --is-ancestor "$orig_head" "$merge_head")
>>
>> I think this needs to be:
>>
>> 	! $(git merge-base --is-ancestor "$orig_head" "$merge_head" ||
>> 	    git merge-base --is-ancestor "$merge_head" "$orig_head")
>
> Neither makes sense.  You want to check the exit status of git
> merge-base --is-ancestor, not execute its (empty) output as a command.

Gaah.  You are right.

Thanks for spotting.

[PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

Because it is so easy to let Git handle automatically a trivial
merge with "git pull", a person who is new to Git may not realize
that the project s/he is interacting with may prefer a "rebase"
workflow.

Add a safety valve to fail "git pull" that does not explicitly
specify what branch from which repository to integrate your history
with, when it is neither a fast-forward or "already up-to-date",
until/unless the user expressed her preference between the two ways
of integration.

This can be an irritating backward incompatible change for old
timers, but it can be a one time irritation by doing:

    git config --global pull.rebase false

once to say "I will always --merge", and they'd not even notice.

    http://thread.gmane.org/gmane.comp.version-control.git/225146/focus=225326

for a full discussion.

Helped-by: John Keeping <john@keeping.me.uk>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
---

 * This time with updates to the documentation and the test suite.

 Documentation/git-pull.txt |  9 ++++++++
 git-pull.sh                | 40 +++++++++++++++++++++++++++++++++++-
 t/t5520-pull.sh            | 51 ++++++++++++++++++++++++++++++++++++++++++++++
 t/t5524-pull-msg.sh        |  2 +-
 4 files changed, 100 insertions(+), 2 deletions(-)

diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
index 24ab07a..86f5170 100644
--- a/Documentation/git-pull.txt
+++ b/Documentation/git-pull.txt
@@ -97,6 +97,14 @@ must be given before the options meant for 'git fetch'.
 Options related to merging
 ~~~~~~~~~~~~~~~~~~~~~~~~~~
 
+When `git pull` that does not explicitly specify what branch from
+which repository is to be integrated with your history on the
+command line, recent Git will refuse to work until you specify how
+that integration should happen, either with a command line option
+(`--merge` or `--rebase`) or a configuration variable (`pull.rebase`
+or `branch.<name>.rebase`, which is the same as `--merge`
+(`--rebase`) when set to `false` (`true`) respectively.
+
 include::merge-options.txt[]
 
 :git-pull: 1
@@ -119,6 +127,7 @@ It rewrites history, which does not bode well when you
 published that history already.  Do *not* use this option
 unless you have read linkgit:git-rebase[1] carefully.
 
+--merge::
 --no-rebase::
 	Override earlier --rebase.
 
diff --git a/git-pull.sh b/git-pull.sh
index 638aabb..88c198f 100755
--- a/git-pull.sh
+++ b/git-pull.sh
@@ -41,13 +41,21 @@ test -f "$GIT_DIR/MERGE_HEAD" && die_merge
 strategy_args= diffstat= no_commit= squash= no_ff= ff_only=
 log_arg= verbosity= progress= recurse_submodules= verify_signatures=
 merge_args= edit=
+
 curr_branch=$(git symbolic-ref -q HEAD)
 curr_branch_short="${curr_branch#refs/heads/}"
+
+# See if we are configured to rebase by default.
+# The value $rebase is, throughout the main part of the code:
+#    (empty) - the user did not have any preference
+#    true    - the user told us to integrate by rebasing
+#    false   - the user told us to integrate by merging
 rebase=$(git config --bool branch.$curr_branch_short.rebase)
 if test -z "$rebase"
 then
 	rebase=$(git config --bool pull.rebase)
 fi
+
 dry_run=
 while :
 do
@@ -113,7 +121,8 @@ do
 	-r|--r|--re|--reb|--reba|--rebas|--rebase)
 		rebase=true
 		;;
-	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase)
+	--no-r|--no-re|--no-reb|--no-reba|--no-rebas|--no-rebase|\
+	-m|--m|--me|--mer|--merg|--merge)
 		rebase=false
 		;;
 	--recurse-submodules)
@@ -219,6 +228,7 @@ test true = "$rebase" && {
 		fi
 	done
 }
+
 orig_head=$(git rev-parse -q --verify HEAD)
 git fetch $verbosity $progress $dry_run $recurse_submodules --update-head-ok "$@" || exit 1
 test -z "$dry_run" || exit 0
@@ -264,6 +274,34 @@ case "$merge_head" in
 		die "$(gettext "Cannot rebase onto multiple branches")"
 	fi
 	;;
+*)
+	# integrating with a single other history; be careful not to
+	# trigger this check when we will say "fast-forward" or "already
+	# up-to-date".
+	merge_head=${merge_head% }
+	if test -z "$rebase$no_ff$ff_only${squash#--no-squash}" &&
+		test -n "$orig_head" &&
+		test $# = 0 &&
+		! git merge-base --is-ancestor "$orig_head" "$merge_head" &&
+		! git merge-base --is-ancestor "$merge_head" "$orig_head"
+	then
+echo >&2 "orig-head was $orig_head"
+echo >&2 "merge-head is $merge_head"
+git show >&2 --oneline -s "$orig_head" "$merge_head"
+
+		die "The pull does not fast-forward; please specify
+if you want to merge or rebase.
+
+Use either
+
+    git pull --rebase
+    git pull --merge
+
+You can also use 'git config pull.rebase true' (if you want --rebase) or
+'git config pull.rebase false' (if you want --merge) to set this once for
+this project and forget about it."
+	fi
+	;;
 esac
 
 if test -z "$orig_head"
diff --git a/t/t5520-pull.sh b/t/t5520-pull.sh
index 6af6c63..1e91eca 100755
--- a/t/t5520-pull.sh
+++ b/t/t5520-pull.sh
@@ -255,4 +255,55 @@ test_expect_success 'git pull --rebase against local branch' '
 	test file = "$(cat file2)"
 '
 
+test_expect_success 'git pull that does not say how to integrate' '
+	git checkout -b other master^1 &&
+	>new &&
+	git add new &&
+	git commit -m "add new file" &&
+
+	git checkout -b test-to-integrate master &&
+
+	test_config branch.test-to-integrate.remote . &&
+	test_config branch.test-to-integrate.merge other &&
+
+	# need real integration
+	test_must_fail git pull &&
+	git reset --hard master &&
+
+
+	# configuration is explicit enough
+	for how in false true
+	do
+		test_config pull.rebase $how &&
+		git pull &&
+		git reset --hard master || break
+	done &&
+
+	# per branch configuration is explicit enough
+	test_unconfig pull.rebase &&
+	for how in false true
+	do
+		test_config branch.test-to-integrate.rebase $how &&
+		git pull &&
+		git reset --hard master || break
+	done &&
+
+	test_unconfig pull.rebase &&
+	test_unconfig branch.test-to-integrate &&
+
+	# already up to date
+	git reset --hard master &&
+	git branch -f other master^1
+	git pull &&
+
+	# fast forward
+	git reset --hard master &&
+	git checkout -B other master &&
+	>new &&
+	git add new &&
+	git commit -m "add new file" &&
+	git checkout -B test-to-integrate master &&
+	git pull
+'
+
 test_done
diff --git a/t/t5524-pull-msg.sh b/t/t5524-pull-msg.sh
index 8cccecc..660714b 100755
--- a/t/t5524-pull-msg.sh
+++ b/t/t5524-pull-msg.sh
@@ -25,7 +25,7 @@ test_expect_success setup '
 test_expect_success pull '
 (
 	cd cloned &&
-	git pull --log &&
+	git pull --log --merge &&
 	git log -2 &&
 	git cat-file commit HEAD >result &&
 	grep Dollar result
-- 
1.8.3.3-992-gf0e5e44

Re: [PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull

[Eric Sunshine]

On Thu, Jul 18, 2013 at 3:35 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Add a safety valve to fail "git pull" that does not explicitly
> specify what branch from which repository to integrate your history
> with, when it is neither a fast-forward or "already up-to-date",
> until/unless the user expressed her preference between the two ways
> of integration.
> ---
> diff --git a/Documentation/git-pull.txt b/Documentation/git-pull.txt
> index 24ab07a..86f5170 100644
> --- a/Documentation/git-pull.txt
> +++ b/Documentation/git-pull.txt
> @@ -97,6 +97,14 @@ must be given before the options meant for 'git fetch'.
>  Options related to merging
>  ~~~~~~~~~~~~~~~~~~~~~~~~~~
>
> +When `git pull` that does not explicitly specify what branch from
> +which repository is to be integrated with your history on the
> +command line, recent Git will refuse to work until you specify how
> +that integration should happen, either with a command line option
> +(`--merge` or `--rebase`) or a configuration variable (`pull.rebase`
> +or `branch.<name>.rebase`, which is the same as `--merge`
> +(`--rebase`) when set to `false` (`true`) respectively.

This paragraph-long single sentence may be intimidating. Perhaps some
simplification is possible:

    As a safety measure, bare `git pull` (without repository or
    branch) needs to be told how to integrate pulled changes with
    your history; either via `--merge` or `--rebase`.  Also see
    configuration variables `pull.rebase` and `branch.<name>.rebase`
    in linkgit:git-config[1].

I intentionally omitted the true/false explanation of the
configuration variables since the user can follow the link and read
about them. It also may make sense to drop mention of those variables
altogether since they are already described (including link) in the
description of --rebase.

I also intentionally omitted "recent Git" since it's rather nebulous.

Re: [PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>> +When `git pull` that does not explicitly specify what branch from
>> +which repository is to be integrated with your history on the
>> +command line, recent Git will refuse to work until you specify how
>> +that integration should happen, either with a command line option
>> +(`--merge` or `--rebase`) or a configuration variable (`pull.rebase`
>> +or `branch.<name>.rebase`, which is the same as `--merge`
>> +(`--rebase`) when set to `false` (`true`) respectively.
>
> This paragraph-long single sentence may be intimidating. Perhaps some
> simplification is possible:
>
>     As a safety measure, bare `git pull` (without repository or
>     branch) needs to be told how to integrate pulled changes with
>     your history; either via `--merge` or `--rebase`.  Also see
>     configuration variables `pull.rebase` and `branch.<name>.rebase`
>     in linkgit:git-config[1].
>
> I intentionally omitted the true/false explanation of the
> configuration variables since the user can follow the link and read
> about them. It also may make sense to drop mention of those variables
> altogether since they are already described (including link) in the
> description of --rebase.
>
> I also intentionally omitted "recent Git" since it's rather nebulous.

Looks much better than the original.  I would further suggest
dropping the "As a safety measure, bare " at the beginning.

      `git pull` (without repository or branch on the command line)
      needs to be told how to integrate the changes with your
      history via either `--merge` or `--rebase` (see configuration
      variables `pull.rebase` and `branch.<name>.rebase` in
      linkgit:git-config[1]).

perhaps?

Re: [PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull

[Eric Sunshine]

On Fri, Jul 19, 2013 at 12:22 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>>> +When `git pull` that does not explicitly specify what branch from
>>> +which repository is to be integrated with your history on the
>>> +command line, recent Git will refuse to work until you specify how
>>> +that integration should happen, either with a command line option
>>> +(`--merge` or `--rebase`) or a configuration variable (`pull.rebase`
>>> +or `branch.<name>.rebase`, which is the same as `--merge`
>>> +(`--rebase`) when set to `false` (`true`) respectively.
>>
>> This paragraph-long single sentence may be intimidating. Perhaps some
>> simplification is possible:
>>
>>     As a safety measure, bare `git pull` (without repository or
>>     branch) needs to be told how to integrate pulled changes with
>>     your history; either via `--merge` or `--rebase`.  Also see
>>     configuration variables `pull.rebase` and `branch.<name>.rebase`
>>     in linkgit:git-config[1].
>>
>> I intentionally omitted the true/false explanation of the
>> configuration variables since the user can follow the link and read
>> about them. It also may make sense to drop mention of those variables
>> altogether since they are already described (including link) in the
>> description of --rebase.
>>
>> I also intentionally omitted "recent Git" since it's rather nebulous.
>
> Looks much better than the original.  I would further suggest
> dropping the "As a safety measure, bare " at the beginning.
>
>       `git pull` (without repository or branch on the command line)
>       needs to be told how to integrate the changes with your
>       history via either `--merge` or `--rebase` (see configuration
>       variables `pull.rebase` and `branch.<name>.rebase` in
>       linkgit:git-config[1]).
>
> perhaps?

That works; or without the mentioning the configuration variables at
all (assuming the reader will discover them from reading --rebase
description):

    `git pull` (without repository or branch on the command line)
    needs to be told how to integrate the changes with your history
    via either `--merge` or `--rebase`.

Dropping the parenthetical comment might improve flow slightly:

    Without repository or branch on the command line, `git pull`
    needs to be told how to integrate the changes with your history,
    via either `--merge` or `--rebase`.

With or without mention of the configuration options, either phrasing
seems pretty easy to digest.

Re: [PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

> Dropping the parenthetical comment might improve flow slightly:
>
>     Without repository or branch on the command line, `git pull`
>     needs to be told how to integrate the changes with your history,
>     via either `--merge` or `--rebase`.
>
> With or without mention of the configuration options, either phrasing
> seems pretty easy to digest.

Yeah, that reads much better, but I do prefer to see something that
explains this is often "just make sure you use the one that suits
your project and always use that".  How about something like this?

    With no repository or branch on the command line, `git pull` needs
    to be told how to integrate the changes with your history.

    This can be done via either `--merge` or `--rebase` option, but most
    people would want to decide which method matches the workflow of the
    project once, and set the configuration variable `pull.rebase` or
    `branch.<name>.rebase` to stick to it; see linkgit:git-config[1].

Re: [PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull

[Eric Sunshine]

On Fri, Jul 19, 2013 at 6:20 PM, Junio C Hamano <gitster@pobox.com> wrote:
> Eric Sunshine <sunshine@sunshineco.com> writes:
>
>> Dropping the parenthetical comment might improve flow slightly:
>>
>>     Without repository or branch on the command line, `git pull`
>>     needs to be told how to integrate the changes with your history,
>>     via either `--merge` or `--rebase`.
>>
>> With or without mention of the configuration options, either phrasing
>> seems pretty easy to digest.
>
> Yeah, that reads much better, but I do prefer to see something that
> explains this is often "just make sure you use the one that suits
> your project and always use that".  How about something like this?
>
>     With no repository or branch on the command line, `git pull` needs
>     to be told how to integrate the changes with your history.
>
>     This can be done via either `--merge` or `--rebase` option, but most
>     people would want to decide which method matches the workflow of the
>     project once, and set the configuration variable `pull.rebase` or
>     `branch.<name>.rebase` to stick to it; see linkgit:git-config[1].

At this point, I'm probably just bike-shedding. Perhaps?

    With no repository or branch on the command line, `git pull`
    needs to be told how to integrate the changes with your history,
    via either `--merge` or `--rebase`.

    To match a project's workflow and make the choice of merge or
    rebase permanent, set configuration variable `pull.rebase` or
    `branch.<name>.rebase` (see linkgit:git-config[1]).

Re: [PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull

[Junio C Hamano]

Eric Sunshine <sunshine@sunshineco.com> writes:

>>     With no repository or branch on the command line, `git pull` needs
>>     to be told how to integrate the changes with your history.
>>
>>     This can be done via either `--merge` or `--rebase` option, but most
>>     people would want to decide which method matches the workflow of the
>>     project once, and set the configuration variable `pull.rebase` or
>>     `branch.<name>.rebase` to stick to it; see linkgit:git-config[1].
>
> At this point, I'm probably just bike-shedding. Perhaps?
>
>     With no repository or branch on the command line, `git pull`
>     needs to be told how to integrate the changes with your history,
>     via either `--merge` or `--rebase`.
>
>     To match a project's workflow and make the choice of merge or
>     rebase permanent, set configuration variable `pull.rebase` or
>     `branch.<name>.rebase` (see linkgit:git-config[1]).

I agree with the bike-shedding aspect of your comment, and actually
I like my version better.

It makes it clear that a single-shot --merge or --rebase from the
command line is not recommended.  "To match the project's workflow"
is not optional in most projects, and it is preferrable to decide
once and set the choice in stone.

Re: [PATCH] pull: require choice between rebase/merge on non-fast-forward pull

[Felipe Contreras]

On Fri, Jun 28, 2013 at 5:41 PM, Junio C Hamano <gitster@pobox.com> wrote:
> John Keeping <john@keeping.me.uk> writes:

>> I don't think "git pull remote branch" falls into the same category as
>> plain "git pull" so I'm not convinced that defaulting to merge there is
>> unreasonable.  The original message about this [1] did talk about only
>> "git pull" with no arguments.
>
> If you want to limit the scope to only "git pull" (without any
> command line argument), I actually do not have strong preference for
> or against it either way.  Perhaps a follow-up patch to be squashed?

I do. Whether the user does 'git pull' or 'git pull origin' doesn't
matter, we still want to reject non-fast-forward merges.

-- 
Felipe Contreras

Re: [PATCH v2] pull: require choice between rebase/merge on non-fast-forward pull

[Flimm]

Has this patch been released yet?



--
View this message in context: http://git.661346.n2.nabble.com/first-parent-commit-graph-layout-and-pull-merge-direction-tp7586671p7602383.html
Sent from the git mailing list archive at Nabble.com.