[PATCH v9] Documentation/remote-helpers: Add invocation section

[PATCH v9] Documentation/remote-helpers: Add invocation section

[Ramkumar Ramachandra]

Add an 'Invocation' section to specify what the command line arguments
mean. Also include a link to git-remote in the 'See Also' section.

Signed-off-by: Ramkumar Ramachandra <artagnon@gmail.com>
---
 Changes: The second argument may be an arbitrary string as
 Jonathan pointed out.

 Documentation/git-remote-helpers.txt |   34 +++++++++++++++++++++++++++++++++-
 1 files changed, 33 insertions(+), 1 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt
b/Documentation/git-remote-helpers.txt
index 7a5569c..b0e7d54 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -7,7 +7,7 @@ git-remote-helpers - Helper programs to interact with
remote repositories

 SYNOPSIS
 --------
-'git remote-<transport>' <remote>
+'git remote-<transport>' <repository> [<URL>]

 DESCRIPTION
 -----------
@@ -38,6 +38,34 @@ transport protocols, such as 'git-remote-http',
'git-remote-https',
 'git-remote-ftp' and 'git-remote-ftps'. They implement the capabilities
 'fetch', 'option', and 'push'.

+INVOCATION
+----------
+
+Remote helper programs are invoked with one or (optionally) two
+arguments. The first argument specifies a remote repository as in git;
+it is either the name of a configured remote or a URL. The second
+argument specifies a URL; it is usually of the form
+'<transport>://<address>', but any arbitrary string is possible.
+
+When git encounters a URL of the form '<transport>://<address>', where
+'<transport>' is a protocol that it cannot handle natively, it
+automatically invokes 'git remote-<transport>' with the full URL as
+the second argument. If such a URL is encountered directly on the
+command line, the first argument is the same as the second, and if it
+is encountered in a configured remote, the first argument is the name
+of that remote.
+
+A URL of the form '<transport>::<address>' explicitly instructs git to
+invoke 'git remote-<transport>' with '<address>' as the second
+argument. If such a URL is encountered directly on the command line,
+the first argument is '<address>', and if it is encountered in a
+configured remote, the first argument is the name of that remote.
+
+Additionally, when a configured remote has 'remote.<name>.vcs' set to
+'<transport>', git explicitly invokes 'git remote-<transport>' with
+'<name>' as the first argument. If set, the second argument is
+'remote.<name>.url'; otherwise, the second argument is omitted.
+
 COMMANDS
 --------

@@ -206,6 +234,10 @@ OPTIONS
 	must not rely on this option being set before
 	connect request occurs.

+SEE ALSO
+--------
+linkgit:git-remote[1]
+
 Documentation
 -------------
 Documentation by Daniel Barkalow and Ilari Liusvaara
-- 
1.7.0.3

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Ramkumar Ramachandra]

Hi Junio,

I see that you've applied v8 of my patch to pu, but there's a small
change in this revision:

diff --git a/Documentation/git-remote-helpers.txt
b/Documentation/git-remote-helpers.txt
index 8838388..6ffc0da 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -44,9 +44,8 @@ INVOCATION
 Remote helper programs are invoked with one or (optionally) two
 arguments. The first argument specifies a remote repository as in git;
 it is either the name of a configured remote or a URL. The second
-argument specifies a URL of the form '<transport>://<address>' or
-'<transport>::<address>', where '<address>' may be an arbitrary
-string.
+argument specifies a URL; it is usually of the form
+'<transport>://<address>', but any arbitrary string is possible.

 When git encounters a URL of the form '<transport>://<address>', where
 '<transport>' is a protocol that it cannot handle natively, it

-- Ram

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Junio C Hamano]

You keep sending a broken patch.  I've queued v7 and replaced with v8 and
both times I had to fix them in my editor.

I'm not replacing v8 with this tonight, as my pre-push test cycles are
already running (finished in one vm and continuing in another).  Please
check what I queued and make sure your future patches applies on top of
what I have.

Thanks.

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Ramkumar Ramachandra]

> You keep sending a broken patch.  I've queued v7 and replaced with v8 and
> both times I had to fix them in my editor.

Oh, thanks for telling me- I should make sure that my future patches
aren't broken.
Can someone tell me what I'm doing wrong? Earlier, Junio also had a
problem with one of my patches being corrupt. I used git-imap-send and
the GMail web interface to fill out the "to", "cc" and some comments.

> I'm not replacing v8 with this tonight, as my pre-push test cycles are
> already running (finished in one vm and continuing in another).

Okay, no issues.

> Please
> check what I queued and make sure your future patches applies on top of
> what I have.

