[PATCH v2 0/1] doc: format-patch

[PATCH v2 0/1] doc: format-patch

[Philip Oakley]

Following the discussion at [1], I decided that while I felt the
documentation changes were OK, the commit message need more explanation
to cover the points raised in the discussion. Thus the only changes
are to the commit message.

In addition I'll separately cover the point raised about the DWIM
description in gitrevisions(7). I'd also picked up that <refspecs>
aren't covered by it either.

Philip

[1] http://thread.gmane.org/gmane.comp.version-control.git/254652

Philip Oakley (1):
  doc: format-patch: don't use origin as a branch name

 Documentation/git-format-patch.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

-- 
1.9.4.msysgit.0

[PATCH v2 1/1] doc: format-patch: don't use origin as a branch name

[Philip Oakley]

Historically (5 Nov 2005 v0.99.9-46-g28ffb89) the git-format-patch used
'origin' as the upstream branch name. That name is now used as the nominal
name for the upstream remote.

While 'origin' would be DWIMmed (do what I mean) to be that remote's
primary branch, do not assume the reader is ready for such magic.

Likewise, do not use 'origin/master' which may not be up to date with the
remote, nor reflect the reader's master branch. The patch series should be
relative to the reader's view of 'git show-branch HEAD master'.

Use the more modern 'master' as the reference branch name.

Signed-off-by: Philip Oakley <philipoakley@iee.org>
---
 Documentation/git-format-patch.txt | 10 +++++-----
 1 file changed, 5 insertions(+), 5 deletions(-)

diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
index c0fd470..b0f041f 100644
--- a/Documentation/git-format-patch.txt
+++ b/Documentation/git-format-patch.txt
@@ -523,25 +523,25 @@ $ git format-patch -k --stdout R1..R2 | git am -3 -k
 ------------
 
 * Extract all commits which are in the current branch but not in the
-origin branch:
+master branch:
 +
 ------------
-$ git format-patch origin
+$ git format-patch master
 ------------
 +
 For each commit a separate file is created in the current directory.
 
-* Extract all commits that lead to 'origin' since the inception of the
+* Extract all commits that lead to 'master' since the inception of the
 project:
 +
 ------------
-$ git format-patch --root origin
+$ git format-patch --root master
 ------------
 
 * The same as the previous one:
 +
 ------------
-$ git format-patch -M -B origin
+$ git format-patch -M -B master
 ------------
 +
 Additionally, it detects and handles renames and complete rewrites
-- 
1.9.4.msysgit.0

Re: [PATCH v2 1/1] doc: format-patch: don't use origin as a branch name

[Junio C Hamano]

Philip Oakley <philipoakley@iee.org> writes:

> Historically (5 Nov 2005 v0.99.9-46-g28ffb89) the git-format-patch used
> 'origin' as the upstream branch name. That name is now used as the nominal
> name for the upstream remote.
>
> While 'origin' would be DWIMmed (do what I mean) to be that remote's
> primary branch, do not assume the reader is ready for such magic.

Good thinking.

> Likewise, do not use 'origin/master' which may not be up to date with the
> remote, nor reflect the reader's master branch. The patch series should be
> relative to the reader's view of 'git show-branch HEAD master'.

This however is backwards, no?  The history on 'origin/master' may
not be up-to-date in the sense that if you run 'git fetch' you might
get more, but it absolutely is up-to-date in the sense that it shows
what the origin has to the best of your repository's current
knowledge.

Compared to that, what the user's local 'master' has is much less
relevant.  For one thing, if a more recent commit that is on the
remote repository is missing on 'origin/master' because you haven't
fetched recently, by definition that commit will not be on your
'master' either, so you have the same staleness issue to the exact
degree.  Even worse, when you are developing a topic to upstream, it
is a good practice to merge your topic to your own 'master' to check
it with the wider project codebase that is more recent than where
your topic earlier forked from, and it makes little sense to tell
'exclude what I have on my master' to format-patch when extracting
changes to upstream out of such a topic.  You send what the other
side has, not what you do not have on your local 'master' branch.

> Use the more modern 'master' as the reference branch name.

There is nothing 'modern' in 'master'.

I think the original description was written before we switched to
the separate remote layout.  What is in 'refs/remote/origin/master'
these days was stored and updated at 'refs/heads/origin' and no
other branch from the remote repository was tracked back then.  The
changes to be upstreamed are output by grabbing what are not in
'origin', whose modern equivalent is 'origin/master'.

