Re: [RFC/PATCH] Documentation/remote-helpers: explain capabilities first
- From
Jonathan Nieder <jrnieder@gmail.com>
- Date
- Mar 19, 2011, 00:29 UTC
- Message-ID
- <20110319002904.GA25155@elie>
- In-Reply-To
- <20110318174504.GA22332@elie>
Hi again,
Sorry for the spam. Just ran into a quick puzzle:
Jonathan Nieder wrote:
Show 5 quoted lines
> +'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'.
[...]
Show 11 quoted lines
> +'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