Right, so you can just squash them. Also, I think I'm sending in
patches too fast so I'll slow down and rebase them on top of `pu`
every time.
Do tell me anything else that I can do to make my patches easier to
review and apply in future.

-- Ram

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Can someone tell me what I'm doing wrong? Earlier, Junio also had a
> problem with one of my patches being corrupt. I used git-imap-send and
> the GMail web interface to fill out the "to", "cc" and some comments.

In the above procedure, up to imap-send, everything would be fine, but I
heard GMail web interface still wraps lines before sending the result out.

Gmail section in Documentation/SubmittingPatches says:

  GMail does not appear to have any way to turn off line wrapping in the web
  interface, so this will mangle any emails that you send.  You can however
  use any IMAP email client to connect to the google imap server, and forward
  the emails through that.  Just make sure to disable line wrapping in that
  email client.  Alternatively, use "git send-email" instead.

This is a tangent, but I think this can still be further improved.  What
it says is not incorrect per-se, but the "can however" gives a false hope
to people who have been burned by the web interface.  The fact it lists
are:

 - GMail web interface will wrap lines and corrupt patches, no matter what
   you do;

 - With imap-send, you can stuff messages to your outbox at GMail, and
   using IMAP client that does _not_ corrupt messages, you can send things
   out.

Combined together, these mean that you lose even if you preprare your
outbox with imap-send if you send out the end result from their web
interface.

At the minimum, we could add a parenthesized comment like this.

  the emails through that.  Just make sure to disable line wrapping in
  that email client (again, there is no way to tell GMail web interface to
  do so).

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Ramkumar Ramachandra]

>  - GMail web interface will wrap lines and corrupt patches, no matter what
>   you do;
>
>  - With imap-send, you can stuff messages to your outbox at GMail, and
>   using IMAP client that does _not_ corrupt messages, you can send things
>   out.

Actually the last paragraph about "git imap-send" clearly states that
the web interface _can_ be used. I just used another email client to
find out when lines break and when they don't- I've accordingly
written a patch for Documentation/SubmittingPatches clarifying this.

-- Ram

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> Right, so you can just squash them. Also, I think I'm sending in
> patches too fast so I'll slow down and rebase them on top of `pu`
> every time.

Thank you for this.  FWIW I find it useful when discussing a piece of
code or documentation to bounce back and forth multiple versions of
small fragments.  Then once the discussion has settled down, it can be
useful to see the big picture again, with the new changes
incorporated.

If you want to make sure the latest version of a patch is always
available, that is a noble goal, too, but I think a frequently-
rebased public branch for your patch series is a better way to achieve
that.

Unrelated: in case you are interested, the git-resurrect.sh script
from contrib can extract topic branches from pu, if you want to see
Junio’s topic branch for your patch.

Cheers,
Jonathan

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Junio C Hamano]

Jonathan Nieder <jrnieder@gmail.com> writes:

> If you want to make sure the latest version of a patch is always
> available, that is a noble goal, too, but I think a frequently-
> rebased public branch for your patch series is a better way to achieve
> that.
>
> Unrelated: in case you are interested, the git-resurrect.sh script
> from contrib can extract topic branches from pu, if you want to see
> Junio’s topic branch for your patch.

It might look unrelated, but I found it the most helpful information in
your message ;-) I really hate it when people base their patches on 'pu'.

At least it would be helpful for me if people based their follow-up
patches on top of their own topics.

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Ramkumar Ramachandra]

Hi,

On Thu, Apr 8, 2010 at 4:19 AM, Jonathan Nieder <jrnieder@gmail.com> wrote:
> Thank you for this.  FWIW I find it useful when discussing a piece of
> code or documentation to bounce back and forth multiple versions of
> small fragments.  Then once the discussion has settled down, it can be
> useful to see the big picture again, with the new changes
> incorporated.

Got it. In future, I'll post diffs for smaller changes hopefully in
the same email thread, and complete patches for larger changes in a
separate email thread. I've been quite haphazard in the past.

> If you want to make sure the latest version of a patch is always
> available, that is a noble goal, too, but I think a frequently-
> rebased public branch for your patch series is a better way to achieve
> that.

Ah, I already do that, but my branch is based on `pu`. What should I do?
http://github.com/artagnon/git

On Thu, Apr 8, 2010 at 4:32 AM, Junio C Hamano <gitster@pobox.com> wrote:
> At least it would be helpful for me if people based their follow-up
> patches on top of their own topics.

Can I see the topic branch corresponding to my patches? If this is
possible, I can avoid the more painful procedure of extracting it from
the latest pu using git-resurrect.sh.

-- Ram

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Jonathan Nieder]

