threads / patch / 32212

patch, 6 partsDocument missing remote helper capabilities

Subject: [PATCH 2/6] Document missing remote helper capabilities

## tl;dr

16 messages between Nov 27, 2012 and Dec 12, 2012. Diffs are folded; open one to read it.

replies: 15people: 4as markdown or json

Max Horn· Nov 27, 2012, 17:44 UTC · lore

[PATCH 0/6] Improve remote helper documentation

Various remote helper capabilities and commands were not documented, in particular 'export', or documented in a misleading way (e.g. 'for-push' was listed as a ref attribute understood by git, which is not the case). This patch series changes that, and also address some other things in the remote helper documentation that I found jarring when reading through it.

Note that the description of export and (im|ex)port-marks probably can be improved, and I hope that somebody who knows more about them than me and/or is better at writing documentation will do just that. But I felt it was better to provide something than to do nothing and only complain, as I did previously on this subject ;-).

Max Horn (6):
  Document invocation first, then input format
  Document missing remote helper capabilities
  Fix grammar
  Rearrange the description of remote helper capabilities
  Make clearer which commands must be supported for which capabilities
  Remove 'for-push' from ref list attributes list, link to subsections
 Documentation/git-remote-helpers.txt | 241 ++++++++++++++++++++---------------
 1 file changed, 136 insertions(+), 105 deletions(-)
-- 
1.8.0.393.gcc9701d
Max Horn· Nov 27, 2012, 17:44 UTC · re: Max Horn · lore

[PATCH 1/6] Document invocation first, then input format

In the distant past, the order was 'Invocation', 'Commands', 'Capabilities', ...

Then it was decided that before giving a list of Commands, there should be an overall description of the 'Input format', which was a wise decision. However, this description was put as the very first thing, with the rationale that any implementor would want to know that first.

However, it seems an implementor would actually first need to know how the remote helper will be invoked, so moving 'Invocation' to the front again seems logical. Moreover, we now don't switch from discussing the input format to the invocation style and then back to input related stuff.

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 62 ++++++++++++++++++------------------
 1 file changed, 31 insertions(+), 31 deletions(-)
Show changes to Documentation/git-remote-helpers.txt +31 −30
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 5ce4cda..9a7e583 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -35,6 +35,37 @@ 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.
+The 'GIT_DIR' environment variable is set up for the remote helper
+and can be used to determine where to store additional data or from
+which directory to invoke auxiliary git commands.
+
+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.
+
 INPUT FORMAT
 ------------
 
@@ -173,37 +204,6 @@ advertised with this capability must cover all refs reported by
 the list command.  If no 'refspec' capability is advertised,
 there is an implied `refspec *:*`.
 
-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.
-The 'GIT_DIR' environment variable is set up for the remote helper
-and can be used to determine where to store additional data or from
-which directory to invoke auxiliary git commands.
-
-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
 --------
 
-- 
1.8.0.393.gcc9701d
Max Horn· Nov 27, 2012, 17:44 UTC · re: Max Horn · lore

The 'export' and '(im|ex)port-marks' capabilities were not documented at all

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 45 +++++++++++++++++++++++++++++++++---
 1 file changed, 42 insertions(+), 3 deletions(-)
Show changes to Documentation/git-remote-helpers.txt +42 −3
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 9a7e583..db63541 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -106,6 +106,10 @@ to the `capabilities` command (see COMMANDS, below).
 	For listing remote refs and fetching the associated history to
 	the local object store.
 
+'export'::
+	For listing remote refs and pushing specified objects from a
+	fast-import stream to remote refs.
+
 'import'::
 	For listing remote refs and fetching the associated history as
 	a fast-import stream.
@@ -143,6 +147,16 @@ there is an implied `refspec *:*`.
 	This is to prevent mixing commands and fast-import responses on the
 	helper's stdin.
 
+'export-marks' <file>::
+	This modifies the 'export' capability, instructing git to dump the
+	internal marks table to <file> when complete. For details,
+	read up on '--export-marks=<file>' in linkgit:git-fast-export[1].
+
+'import-marks' <file>::
+	This modifies the 'export' capability, instructing git to load the
+	marks specified in <file> before processing any input. For details,
+	read up on '--import-marks=<file>' in linkgit:git-fast-export[1].
+
 Capabilities for Pushing
 ~~~~~~~~~~~~~~~~~~~~~~~~
 'connect'::
