git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Ivan Todoroski <grnch_lists@gmx.net>
To: Jeff King <peff@peff.net>
Cc: Shawn Pearce <spearce@spearce.org>,
	Nguyen Thai Ngoc Duy <pclouds@gmail.com>,
	Jakub Narebski <jnareb@gmail.com>,
	git@vger.kernel.org
Subject: Re: [PATCH/RFC 2/2] remote-curl: send the refs to fetch-pack on stdin
Date: Sun, 25 Mar 2012 11:52:35 +0200	[thread overview]
Message-ID: <4F6EEAE3.60204@gmx.net> (raw)
In-Reply-To: <20120325012432.GD27651@sigill.intra.peff.net>

On 25.03.2012 03:24, Jeff King wrote:
> On Sat, Mar 24, 2012 at 09:54:16PM +0100, Ivan Todoroski wrote:
>> -static int rpc_service(struct rpc_state *rpc, struct discovery *heads)
>> +static int rpc_service(struct rpc_state *rpc, struct discovery *heads,
>> +		int nr_fetch, struct ref **to_fetch)
> 
> I was curious why you needed to add new arguments for this, since we
> surely must be passing the information into rpc_service already (since
> it has to put them on the command line). And the answer is that they are
> already in the argv member of the struct rpc_state. I wonder if it would
> fit the existing style of the code to pass the arguments through a
> member in the rpc_state in the same way. I don't feel strongly about it,
> though.

Yeah, I realized this was ugly after I sent it.... not only are new 
arguments added to rpc_service(), but they seem completely arbitrary and 
specific to fetch-pack. There is no reason other callers of 
rpc_service() would want args in the exact same format.

On the other hand I can't reuse rpc_state.argv because that is already 
passed to client.argv for start_command().

Would it be OK if I add a new memeber in rpc_state, e.g. "struct strbuf 
*stdin_preamble"? If non-NULL, it would contain any data the caller of 
rpc_service() wants shoved into the stdin of the sub-command before 
anything else. That way the caller is free to format this data in 
whatever format they need, not only in the pkt-line format needed by 
fetch-pack.

  reply	other threads:[~2012-03-25  9:52 UTC|newest]

Thread overview: 54+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-03-18  8:14 Clone fails on a repo with too many heads/tags Ivan Todoroski
2012-03-18 11:37 ` Ivan Todoroski
2012-03-18 12:04   ` Nguyen Thai Ngoc Duy
2012-03-18 16:36 ` Jakub Narebski
2012-03-18 19:07   ` Jeff King
2012-03-18 22:07     ` Jakub Narebski
2012-03-19  2:32       ` Jeff King
2012-03-19  2:43         ` Nguyen Thai Ngoc Duy
2012-03-19  2:45           ` Jeff King
2012-03-19  1:05     ` Ivan Todoroski
2012-03-19  1:30     ` Nguyen Thai Ngoc Duy
2012-03-19  2:44       ` Jeff King
2012-03-21 11:05         ` Ivan Todoroski
2012-03-21 14:28           ` Shawn Pearce
2012-03-21 17:14             ` Jeff King
2012-03-21 17:59               ` Jakub Narebski
2012-03-21 20:02               ` Ivan Todoroski
2012-03-21 20:17                 ` Jeff King
2012-03-24 20:49                   ` Ivan Todoroski
2012-03-25  1:06                     ` Jeff King
2012-03-25  2:32                       ` Jeff King
2012-03-25 17:33                         ` Ivan Todoroski
2012-03-25 17:54                           ` Ivan Todoroski
2012-03-26 17:33                             ` Jeff King
2012-03-27  7:07                               ` Ivan Todoroski
2012-03-25 15:30                       ` Ivan Todoroski
2012-03-24 20:53                   ` [PATCH/RFC 1/2] fetch-pack: new option to read refs from stdin Ivan Todoroski
2012-03-25  1:19                     ` Jeff King
2012-03-25  9:39                       ` Ivan Todoroski
2012-03-25 15:15                         ` Ivan Todoroski
2012-03-25 20:00                       ` Ivan Todoroski
2012-03-26 17:21                         ` Jeff King
2012-03-26 17:49                           ` Ivan Todoroski
2012-03-26 17:51                             ` Jeff King
2012-03-24 20:54                   ` [PATCH/RFC 2/2] remote-curl: send the refs to fetch-pack on stdin Ivan Todoroski
2012-03-25  1:24                     ` Jeff King
2012-03-25  9:52                       ` Ivan Todoroski [this message]
2012-03-26 17:24                         ` Jeff King
2012-03-27  6:23               ` [PATCH/RFC v2 0/4] Fix fetch-pack command line overflow during clone Ivan Todoroski
2012-03-27  6:25                 ` [PATCH/RFC v2 1/4] fetch-pack: new --stdin option to read refs from stdin Ivan Todoroski
2012-03-27 16:59                   ` Junio C Hamano
2012-03-27 23:18                     ` Ivan Todoroski
2012-03-27 23:26                       ` Junio C Hamano
2012-03-27 23:48                         ` Ivan Todoroski
2012-03-27  6:26                 ` [PATCH/RFC v2 2/4] remote-curl: send the refs to fetch-pack on stdin Ivan Todoroski
2012-03-27 17:18                   ` Junio C Hamano
2012-03-27 23:20                     ` Ivan Todoroski
2012-03-27  6:27                 ` [PATCH/RFC v2 3/4] fetch-pack: test cases for the new --stdin option Ivan Todoroski
2012-03-27 17:40                   ` Junio C Hamano
2012-03-27 23:36                     ` Ivan Todoroski
2012-03-27 23:43                       ` Junio C Hamano
2012-03-28  0:14                       ` Ivan Todoroski
2012-03-27  6:28                 ` [PATCH/RFC v2 4/4] remote-curl: main test case for the OS command line overflow Ivan Todoroski
2012-03-27 17:43                   ` Junio C Hamano

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=4F6EEAE3.60204@gmx.net \
    --to=grnch_lists@gmx.net \
    --cc=git@vger.kernel.org \
    --cc=jnareb@gmail.com \
    --cc=pclouds@gmail.com \
    --cc=peff@peff.net \
    --cc=spearce@spearce.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).