Ramkumar Ramachandra wrote:

> Can I see the topic branch corresponding to my patches? If this is
> possible, I can avoid the more painful procedure of extracting it from
> the latest pu using git-resurrect.sh.

I tend to do the extraction by hand:

 1. git log --first-parent origin/pu
 2. Search for "rr/".
 3. git branch -f rr/whatever <relevant sha1>

Maybe it would be nice to have a script that does just that instead of
the more costly "git resurrect -m rr/whatever".

Cheers,
Jonathan

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Junio C Hamano]

Ramkumar Ramachandra <artagnon@gmail.com> writes:

> Can I see the topic branch corresponding to my patches? If this is
> possible, I can avoid the more painful procedure of extracting it from
> the latest pu using git-resurrect.sh.

Run:

    $ git log --first-parent --oneline origin/master..origin/pu

and find "Merge branch 'rr/remote-helper-doc'" in the output.  The second
parent of that merge commit (e.g. 6144af5^2 == ad466d1) is the tip of your
topic.  You can verify it with:

    $ git log -p origin/master..ad466d1
    $ git checkout -b remote-helper-doc ad466d1
    ... build further on it ...

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Ramkumar Ramachandra]

Hi,

>    $ git log --first-parent --oneline origin/master..origin/pu
>
> and find "Merge branch 'rr/remote-helper-doc'" in the output.  The second
> parent of that merge commit (e.g. 6144af5^2 == ad466d1) is the tip of your
> topic.  You can verify it with:
>
>    $ git log -p origin/master..ad466d1
>    $ git checkout -b remote-helper-doc ad466d1
>    ... build further on it ...

Thanks for the excellent instructions. I'm just curious: why didn't
you include these instructions in your rewrite of SubmittingPatches?

-- Ram

Re: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Sverre Rabbelier]

Heya,

On Thu, Apr 8, 2010 at 01:02, Junio C Hamano <gitster@pobox.com> wrote:
> It might look unrelated, but I found it the most helpful information in
> your message ;-) I really hate it when people base their patches on 'pu'.
>
> At least it would be helpful for me if people based their follow-up
> patches on top of their own topics.

Hmmm, perhaps we should update SubmittingPatches to say something
about that? The section that talks about what to base your patch
against [0] is not very explicit in that aspect.

[0] http://git.kernel.org/?p=git/git.git;a=blob;f=Documentation/SubmittingPatches#l106

-- 
Cheers,

Sverre Rabbelier

Re*: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> Hmmm, perhaps we should update SubmittingPatches to say something
> about that? The section that talks about what to base your patch
> against is not very explicit in that aspect.

Ok, here is a rough draft.

 Documentation/SubmittingPatches |   52 ++++++++++++++++++++++++++++++--------
 1 files changed, 41 insertions(+), 11 deletions(-)

diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
index c686f86..1d403ee 100644
--- a/Documentation/SubmittingPatches
+++ b/Documentation/SubmittingPatches
@@ -53,6 +53,37 @@ But the patch submission requirements are a lot more relaxed
 here on the technical/contents front, because the core GIT is
 thousand times smaller ;-).  So here is only the relevant bits.
 
+(0) Decide what to base your work on.
+
+The general principle is always to base your work on the oldest branch
+that your change is relevant to.
+
+ - A fix for a bug that has been with git from older releases should be
+   included in both the upcoming feature release and the current
+   maintenance release.  Try to base your work on the 'maint' branch.  A
+   work to kill a bug that is in 'master' but not in 'maint' should be
+   based on 'master'.
+
+ - A fix for a bug that is not yet in 'master' is the best bug to kill.
+   If you can find the topic that introduces the regression, base your
+   work on the tip of the topic.  "log --first-parent master..pu" would be
+   a good way to find the tips of topic branches.
+
+ - A new feature should be based on the 'master' branch in general.
+
+ - If your new feature depends on some other topics that are not in
+   'master' yet, and if it relies only on one topic, base your work on the
+   tip of that topic.  If it depends on too many topics that are not in
+   'master', you can privately start working on 'next' or even 'pu' and
+   send out your patches for discussion, but it is possible that your
+   maintainer may ask you to wait and rebase your changes on 'master'
+   after some of the larger topics your topic depends on graduate to
+   'master'.
+
+ - Base corrections and enhancements on a topic that are not in 'master'
+   yet but already merged to 'next' on the tip of the topic.  If the topic
+   has not been merged to 'next', it is Ok to add a note that the patch is
+   a trivial fix and can be squashed into the series.
 
 (1) Make separate commits for logically separate changes.
 