@@ -158,9 +172,18 @@ Supported commands: 'connect'.
 +
 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).
+'export'::
+	Can discover remote refs and push specified objects from a
+	fast-import stream to remote refs.
++
+Supported commands: 'list for-push', 'export'.
+
+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 'push' and 'export', git prefers 'push'.
+Other frontends may have some other order of preference.
+
 
 Capabilities for Fetching
 ~~~~~~~~~~~~~~~~~~~~~~~~~
@@ -307,6 +330,22 @@ stdin.
 +
 Supported if the helper has the 'import' capability.
 
+'export'::
+	Instructs the remote helper that any subsequent input is
+	part of a fast-import stream (generated by 'git fast-export')
+	containing objects which should be pushed to the remote.
++
+Especially useful for interoperability with a foreign versioning
+system.
++
+The 'export-marks' and 'import-marks' capabilities, if specified,
+affect this command in so far as they are passed on to 'git
+fast-export', which then will load/store a table of marks for
+local objects. This can be used to implement for incremental
+operations.
++
+Supported if the helper has the 'export' capability.
+
 'connect' <service>::
 	Connects to given service. Standard input and standard output
 	of helper are connected to specified service (git prefix is
-- 
1.8.0.393.gcc9701d
Max Horn· Nov 27, 2012, 17:44 UTC · re: Max Horn · lore

[PATCH 3/6] Fix grammar

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 6 +++---
 1 file changed, 3 insertions(+), 3 deletions(-)
Show changes to Documentation/git-remote-helpers.txt +3 −3
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index db63541..7eb43d7 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -235,9 +235,9 @@ Commands are given by the caller on the helper's standard input, one per line.
 'capabilities'::
 	Lists the capabilities of the helper, one per line, ending
 	with a blank line. Each capability may be preceded with '*',
-	which marks them mandatory for git version using the remote
-	helper to understand (unknown mandatory capability is fatal
-	error).
+	which marks them mandatory for git versions using the remote
+	helper to understand. Any unknown mandatory capability is a
+	fatal error.
 
 'list'::
 	Lists the refs, one per line, in the format "<value> <name>
-- 
1.8.0.393.gcc9701d
Junio C Hamano· Nov 27, 2012, 20:39 UTC · re: Max Horn · lore

Re: [PATCH 3/6] Fix grammar

Max Horn <max@quendi.de> writes:
> Subject: Re: [PATCH 3/6] Fix grammar

Please run "git shortlog -200 --no-merges" from the tip of your topic branch before sending a series out, and see if you can immediately identify what area each of your patches affects.

Show 22 quoted lines
> Signed-off-by: Max Horn <max@quendi.de>
> ---
>  Documentation/git-remote-helpers.txt | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
> index db63541..7eb43d7 100644
> --- a/Documentation/git-remote-helpers.txt
> +++ b/Documentation/git-remote-helpers.txt
> @@ -235,9 +235,9 @@ Commands are given by the caller on the helper's standard input, one per line.
>  'capabilities'::
>  	Lists the capabilities of the helper, one per line, ending
>  	with a blank line. Each capability may be preceded with '*',
> -	which marks them mandatory for git version using the remote
> -	helper to understand (unknown mandatory capability is fatal
> -	error).
> +	which marks them mandatory for git versions using the remote
> +	helper to understand. Any unknown mandatory capability is a
> +	fatal error.
>  
>  'list'::
>  	Lists the refs, one per line, in the format "<value> <name>
Max Horn· Nov 27, 2012, 21:23 UTC · re: Junio C Hamano · lore

Re: [PATCH 3/6] Fix grammar

On 27.11.2012, at 21:39, Junio C Hamano wrote:
Show 7 quoted lines
> Max Horn <max@quendi.de> writes:
> 
>> Subject: Re: [PATCH 3/6] Fix grammar
> 
> Please run "git shortlog -200 --no-merges" from the tip of your
> topic branch before sending a series out, and see if you can
> immediately identify what area each of your patches affects.
First off: I apologize for the inconvenience my stumbling causes :-(, and I'll try to learn and do better next time I send this or another series.
In this particular case, I am not 100% sure if I understood you correctly, i.e. what exactly you are trying to tell me. Is it (in a nutshell) that the subject lines of my commits suck, as they don't identify which area of code they touch? At least that's the thing I notice when looking at that shortlog... Bad, of course...
If this is indeed it, would a commit message like
   git-remote-helper.txt: minor grammar fix
be OK? If this is indeed it, I'll be happy to reroll and resubmit the series accordingly. If I am mistaken and something else / more is wrong, please be so kind and let me know, too.

Sorry once more and thank you very much for your feedback, Max

Show 28 quoted lines
> 
>> Signed-off-by: Max Horn <max@quendi.de>
>> ---
>> Documentation/git-remote-helpers.txt | 6 +++---
>> 1 file changed, 3 insertions(+), 3 deletions(-)
>> 
>> diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
>> index db63541..7eb43d7 100644
>> --- a/Documentation/git-remote-helpers.txt
>> +++ b/Documentation/git-remote-helpers.txt
>> @@ -235,9 +235,9 @@ Commands are given by the caller on the helper's standard input, one per line.
>> 'capabilities'::
>> 	Lists the capabilities of the helper, one per line, ending
>> 	with a blank line. Each capability may be preceded with '*',
>> -	which marks them mandatory for git version using the remote
>> -	helper to understand (unknown mandatory capability is fatal
>> -	error).
>> +	which marks them mandatory for git versions using the remote
>> +	helper to understand. Any unknown mandatory capability is a
>> +	fatal error.
>> 
>> 'list'::
>> 	Lists the refs, one per line, in the format "<value> <name>
> --
> To unsubscribe from this list: send the line "unsubscribe git" in
> the body of a message to majordomo@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
> 
Max Horn· Nov 27, 2012, 17:44 UTC · re: Max Horn · lore

[PATCH 4/6] Rearrange the description of remote helper capabilities

This also remove some duplication in the descriptions (e.g. refspec was explained twice with similar level of detail)

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 134 +++++++++++++++--------------------
 1 file changed, 56 insertions(+), 78 deletions(-)
Show changes to Documentation/git-remote-helpers.txt +56 −78
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 7eb43d7..7ac1461 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -88,81 +88,17 @@ Each remote helper is expected to support only a subset of commands.
 The operations a helper supports are declared to git in the response
 to the `capabilities` command (see COMMANDS, below).
 
-'option'::
-	For specifying 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.
-
-'connect'::
-	For fetching and pushing using git's native packfile protocol
-	that requires a bidirectional, full-duplex connection.
-
-'push'::
-	For listing remote refs and pushing specified objects from the
-	local object store to remote refs.
-
-'fetch'::
-	For listing remote refs and fetching the associated history to
-	the local object store.
-
-'export'::
-	For listing remote refs and pushing specified objects from a
-	fast-import stream to remote refs.
-
-'import'::
-	For listing remote refs and fetching the associated history as
-	a fast-import stream.
-
-'refspec' <refspec>::
-	This modifies the 'import' capability, allowing the produced
-	fast-import stream to modify refs in a private namespace
-	instead of writing to refs/heads or refs/remotes directly.
-	It is recommended that all importers providing the 'import'
-	capability use this.
-+
-A helper advertising the capability
-`refspec refs/heads/*:refs/svn/origin/branches/*`
-is saying that, when it is asked to `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 *:*`.
-
-'bidi-import'::
-	The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers
-	to retrieve information about blobs and trees that already exist in
-	fast-import's memory. This requires a channel from fast-import to the
-	remote-helper.
-	If it is advertised in addition to "import", git establishes a pipe from
-	fast-import to the remote-helper's stdin.
-	It follows that git and fast-import are both connected to the
-	remote-helper's stdin. Because git can send multiple commands to
-	the remote-helper it is required that helpers that use 'bidi-import'
-	buffer all 'import' commands of a batch before sending data to fast-import.
-	This is to prevent mixing commands and fast-import responses on the
-	helper's stdin.
-
-'export-marks' <file>::
-	This modifies the 'export' capability, instructing git to dump the
-	internal marks table to <file> when complete. For details,
-	read up on '--export-marks=<file>' in linkgit:git-fast-export[1].
-
-'import-marks' <file>::
-	This modifies the 'export' capability, instructing git to load the
-	marks specified in <file> before processing any input. For details,
-	read up on '--import-marks=<file>' in linkgit:git-fast-export[1].
+In the following, we list all defined capabilities and for
+each we list which commands a helper with that capability
+must provide.
 
 Capabilities for Pushing
-~~~~~~~~~~~~~~~~~~~~~~~~
+^^^^^^^^^^^^^^^^^^^^^^^^
 'connect'::
 	Can attempt to connect to 'git receive-pack' (for pushing),
-	'git upload-pack', etc for communication using the
-	packfile protocol.
+	'git upload-pack', etc for communication using
+	git's native packfile protocol. This
+	requires a bidirectional, full-duplex connection.
 +
 Supported commands: 'connect'.
 
@@ -186,11 +122,12 @@ Other frontends may have some other order of preference.
 
 
 Capabilities for Fetching
-~~~~~~~~~~~~~~~~~~~~~~~~~
+^^^^^^^^^^^^^^^^^^^^^^^^^
 'connect'::
 	Can try to connect to 'git upload-pack' (for fetching),
 	'git receive-pack', etc for communication using the
-	packfile protocol.
+	git's native packfile protocol. This
+	requires a bidirectional, full-duplex connection.
 +
 Supported commands: 'connect'.
 
@@ -212,14 +149,27 @@ 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.
 
+Miscellaneous capabilities
+^^^^^^^^^^^^^^^^^^^^^^^^^^
+
+'option'::
+	For specifying 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.
+
 'refspec' <refspec>::
-	This modifies the 'import' capability.
+	This modifies the 'import' capability, allowing the produced
+	fast-import stream to modify refs in a private namespace
+	instead of writing to refs/heads or refs/remotes directly.
+	It is recommended that all importers providing the 'import'
+	capability use this.
 +
-A helper advertising
+A helper advertising the capability
 `refspec refs/heads/*:refs/svn/origin/branches/*`
-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.
+is saying that, when it is asked to `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
@@ -227,6 +177,34 @@ advertised with this capability must cover all refs reported by
 the list command.  If no 'refspec' capability is advertised,
 there is an implied `refspec *:*`.
 
+'bidi-import'::
+	This modifies the 'import' capability.
+	The fast-import commands 'cat-blob' and 'ls' can be used by remote-helpers
+	to retrieve information about blobs and trees that already exist in
+	fast-import's memory. This requires a channel from fast-import to the
+	remote-helper.
+	If it is advertised in addition to "import", git establishes a pipe from
+	fast-import to the remote-helper's stdin.
+	It follows that git and fast-import are both connected to the
+	remote-helper's stdin. Because git can send multiple commands to
+	the remote-helper it is required that helpers that use 'bidi-import'
+	buffer all 'import' commands of a batch before sending data to fast-import.
+	This is to prevent mixing commands and fast-import responses on the
+	helper's stdin.
+
+'export-marks' <file>::
+	This modifies the 'export' capability, instructing git to dump the
+	internal marks table to <file> when complete. For details,
+	read up on '--export-marks=<file>' in linkgit:git-fast-export[1].
+
+'import-marks' <file>::
+	This modifies the 'export' capability, instructing git to load the
+	marks specified in <file> before processing any input. For details,
+	read up on '--import-marks=<file>' in linkgit:git-fast-export[1].
+
+
+
+
 COMMANDS
 --------
 
-- 
1.8.0.393.gcc9701d
Max Horn· Nov 27, 2012, 17:44 UTC · re: Max Horn · lore

[PATCH 5/6] Make clearer which commands must be supported for which capabilities

In particular, document 'list for-push' separately from 'list', as the former needs only be supported for the 'push' capability, and the latter only for fetch/import/export. In particular, a hypothetically 'push-only' helper only needs to support the former, not the latter.

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 21 ++++++++++++++++-----
 1 file changed, 16 insertions(+), 5 deletions(-)
Show changes to Documentation/git-remote-helpers.txt +16 −5
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 7ac1461..023dcca 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -216,6 +216,8 @@ Commands are given by the caller on the helper's standard input, one per line.
 	which marks them mandatory for git versions using the remote
 	helper to understand. Any unknown mandatory capability is a
 	fatal error.
++
+Support for this command is mandatory.
 
 'list'::
 	Lists the refs, one per line, in the format "<value> <name>
@@ -225,9 +227,18 @@ Commands are given by the caller on the helper's standard input, one per line.
 	the name; unrecognized attributes are ignored. The list ends
 	with a blank line.
 +
-If 'push' is supported this may be called as 'list for-push'
-to obtain the current refs prior to sending one or more 'push'
-commands to the helper.
+Supported if the helper has the "fetch" or "import" capability.
+
+'list for-push'::
+	Similar to 'list', except that it is used if and only if
+	the caller wants to the resulting ref list to prepare
+	push commands.
+	A helper supporting both push and fetch can use this
+	to distinguish for which operation the output of 'list'
+	is going to be used, possibly reducing the amount
+	of work that needs to be performed.
++
+Supported if the helper has the "push" or "export" capability.
 
 'option' <name> <value>::
 	Sets the transport helper option <name> to <value>.  Outputs a
@@ -306,7 +317,7 @@ sequence has to be buffered before starting to send data to fast-import
 to prevent mixing of commands and fast-import responses on the helper's
 stdin.
 +
-Supported if the helper has the 'import' capability.
+Supported if the helper has the "import" capability.
 
 'export'::
 	Instructs the remote helper that any subsequent input is
@@ -322,7 +333,7 @@ fast-export', which then will load/store a table of marks for
 local objects. This can be used to implement for incremental
 operations.
 +
-Supported if the helper has the 'export' capability.
+Supported if the helper has the "export" capability.
 
 'connect' <service>::
 	Connects to given service. Standard input and standard output
-- 
1.8.0.393.gcc9701d
Max Horn· Nov 27, 2012, 17:44 UTC · re: Max Horn · lore

[PATCH 6/6] Remove 'for-push' from ref list attributes list, link to subsections

The documentation was misleading in that it gave the impression that 'for-push' could be used as a ref attribute in the output of the 'list' command. That is wrong.

Signed-off-by: Max Horn <max@quendi.de>
---
 Documentation/git-remote-helpers.txt | 13 ++++++++-----
 1 file changed, 8 insertions(+), 5 deletions(-)
Show changes to Documentation/git-remote-helpers.txt +8 −5
diff --git a/Documentation/git-remote-helpers.txt b/Documentation/git-remote-helpers.txt
index 023dcca..081cb06 100644
--- a/Documentation/git-remote-helpers.txt
+++ b/Documentation/git-remote-helpers.txt
@@ -227,6 +227,8 @@ Support for this command is mandatory.
 	the name; unrecognized attributes are ignored. The list ends
 	with a blank line.
 +
+See REF LIST ATTRIBUTES for a list of currently defined options.
++
 Supported if the helper has the "fetch" or "import" capability.
 
 'list for-push'::
@@ -248,6 +250,8 @@ Supported if the helper has the "push" or "export" capability.
 	for it).  Options should be set before other commands,
 	and may influence the behavior of those commands.
 +
+See OPTIONS for a list of currently defined options.
++
 Supported if the helper has the "option" capability.
 
 'fetch' <sha1> <name>::
@@ -256,7 +260,7 @@ Supported if the helper has the "option" capability.
 	per line, terminated with a blank line.
 	Outputs a single blank line when all fetch commands in the
 	same batch are complete. Only objects which were reported
-	in the ref list with a sha1 may be fetched this way.
+	in the output of 'list' with a sha1 may be fetched this way.
 +
 Optionally may output a 'lock <file>' line indicating a file under
 GIT_DIR/objects/pack which is keeping a pack until refs can be
@@ -360,10 +364,9 @@ capabilities reported by the helper.
 REF LIST ATTRIBUTES
 -------------------
 
-'for-push'::
-	The caller wants to use the ref list to prepare push
-	commands.  A helper might chose to acquire the ref list by
-	opening a different type of connection to the destination.
+The 'list' command produces a list of refs in which each ref
+may be followed by a list of attributes. The following ref list
+attributes are defined.
 
 'unchanged'::
 	This ref is unchanged since the last import or fetch, although
-- 
1.8.0.393.gcc9701d
Junio C Hamano· Dec 7, 2012, 19:09 UTC · re: Max Horn · lore

Re: [PATCH 0/6] Improve remote helper documentation

Max Horn <max@quendi.de> writes:
Show 12 quoted lines
> Various remote helper capabilities and commands were not
> documented, in particular 'export', or documented in a misleading
> way (e.g. 'for-push' was listed as a ref attribute understood by
> git, which is not the case). This patch series changes that, and
> also address some other things in the remote helper documentation
> that I found jarring when reading through it.
>
> Note that the description of export and (im|ex)port-marks probably can be
> improved, and I hope that somebody who knows more about them
> than me and/or is better at writing documentation will do just that.
> But I felt it was better to provide something than to do nothing
> and only complain, as I did previously on this subject ;-).

A second ping to people who have touched transport-helper.c, remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in the past 18 months for comments. I've re-read the documentation updates myself and didn't find anything majorly objectionable.

Except for a minor nit in 6/6; I think "defined options" should be "defined attributes".

    --- a/Documentation/git-remote-helpers.txt
    +++ b/Documentation/git-remote-helpers.txt
    @@ -227,6 +227,8 @@ Support for this command is mandatory.
            the name; unrecognized attributes are ignored. The list ends
            with a blank line.
     +
    +See REF LIST ATTRIBUTES for a list of currently defined options.
    ++
Sverre Rabbelier· Dec 7, 2012, 20:26 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/6] Improve remote helper documentation

On Fri, Dec 7, 2012 at 11:09 AM, Junio C Hamano <gitster@pobox.com> wrote:
> A second ping to people who have touched transport-helper.c,
> remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in
> the past 18 months for comments.  I've re-read the documentation
> updates myself and didn't find anything majorly objectionable.
FWIW:
Acked-by: Sverre Rabbelier <srabbelier@gmail.com>
-- 
Cheers,

Sverre Rabbelier
Max Horn· Dec 7, 2012, 21:41 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/6] Improve remote helper documentation

On 07.12.2012, at 20:09, Junio C Hamano wrote:
Show 31 quoted lines
> Max Horn <max@quendi.de> writes:
> 
>> Various remote helper capabilities and commands were not
>> documented, in particular 'export', or documented in a misleading
>> way (e.g. 'for-push' was listed as a ref attribute understood by
>> git, which is not the case). This patch series changes that, and
>> also address some other things in the remote helper documentation
>> that I found jarring when reading through it.
>> 
>> Note that the description of export and (im|ex)port-marks probably can be
>> improved, and I hope that somebody who knows more about them
>> than me and/or is better at writing documentation will do just that.
>> But I felt it was better to provide something than to do nothing
>> and only complain, as I did previously on this subject ;-).
> 
> A second ping to people who have touched transport-helper.c,
> remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in
> the past 18 months for comments.  I've re-read the documentation
> updates myself and didn't find anything majorly objectionable.
> 
> Except for a minor nit in 6/6; I think "defined options" should be
> "defined attributes".
> 
>    --- a/Documentation/git-remote-helpers.txt
>    +++ b/Documentation/git-remote-helpers.txt
>    @@ -227,6 +227,8 @@ Support for this command is mandatory.
>            the name; unrecognized attributes are ignored. The list ends
>            with a blank line.
>     +
>    +See REF LIST ATTRIBUTES for a list of currently defined options.
>    ++
Indeed, my mistake. Dumb question since I am still not completely familiar with the procedures here: Would you just fix this yourself as you apply this, or is it better (or even required) that I reroll for this? (Of course if I ned to re-roll for other reasons, I'd fix this one as well).

Thanks, Max

Junio C Hamano· Dec 7, 2012, 21:52 UTC · re: Max Horn · lore

Re: [PATCH 0/6] Improve remote helper documentation

Max Horn <max@quendi.de> writes:
Show 19 quoted lines
> On 07.12.2012, at 20:09, Junio C Hamano wrote:
>
>> Except for a minor nit in 6/6; I think "defined options" should be
>> "defined attributes".
>> 
>>    --- a/Documentation/git-remote-helpers.txt
>>    +++ b/Documentation/git-remote-helpers.txt
>>    @@ -227,6 +227,8 @@ Support for this command is mandatory.
>>            the name; unrecognized attributes are ignored. The list ends
>>            with a blank line.
>>     +
>>    +See REF LIST ATTRIBUTES for a list of currently defined options.
>>    ++
>
> Indeed, my mistake. Dumb question since I am still not completely
> familiar with the procedures here: Would you just fix this
> yourself as you apply this, or is it better (or even required)
> that I reroll for this? (Of course if I ned to re-roll for other
> reasons, I'd fix this one as well).

I didn't see any other issues myself, and I didn't see anybody raising issues on the series, either, so I could fix it up by amending what is already queued, as long as you are OK with it.

Thanks.
Max Horn· Dec 7, 2012, 22:38 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/6] Improve remote helper documentation

On 07.12.2012, at 22:52, Junio C Hamano wrote:
Show 25 quoted lines
> Max Horn <max@quendi.de> writes:
> 
>> On 07.12.2012, at 20:09, Junio C Hamano wrote:
>> 
>>> Except for a minor nit in 6/6; I think "defined options" should be
>>> "defined attributes".
>>> 
>>>   --- a/Documentation/git-remote-helpers.txt
>>>   +++ b/Documentation/git-remote-helpers.txt
>>>   @@ -227,6 +227,8 @@ Support for this command is mandatory.
>>>           the name; unrecognized attributes are ignored. The list ends
>>>           with a blank line.
>>>    +
>>>   +See REF LIST ATTRIBUTES for a list of currently defined options.
>>>   ++
>> 
>> Indeed, my mistake. Dumb question since I am still not completely
>> familiar with the procedures here: Would you just fix this
>> yourself as you apply this, or is it better (or even required)
>> that I reroll for this? (Of course if I ned to re-roll for other
>> reasons, I'd fix this one as well).
> 
> I didn't see any other issues myself, and I didn't see anybody
> raising issues on the series, either, so I could fix it up by
> amending what is already queued, as long as you are OK with it.
Sure, I'd appreciate if you could make that fix.

Thanks, Max

Felipe Contreras· Dec 12, 2012, 22:22 UTC · re: Junio C Hamano · lore

Re: [PATCH 0/6] Improve remote helper documentation

On Fri, Dec 7, 2012 at 1:09 PM, Junio C Hamano <gitster@pobox.com> wrote:
Show 22 quoted lines
> Max Horn <max@quendi.de> writes:
>
>> Various remote helper capabilities and commands were not
>> documented, in particular 'export', or documented in a misleading
>> way (e.g. 'for-push' was listed as a ref attribute understood by
>> git, which is not the case). This patch series changes that, and
>> also address some other things in the remote helper documentation
>> that I found jarring when reading through it.
>>
>> Note that the description of export and (im|ex)port-marks probably can be
>> improved, and I hope that somebody who knows more about them
>> than me and/or is better at writing documentation will do just that.
>> But I felt it was better to provide something than to do nothing
>> and only complain, as I did previously on this subject ;-).
>
> A second ping to people who have touched transport-helper.c,
> remote-testsvn.c, git-remote-testgit, and contrib/remote-helpers/ in
> the past 18 months for comments.  I've re-read the documentation
> updates myself and didn't find anything majorly objectionable.
>
> Except for a minor nit in 6/6; I think "defined options" should be
> "defined attributes".

I think the wording in 1/6 can be improved. Other than that I don't have any comments, what I'm familiar with looks good to me.

-- 
Felipe Contreras

← back to recent threads