In short, if your patch were s|origin|origin/master|, instead of
s|origin|master|, that would be an adjustment to the more modern
world that is still faithful to the intent of the original.

> Signed-off-by: Philip Oakley <philipoakley@iee.org>
> ---
>  Documentation/git-format-patch.txt | 10 +++++-----
>  1 file changed, 5 insertions(+), 5 deletions(-)
>
> diff --git a/Documentation/git-format-patch.txt b/Documentation/git-format-patch.txt
> index c0fd470..b0f041f 100644
> --- a/Documentation/git-format-patch.txt
> +++ b/Documentation/git-format-patch.txt
> @@ -523,25 +523,25 @@ $ git format-patch -k --stdout R1..R2 | git am -3 -k
>  ------------
>  
>  * Extract all commits which are in the current branch but not in the
> -origin branch:
> +master branch:
>  +
>  ------------
> -$ git format-patch origin
> +$ git format-patch master
>  ------------
>  +
>  For each commit a separate file is created in the current directory.
>  
> -* Extract all commits that lead to 'origin' since the inception of the
> +* Extract all commits that lead to 'master' since the inception of the
>  project:
>  +
>  ------------
> -$ git format-patch --root origin
> +$ git format-patch --root master
>  ------------
>  
>  * The same as the previous one:
>  +
>  ------------
> -$ git format-patch -M -B origin
> +$ git format-patch -M -B master
>  ------------
>  +
>  Additionally, it detects and handles renames and complete rewrites

Re: [PATCH v2 1/1] doc: format-patch: don't use origin as a branch name

[Junio C Hamano]

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

> Compared to that, what the user's local 'master' has is much less
> relevant.  For one thing, if a more recent commit that is on the
> remote repository is missing on 'origin/master' because you haven't
> fetched recently, by definition that commit will not be on your
> 'master' either, so you have the same staleness issue to the exact
> degree.  Even worse, when you are developing a topic to upstream, it

clarification.  I used "to upstream" as a verb to mean "sending the
work you did to be applied".

> is a good practice to merge your topic to your own 'master' to check
> it with the wider project codebase that is more recent than where
> your topic earlier forked from, and it makes little sense to tell
> 'exclude what I have on my master' to format-patch when extracting
> changes to upstream out of such a topic.  You send what the other
> side has, not what you do not have on your local 'master' branch.

and I have a stupid typo here; obviously I should have typed: You
send what the other side "does not have".

Re: [PATCH v2 1/1] doc: format-patch: don't use origin as a branch name

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> Philip Oakley <philipoakley@iee.org> writes:
>
>> Historically (5 Nov 2005 v0.99.9-46-g28ffb89) the git-format-patch 
>> used
>> 'origin' as the upstream branch name. That name is now used as the 
>> nominal
>> name for the upstream remote.
>>
>> While 'origin' would be DWIMmed (do what I mean) to be that remote's
>> primary branch, do not assume the reader is ready for such magic.
>
> Good thinking.
>
>> Likewise, do not use 'origin/master' which may not be up to date with 
>> the
>> remote, nor reflect the reader's master branch. The patch series 
>> should be
>> relative to the reader's view of 'git show-branch HEAD master'.
>
> This however is backwards, no?  The history on 'origin/master' may
> not be up-to-date in the sense that if you run 'git fetch' you might
> get more, but it absolutely is up-to-date in the sense that it shows
> what the origin has to the best of your repository's current
> knowledge.

I still think that the user/reader shouldn't be creating patches based 
on wherever someone else had got to, rather it should just be patches 
from their own feature branch. However the rest of your argument still 
stands with regard to accidental/unexpected conflicts with other 
upstream work, and the reader should ensure they are already up to 
date - maybe it needs a comment line to state that.

>
> Compared to that, what the user's local 'master' has is much less
> relevant.  For one thing, if a more recent commit that is on the
> remote repository is missing on 'origin/master' because you haven't
> fetched recently, by definition that commit will not be on your
> 'master' either, so you have the same staleness issue to the exact
> degree.  Even worse, when you are developing a topic to upstream, it
> is a good practice to merge your topic to your own 'master' to check
> it with the wider project codebase that is more recent than where
> your topic earlier forked from, and it makes little sense to tell
> 'exclude what I have on my master' to format-patch when extracting
> changes to upstream out of such a topic.  You send what the other
> side has, not what you do not have on your local 'master' branch.
>
>> Use the more modern 'master' as the reference branch name.
>
> There is nothing 'modern' in 'master'.