@@ -170,17 +201,16 @@ patch, format it as "multipart/signed", not a text/plain message
 that starts with '-----BEGIN PGP SIGNED MESSAGE-----'.  That is
 not a text/plain, it's something else.
 
-Note that your maintainer does not necessarily read everything
-on the git mailing list.  If your patch is for discussion first,
-send it "To:" the mailing list, and optionally "cc:" him.  If it
-is trivially correct or after the list reached a consensus, send
-it "To:" the maintainer and optionally "cc:" the list for
-inclusion.
-
-Also note that your maintainer does not actively involve himself in
-maintaining what are in contrib/ hierarchy.  When you send fixes and
-enhancements to them, do not forget to "cc: " the person who primarily
-worked on that hierarchy in contrib/.
+Unless your patch is a very trivial and an obviously correct one,
+first send it with "To:" set to the mailing list, with "cc:" listing
+people who are involved in the area you are touching (the output from
+"git blame $path" and "git shortlog --no-merges $path" would help to
+identify them), to solicit comments and reviews.  After the list
+reached a consensus that it is a good idea to apply the patch, re-send
+it with "To:" set to the maintainer and optionally "cc:" the list for
+inclusion.  Do not forget to add trailers such as "Acked-by:",
+"Reviewed-by:" and "Tested-by:" after your "Signed-off-by:" line as
+necessary.
 
 
 (4) Sign your work

Re: Re*: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Sverre Rabbelier]

Heya,

On Thu, Apr 8, 2010 at 22:01, Junio C Hamano <gitster@pobox.com> wrote:
> Ok, here is a rough draft.

I like it, but...

> -Note that your maintainer does not necessarily read everything
> -on the git mailing list.

> -Also note that your maintainer does not actively involve himself in
> -maintaining what are in contrib/ hierarchy.

... I'm curious as to why you removed these two statements.

-- 
Cheers,

Sverre Rabbelier

Re: Re*: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Junio C Hamano]

Sverre Rabbelier <srabbelier@gmail.com> writes:

> I like it, but...
>
>> -Note that your maintainer does not necessarily read everything
>> -on the git mailing list.
>
>> -Also note that your maintainer does not actively involve himself in
>> -maintaining what are in contrib/ hierarchy.
>
> ... I'm curious as to why you removed these two statements.

The rewritten paragraph already covers these two points, no?  Proposals
can be cc:ed to me only if what I did in the past is relevant to the
proposal, meaning "treat the maintainer just like any other contributors
and reviewers", and it applies to all the codebase, not just the contrib/
area.

Re: Re*: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Michael J Gruber]

Junio C Hamano venit, vidit, dixit 09.04.2010 04:01:
> Sverre Rabbelier <srabbelier@gmail.com> writes:
> 
>> Hmmm, perhaps we should update SubmittingPatches to say something
>> about that? The section that talks about what to base your patch
>> against is not very explicit in that aspect.
> 
> Ok, here is a rough draft.
> 
>  Documentation/SubmittingPatches |   52 ++++++++++++++++++++++++++++++--------
>  1 files changed, 41 insertions(+), 11 deletions(-)
> 
> diff --git a/Documentation/SubmittingPatches b/Documentation/SubmittingPatches
> index c686f86..1d403ee 100644
> --- a/Documentation/SubmittingPatches
> +++ b/Documentation/SubmittingPatches
> @@ -53,6 +53,37 @@ But the patch submission requirements are a lot more relaxed
>  here on the technical/contents front, because the core GIT is
>  thousand times smaller ;-).  So here is only the relevant bits.
>  
> +(0) Decide what to base your work on.
> +
> +The general principle is always to base your work on the oldest branch
> +that your change is relevant to.
> +
> + - A fix for a bug that has been with git from older releases should be
> +   included in both the upcoming feature release and the current
> +   maintenance release.  Try to base your work on the 'maint' branch.  A
> +   work to kill a bug that is in 'master' but not in 'maint' should be
> +   based on 'master'.
> +
> + - A fix for a bug that is not yet in 'master' is the best bug to kill.
> +   If you can find the topic that introduces the regression, base your
> +   work on the tip of the topic.  "log --first-parent master..pu" would be
> +   a good way to find the tips of topic branches.
> +
> + - A new feature should be based on the 'master' branch in general.
> +
> + - If your new feature depends on some other topics that are not in
> +   'master' yet, and if it relies only on one topic, base your work on the
> +   tip of that topic.  If it depends on too many topics that are not in
> +   'master', you can privately start working on 'next' or even 'pu' and
> +   send out your patches for discussion, but it is possible that your
> +   maintainer may ask you to wait and rebase your changes on 'master'
> +   after some of the larger topics your topic depends on graduate to
> +   'master'.
> +
> + - Base corrections and enhancements on a topic that are not in 'master'
> +   yet but already merged to 'next' on the tip of the topic.  If the topic
> +   has not been merged to 'next', it is Ok to add a note that the patch is
> +   a trivial fix and can be squashed into the series.
>  
>  (1) Make separate commits for logically separate changes.
>  
> @@ -170,17 +201,16 @@ patch, format it as "multipart/signed", not a text/plain message
>  that starts with '-----BEGIN PGP SIGNED MESSAGE-----'.  That is
>  not a text/plain, it's something else.
>  
> -Note that your maintainer does not necessarily read everything
> -on the git mailing list.  If your patch is for discussion first,
> -send it "To:" the mailing list, and optionally "cc:" him.  If it
> -is trivially correct or after the list reached a consensus, send
> -it "To:" the maintainer and optionally "cc:" the list for
> -inclusion.
> -
> -Also note that your maintainer does not actively involve himself in
> -maintaining what are in contrib/ hierarchy.  When you send fixes and
> -enhancements to them, do not forget to "cc: " the person who primarily
> -worked on that hierarchy in contrib/.
> +Unless your patch is a very trivial and an obviously correct one,
> +first send it with "To:" set to the mailing list, with "cc:" listing
> +people who are involved in the area you are touching (the output from
> +"git blame $path" and "git shortlog --no-merges $path" would help to
> +identify them), to solicit comments and reviews.  After the list
> +reached a consensus that it is a good idea to apply the patch, re-send
> +it with "To:" set to the maintainer and optionally "cc:" the list for
> +inclusion.  Do not forget to add trailers such as "Acked-by:",
> +"Reviewed-by:" and "Tested-by:" after your "Signed-off-by:" line as
> +necessary.

