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

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

[Jonathan Nieder]

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 <jrnieder@gmail.com>
---
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

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

[Sverre Rabbelier]

Heya,

On Fri, Mar 18, 2011 at 18:45, Jonathan Nieder <jrnieder@gmail.com> 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

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

[Jonathan Nieder]

Sverre Rabbelier wrote:
> On Fri, Mar 18, 2011 at 18:45, Jonathan Nieder <jrnieder@gmail.com> 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
> ;).

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 :)).

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

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

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

[Sverre Rabbelier]

Heya,

On Fri, Mar 18, 2011 at 23:21, Jonathan Nieder <jrnieder@gmail.com> 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

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

[Jonathan Nieder]

Sverre Rabbelier wrote:
> On Fri, Mar 18, 2011 at 23:21, Jonathan Nieder <jrnieder@gmail.com> 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?

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.

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

[Jonathan Nieder]

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 <jrnieder@gmail.com>
---
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