Noted.

>
> I think the original description was written before we switched to
> the separate remote layout.  What is in 'refs/remote/origin/master'
> these days was stored and updated at 'refs/heads/origin' and no
> other branch from the remote repository was tracked back then.  The
> changes to be upstreamed are output by grabbing what are not in
> 'origin', whose modern equivalent is 'origin/master'.
>
> In short, if your patch were s|origin|origin/master|, instead of
> s|origin|master|, that would be an adjustment to the more modern
> world that is still faithful to the intent of the original.

I think we would need to clarify that (the intent) for the reader. I'll 
see what I can do. (suggestion below)

>
>> Signed-off-by: Philip Oakley <philipoakley@iee.org>
>> ---
>>  Documentation/git-format-patch.txt | 10 +++++-----
>>  1 file changed, 5 insertions(+), 5 deletions(-)
>>
>> diff --git a/Documentation/git-format-patch.txt 
>> b/Documentation/git-format-patch.txt
>> index c0fd470..b0f041f 100644
>> --- a/Documentation/git-format-patch.txt
>> +++ b/Documentation/git-format-patch.txt
>> @@ -523,25 +523,25 @@ $ git format-patch -k --stdout R1..R2 | git 
>> am -3 -k
>>  ------------
>>
>>  * Extract all commits which are in the current branch but not in the
>> -origin branch:
>> +master branch:
>>  +
>>  ------------
>> -$ git format-patch origin
>> +$ git format-patch master
>>  ------------
>>  +
>>  For each commit a separate file is created in the current directory.

Perhaps insert "Note: Your 'master' should be up to date with respect to 
'origin/master' before creating and sending patches upstream to avoid 
unexpected conflicts." ?

>>
>> -* Extract all commits that lead to 'origin' since the inception of 
>> the
>> +* Extract all commits that lead to 'master' since the inception of 
>> the
>>  project:
>>  +
>>  ------------
>> -$ git format-patch --root origin
>> +$ git format-patch --root master
>>  ------------
>>
>>  * The same as the previous one:
>>  +
>>  ------------
>> -$ git format-patch -M -B origin
>> +$ git format-patch -M -B master
>>  ------------
>>  +
>>  Additionally, it detects and handles renames and complete rewrites
> --
Philip 

Re: [PATCH v2 1/1] doc: format-patch: don't use origin as a branch name

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

>> This however is backwards, no?  The history on 'origin/master' may
>> not be up-to-date in the sense that if you run 'git fetch' you might
>> get more, but it absolutely is up-to-date in the sense that it shows
>> what the origin has to the best of your repository's current
>> knowledge.
>
> I still think that the user/reader shouldn't be creating patches based
> on wherever someone else had got to, rather it should just be patches
> from their own feature branch.

You forked your topic branch off of the shared project history aka
origin/master and built some.  You may have sent some patches off of
your previous work to the upstream, and origin/master may or may not
have applied some of them since your topic forked from it.  The
patches you are sending out is from your own topic branch.

You may be cooking multiple topics, and your local 'master' branch,
which you never push back to 'origin/master', may contain any of
these branches.  You do not fork off a new topic out of there.  Best
case, you would fork from 'origin/master'; a bit worse case, you
have to fork from another of your topic branch that your new topic
has to depend on.

Nowhere I am assuming that "the reader is creating paches based on
wherever someone else had got to".  Sorry, but I have no idea what
you are complaining about.

> However the rest of your argument still
> stands with regard to accidental/unexpected conflicts with other
> upstream work, and the reader should ensure they are already up to
> date - maybe it needs a comment line to state that.

Sorry, but I am not sure how much you understood what I wrote.

The primary reason why 'origin' in the example should be replaced
with 'origin/master' is because that is the literal adjustment from
the pre-separate-remote world order to today's world order.  The
local branch 'origin' (more specifically, 'refs/heads/origin') used
to be what we used to keep track of 'master' of the upstream, which
we use 'refs/remotes/origin/master' these days.

	Side note: DWIMming origin to remotes/origin/HEAD to
	remotes/origin/master was invented to keep supporting this
	"'origin' keeps track of the default upstream" convention
	when we transitioned from the old world order to
	separate-remote layout.