I'm wondering how necessary that flipping of to and cc is. It means one
has to switch one's send-email config between RFCs and actual patches.
It also means I should send fewer patches to you (Junio) directly (in
addition to cc'ing the list), which is probably the intention :)
OK, I've learned about aliasesfile (and wondered about the different
wording compared to aliasfiletype) meanwhile, so no problem...

>  
>  
>  (4) Sign your work

Re: Re*: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Junio C Hamano]

Michael J Gruber <git@drmicha.warpmail.net> writes:

> I'm wondering how necessary that flipping of to and cc is. It means one
> has to switch one's send-email config between RFCs and actual patches.

This was my thinko.  "An ideal patch flow" says the final submission still
goes to the list with maintainer on the Cc: line, and that is fine with
me.

> It also means I should send fewer patches to you (Junio) directly (in
> addition to cc'ing the list), which is probably the intention :)

That, and more importantly, less uncooked patches hitting my mailbox with
me as an addressee, was an important goal.  I need to deal with patch
e-mails in three different ways:

 (0) just read as a bystander without much interesting input from my side;
 (1) read and comment for improvement; or
 (2) act on it by applying.

I was hoping if we can have some way for me to sift (2) from others
without adding extra burden on the contributors.

One way to reduce (1) I've been experimenting with is not to comment on
too many threads.  The theory is that the initial rfc patch hopefully does
not have my address on To/Cc, but once I comment on a patch, follow-up
patches will be posted with address of everybody involved in the
discussion Cc:ed to the thread (which by the way is a good practice and I
do not want people to break it), and my "grep for patch messages with my
address on it" trick to find the finalized patches will not work well.

But that strategy does not work very well for another reason: we lose one
reviewer if I try to comment on patches as little as possible, and we do
not have enough people who read other's patches and help polishing to
afford that.

Re: Re*: [PATCH v9] Documentation/remote-helpers: Add invocation section

[Ramkumar Ramachandra]

Hi,

I like the idea, but I think it's too verbose. Perhaps something along
these lines?

1. In general, base your patches on 'master'.
2. If your patch is a bugfix try to base it on 'maint'. If the bug's
in 'master, but not in 'maint' also, base it on 'master'.
3. If you're working on a feature, some of whose patches are already
in 'pu', base your work on the tip of the last commit in your topic
branch (See THIS). If it's a minor correction, you might want to add a
note asking the maintainer to squash it into the previous commit.
4. If you're working on an elaborate feature that depends on many
commits in 'pu', maintain a public branch based on 'pu', and
periodically post patches to the mailing list for feedback (all based
on 'pu'). You might have to rebase/ wait before it's finally merged.

THIS:
> +   work on the tip of the topic.  "log --first-parent master..pu" would be
> +   a good way to find the tips of topic branches.

+ "The grandparent of this merge commit is your latest commit in this
topic. This is the commit you should base your patch on."

-- Ram