All of lore.kernel.org
 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.