And the reason why 'origin' should not be replaced with 'master' is
because your 'master' may already have patches from the topic you
are working on, i.e. in your current branch, that the upstream does
not yet have.  Running "git format-patch origin/master" will show
what needs to be accepted by the upstream from you to reproduce your
work in full; if you run "git format-patch master", it may miss some
parts that you already have in your local 'master' but not yet in
the upstream.

I never talked about conflicts, and I still think that it is
completely outside the scope of these examples.  Avoidance of
conflicts with the work that is already commited to your upstream
since you forked is the job for "rebase", not "format-patch".  The
reason why it is wrong to replace 'origin' in that text with 'master'
does not have anything to do with conflict avoidance.

Puzzled...

Re: [PATCH v2 1/1] doc: format-patch: don't use origin as a branch name

[Philip Oakley]

From: "Junio C Hamano" <gitster@pobox.com>
> "Philip Oakley" <philipoakley@iee.org> writes:
>
>>> This however is backwards, no?  The history on 'origin/master' may
>>> not be up-to-date in the sense that if you run 'git fetch' you might
>>> get more, but it absolutely is up-to-date in the sense that it shows
>>> what the origin has to the best of your repository's current
>>> knowledge.
>>
>> I still think that the user/reader shouldn't be creating patches 
>> based
>> on wherever someone else had got to, rather it should just be patches
>> from their own feature branch.
>
> You forked your topic branch off of the shared project history aka
> origin/master and built some.  You may have sent some patches off of
> your previous work to the upstream, and origin/master may or may not
> have applied some of them since your topic forked from it.  The
> patches you are sending out is from your own topic branch.
>
> You may be cooking multiple topics, and your local 'master' branch,
> which you never push back to 'origin/master', may contain any of
> these branches.  You do not fork off a new topic out of there.  Best
> case, you would fork from 'origin/master'; a bit worse case, you
> have to fork from another of your topic branch that your new topic
> has to depend on.
>
> Nowhere I am assuming that "the reader is creating paches based on
> wherever someone else had got to".  Sorry, but I have no idea what
> you are complaining about.

