[RFC/PATCH] Documentation/remote-helpers: explain capabilities first

The current remote helper documentation is from the perspective of
git, so to speak: it presents a full menu of commands for a person
invoking a remote helper to choose from.  In practice, that's less
useful than it could be, since the daunted novice remote-helper author
probably just wanted a list of commands she needs to implement to get
started.  So preface the command list with an overview of each
capability, its purpose, and what commands it requires.

As a side effect, this makes it a little clearer that git doesn't just
choose arbitrary commands to run, even if the remote helper advertises
all capabilities --- instead, there are well defined command sequences
for various tasks.

Signed-off-by: Jonathan Nieder <redacted>
---
No documentation for "export" and "gitdir" yet (by the way: would it
make sense to export the GIT_DIR environment variable so a round trip
for the 'gitdir' capability would eventually be unnecessary?).

The current documentation left me lost, so I wrote this (which still
probably leaves one lost).  Thoughts?

 Documentation/git-remote-helpers.txt |  101 +++++++++++++++++++++++++--------
 1 files changed, 76 insertions(+), 25 deletions(-)

diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 87cd11f..e7a3985 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt

@@ -29,16 +29,87 @@ commands generally concern facilities like discovering and updating
 remote refs, transporting objects between the object database and
 the remote repository, and updating the local object store.
 
-Helpers supporting the 'fetch' capability can discover refs from the
-remote repository and transfer objects reachable from those refs to
-the local object store. Helpers supporting the 'push' capability can
-transfer local objects to the remote repository and update remote refs.
-
 Git comes with a "curl" family of remote helpers, that handle various
 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'.
 
+CAPABILITIES
+------------
+
+Different remote helpers expect to communicate with git in different
+ways.  These are declared to git through the helper's response to the
+`capabilities` command (see COMMANDS, below).
+
+'option'::
+	Can accept settings like `verbosity` (how much output to write
+	to stderr) and `depth` (how much history is wanted in the case
+	of a shallow clone) that affect how other commands are carried
+	out.
++
+Supported commands: 'option'.
+
+Capabilities for Pushing
+~~~~~~~~~~~~~~~~~~~~~~~~
+'connect'::
+	Can attempt to connect to 'git receive-pack' (for pushing),
+	'git upload-pack', etc for communication using the
+	packfile protocol.
++
+Supported commands: 'connect'.
+
+'push'::
+	Can discover remote refs and push local commits and the
+	history leading up to them to new or existing remote refs.
++
+Supported commands: 'list for-push', 'push'.
+
+If a helper advertises both 'connect' and 'push', git will use
+'connect' if possible and fall back to 'push' if the helper requests
+so when connecting (see the 'connect' command under COMMANDS).
+
+Capabilities for Fetching
+~~~~~~~~~~~~~~~~~~~~~~~~~
+'connect'::
+	Can try to connect to 'git upload-pack' (for fetching),
+	'git receive-pack', etc for communication using the
+	packfile protocol.
++
+Supported commands: 'connect'.
+
+'fetch'::
+	Can discover remote refs and transfer objects reachable from
+	them to the local object store.
++
+Supported commands: 'list', 'fetch'.
+
+'import'::
+	Can discover remote refs and output objects reachable from
+	them as a stream in fast-import format.
++
+Supported commands: 'list', 'import'.
+
+If a helper advertises 'connect', git will use it if possible and
+fall back to another capability if the helper requests so when
+connecting (see the 'connect' command under COMMANDS).
+When choosing between 'fetch' and 'import', git prefers 'fetch'.
+Other frontends may have some other order of preference.
+
+'refspec' <refspec>::
+	This modifies the 'import' capability.
++
+A helper advertising
+`refspec refs/heads/{asterisk}:refs/svn/origin/branches/{asterisk}`
+in its capabilities is saying that, when it handles
+`import refs/heads/topic`, the stream it outputs will update the
+`refs/svn/origin/branches/topic` ref.
++
+This capability can be advertised multiple times.  The first
+applicable refspec takes precedence.  The left-hand of refspecs
+advertised with this capability must cover all refs reported by
+the list command.  If no 'refspec' capability is advertised,
+there is an implied `refspec {asterisk}:{asterisk}`.
+
 INVOCATION
 ----------
 

@@ -167,26 +238,6 @@ completing a valid response for the current command.
 Additional commands may be supported, as may be determined from
 capabilities reported by the helper.
 
-CAPABILITIES
-------------
-
-'fetch'::
-'option'::
-'push'::
-'import'::
-'connect'::
-	This helper supports the corresponding command with the same name.
-
-'refspec' 'spec'::
-	When using the import command, expect the source ref to have
-	been written to the destination ref. The earliest applicable
-	refspec takes precedence. For example
-	"refs/heads/*:refs/svn/origin/branches/*" means that, after an
-	"import refs/heads/name", the script has written to
-	refs/svn/origin/branches/name. If this capability is used at
-	all, it must cover all refs reported by the list command; if
-	it is not used, it is effectively "*:*"
-
 REF LIST ATTRIBUTES
 -------------------
 

-- 
1.7.4.1

Heya,

On Fri, Mar 18, 2011 at 18:45, Jonathan Nieder [off-list ref] wrote:

No documentation for "export" ... yet

Which is for the better, since I think the current interface is
broken. It'll be easier to justify changing it if it's not documented
;).

(by the way: would it
make sense to export the GIT_DIR environment variable so a round trip
for the 'gitdir' capability would eventually be unnecessary?).

Yes, that makes a lot of sense.

The current documentation left me lost, so I wrote this (which still
probably leaves one lost).  Thoughts?

I think it makes sense.

-- 
Cheers,

Sverre Rabbelier

Sverre Rabbelier wrote:

On Fri, Mar 18, 2011 at 18:45, Jonathan Nieder [off-list ref] wrote:

quoted

No documentation for "export" ... yet

Which is for the better, since I think the current interface is
broken. It'll be easier to justify changing it if it's not documented
;).

Thanks for explaining.  FWIW I'd be interested in your thoughts on this
sometime (not now necessarily, just some time before the svn push
support project is on the front burner :)).

quoted

(by the way: would it
make sense to export the GIT_DIR environment variable so a round trip
for the 'gitdir' capability would eventually be unnecessary?).

Yes, that makes a lot of sense.

quoted

The current documentation left me lost, so I wrote this (which still
probably leaves one lost).  Thoughts?

I think it makes sense.

Glad to hear it.  (The GIT_DIR idea is Tom's iirc.)

Heya,

On Fri, Mar 18, 2011 at 23:21, Jonathan Nieder [off-list ref] wrote:

Thanks for explaining.  FWIW I'd be interested in your thoughts on this
sometime (not now necessarily, just some time before the svn push
support project is on the front burner :)).

Any idea on when that will be? I've been wanting to finish push
support for git-remote-hg for ages now, but haven't been able to make
the time :(.

-- 
Cheers,

Sverre Rabbelier

Sverre Rabbelier wrote:

On Fri, Mar 18, 2011 at 23:21, Jonathan Nieder [off-list ref] wrote:

quoted

Thanks for explaining.  FWIW I'd be interested in your thoughts on this
sometime (not now necessarily, just some time before the svn push
support project is on the front burner :)).

Any idea on when that will be?

No, not sure.  My current priorities are

 - getting "git clone svndump-alpha::/home/jrn/dump/svn-kde0:1200000"
   to work;
 - mustering up the courage to clean up glibc vfprintf

After that, who knows?

But I hear that Ram has done some work on a fast-import to dumpfile
converter, so push support is definitely simmering even if there is
not much momentum yet.

Hi again,

Sorry for the spam.  Just ran into a quick puzzle:

Jonathan Nieder wrote:

+'push'::
+	Can discover remote refs and push local commits and the
+	history leading up to them to new or existing remote refs.
++
+Supported commands: 'list for-push', 'push'.

[...]

+'fetch'::
+	Can discover remote refs and transfer objects reachable from
+	them to the local object store.
++
+Supported commands: 'list', 'fetch'.
+
+'import'::
+	Can discover remote refs and output objects reachable from
+	them as a stream in fast-import format.
++
+Supported commands: 'list', 'import'.

In the actual code:

 * "list for-push" is used if:
    - we are scouting out for "refs in common" in preparation for a push;
    - the "push" capability has been advertised.
 * "list" is used in all other cases.
 * in particular, even if no capabilities are advertised, the "list"
   command will be used.

It's been this way since v1.6.6-rc0~22^2~26 (2009-10-30).  Anyway, the
rule doesn't sound very principled --- if the "push" capability is not
advertised, isn't the push going to error out anyway?  Why fall back
to a normal "list" which is likely not to work, either?

One might suggest:

A.
 - if scouting for "refs in common", use "list for-push";
 - otherwise, use "list" without for-push;
 - in all cases, check for an appropriate capability first
   (push/export in the for-push case, fetch/import otherwise)

At first it sounds ok but this one makes for a lousy story when new
capabilities are invented.  If "git remote-helper" only supports
fetching with a "better-import" capability that my copy of git does
not support, I will still try "git ls-remote helper::url" to learn
what I am missing.

B.
 - if scouting for "refs in common", use "list for-push";
 - otherwise, use "list" without for-push;
 - all remote helpers must implement both "list" and "list for-push".

This rule sounds better, and it doesn't seem to break remote-testgit
(which currently only receives "list" commands without "for-push"
because it only advertises "export" and not "push").  What do you
think?

Signed-off-by: Jonathan Nieder <redacted>
---
Probably I confused myself too far after reading in the manpage that
for-push is an attribute.  Note to self: it isn't.

 transport-helper.c |    2 +-
 1 files changed, 1 insertions(+), 1 deletions(-)

diff --git a/transport-helper.c b/transport-helper.c
index 0c5b1bd..6d3d15e 100644
--- a/transport-helper.c
+++ b/transport-helper.c

@@ -802,7 +802,7 @@ static struct ref *get_refs_list(struct transport *transport, int for_push)
 		return transport->get_refs_list(transport, for_push);
 	}
 
-	if (data->push && for_push)
+	if (for_push)
 		write_str_in_full(helper->in, "list for-push\n");
 	else
 		write_str_in_full(helper->in, "list\n");

-- 
1.7.4.1