I think we are talking at cross purposes. My starting point is that (the 
examples says that) the reader wants to create a patch series for a 
local branch, relative to their <some name> branch which they branched 
from (e.g. the example, relative to Git, could have been from a 'pu' 
picked up a couple of days earlier, when they'd have said 'git 
format-patch pu' ;-).

Having generated their short patch series, and they probably want to 
expose the series to some collaborator or other for comment, help and 
guidance (or whatever). They may just want to review it themselves. But 
that choice of what to do with it is surely not part of the format-patch 
documentation. So I'm trying to avoid defining a workflow (then or now).

In the case when the patch series is meant to be ready for being applied 
upstream then all the other considerations apply, but the example 
doesn't, at least in my eyes, claim to be that.

IIRC I once did make the mistake of using format-patch on a branch (off 
pu) I hadn't rebased since fetching and updating the local pu, so it 
produced a lot of extra patches, but I digress.
>
>> However the rest of your argument still
>> stands with regard to accidental/unexpected conflicts with other
>> upstream work, and the reader should ensure they are already up to
>> date - maybe it needs a comment line to state that.
>
> Sorry, but I am not sure how much you understood what I wrote.
That may be true. I taken your "not be up-to-date" to be relative to the 
real upstream.

>
> The primary reason why 'origin' in the example should be replaced
> with 'origin/master' is because that is the literal adjustment from
> the pre-separate-remote world order to today's world order.

I was trying to avoid a literal adjustment to what I'd perceived as a 
presumed workflow.

> The
> local branch 'origin' (more specifically, 'refs/heads/origin') used
> to be what we used to keep track of 'master' of the upstream, which
> we use 'refs/remotes/origin/master' these days.
>
> Side note: DWIMming origin to remotes/origin/HEAD to
> remotes/origin/master was invented to keep supporting this
> "'origin' keeps track of the default upstream" convention
> when we transitioned from the old world order to
> separate-remote layout.
>
> And the reason why 'origin' should not be replaced with 'master' is
> because your 'master' may already have patches from the topic you
> are working on, i.e. in your current branch, that the upstream does
> not yet have.

So this a 'develop on master' view, rather than a 'develop on feature 
branches' approach? Which could explain the misunderstanding.

> Running "git format-patch origin/master" will show
> what needs to be accepted by the upstream from you to reproduce your
> work in full; if you run "git format-patch master", it may miss some
> parts that you already have in your local 'master' but not yet in
> the upstream.
>
> I never talked about conflicts, and I still think that it is
> completely outside the scope of these examples.  Avoidance of
> conflicts with the work that is already commited to your upstream
> since you forked is the job for "rebase", not "format-patch".  The
> reason why it is wrong to replace 'origin' in that text with 'master'
> does not have anything to do with conflict avoidance.
>
> Puzzled...
> --
Did any of that help?
--
Philip 

Re: [PATCH v2 1/1] doc: format-patch: don't use origin as a branch name

[Junio C Hamano]

"Philip Oakley" <philipoakley@iee.org> writes:

> From: "Junio C Hamano" <gitster@pobox.com>
> ...
>> Nowhere I am assuming that "the reader is creating paches based on
>> wherever someone else had got to".  Sorry, but I have no idea what
>> you are complaining about.
>
> I think we are talking at cross purposes. My starting point is that
> (the examples says that) the reader wants to create a patch series for
> a local branch, relative to their <some name> branch which they
> branched from...

Perhaps what you are missing is that the 'origin' in that example is
not "their" <some name> branch.  It is how we used to spell what we
call 'refs/remotes/origin/HEAD' these days, a copy of their upstream
repository's primary branch.

> (e.g. the example, relative to Git, could have been from
> branched from (e.g. the example, relative to Git, could have been from
> a 'pu' picked up a couple of days earlier, when they'd have said 'git
> format-patch pu' ;-).

Again, if that were a "'pu' picked up a few days earlier, it would
not be 'pu', but be 'origin/pu'".

>> The primary reason why 'origin' in the example should be replaced
>> with 'origin/master' is because that is the literal adjustment from
>> the pre-separate-remote world order to today's world order.
>
> I was trying to avoid a literal adjustment to what I'd perceived as a
> presumed workflow.

These are "examples", showing uses of commands in some hopefully
common scenarios.  I am not exactly sure what you are aiming at, but
if you are trying to strip context and/or background from them and
trying to limit them purely to "If you do X, Y happens", the
resulting description would lack clues that readers rely on in order
to choose the usage pattern of the command that is suitable for
their situation, which I do not think is a good change to make.  The
readers would be helped more with "You are in state A and want to
achieve B. If you do X starting from state A, Y happens, which helps
you achieve B.", and that is what examples are about.

Now, these "where you are and what you want to do" may not be
explicitly spelled out to avoid redundancy, and it may be an
improvement to enhance the scenario without making them too narrow.
But that would be a separate change, and renaming 'origin' (whose
modern equivalent is 'origin/master' in the context of these
examples) to 'master' alone would not do any such enhancement.

>> The
>> local branch 'origin' (more specifically, 'refs/heads/origin') used
>> to be what we used to keep track of 'master' of the upstream, which
>> we use 'refs/remotes/origin/master' these days.
>>
>> Side note: DWIMming origin to remotes/origin/HEAD to
>> remotes/origin/master was invented to keep supporting this
>> "'origin' keeps track of the default upstream" convention
>> when we transitioned from the old world order to
>> separate-remote layout.
>>
>> And the reason why 'origin' should not be replaced with 'master' is
>> because your 'master' may already have patches from the topic you
>> are working on, i.e. in your current branch, that the upstream does
>> not yet have.
>
> So this a 'develop on master' view, rather than a 'develop on feature
> branches' approach? Which could explain the misunderstanding.

The new work on the feature branches may be merged in 'master'
without ever intending to push 'master' out.  The development is
still done on the topic branches that are merged to your local
'master', perhaps for testing purposes and most likely to personally
use it before the upstream picks them up.

I suspect your misunderstanding is primarily coming from that you
may have forgotten, or you may be too new to know, that 'origin' in
the example, 'refs/heads/origin', used to be how we tracked the
primary branch of the other side back in the era when these examples
were written, and refs/remotes/origin/master is used for the same
tracking these days.