git.vger.kernel.org archive mirror
 help / color / mirror / Atom feed
From: Johannes Sixt <j.sixt@viscovery.net>
To: sam@vilain.net
Cc: git@vger.kernel.org, Nick Edelen <sirnot@gmail.com>,
	"Shawn O. Pearce" <spearce@spearce.org>,
	Johannes Schindelin <Johannes.Schindelin@gmx.de>,
	Andreas Ericsson <exon@op5.se>,
	Christian Couder <christian@couder.net>,
	Jeff King <peff@peff.net>
Subject: Re: [WIP] Shift rev-list enumeration from upload-pack to pack-objects
Date: Fri, 05 Jun 2009 10:10:23 +0200	[thread overview]
Message-ID: <4A28D2EF.8040704@viscovery.net> (raw)
In-Reply-To: <20090605054500.06A9D21C3F4@mail.utsl.gen.nz>

sam@vilain.net schrieb:
> instead of using the internal revision walker and piping object refs
> to pack-objects this patch passes only the revs to pack-objects, which
> in turn handles both enumeration and packing.

I appreciate this move. We have one test failing in MinGW git (t5530.6)
because of the rev-list that is run using start_async(). Even though this
patch doesn't change that (the test case still uses the start_async()
path), it is one step closer to the solution.

> +	/* sending rev params to pack-objects directly is great, but unfortunately pack-objects 
> +	 * has no way of turning off thin pack generation.  this would be a relatively simple 
> +	 * addition, but as we also have to deal with shallow grafts and all it's simplest to 
> +	 * just resort to piping object refs.
> +	 */

You certainly will reformat comments like this to shorter lines, proper
capitalization, without trailing spaces, and adjust the style (initial /*
is on its own line)?

> @@ -181,6 +195,28 @@ static void create_pack_file(void)
>  	if (start_command(&pack_objects))
>  		die("git upload-pack: unable to fork git-pack-objects");
>  
> +	/* pass on revisions we (don't) want 
> +	 * (do we need to check the validity of pack_objects.in?)

No, you don't need to check. It's valid, or you would have died above.

FYI, with this patch MinGW git hangs in t5530.8; the test-case exercises
the new code path.

-- Hannes

  parent reply	other threads:[~2009-06-05  8:11 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2009-06-05  5:45 [WIP] Shift rev-list enumeration from upload-pack to pack-objects sam, Nick Edelen
2009-06-05  5:46 ` Sam Vilain
2009-06-05  8:10 ` Johannes Sixt [this message]
2009-06-08  8:51   ` [PATCH] fetch-pack: close output channel after sideband demultiplexer terminates Johannes Sixt
2009-06-05 16:51 ` [WIP] Shift rev-list enumeration from upload-pack to pack-objects Nicolas Pitre
2009-06-07 13:25   ` Nick Edelen
2009-06-07 13:31     ` Nick Edelen
2009-06-07 16:41     ` Nicolas Pitre
2009-06-07 16:47       ` Nick Edelen
2009-06-07 18:55         ` Nick Edelen
2009-06-07 20:48           ` Sam Vilain
2009-06-07 20:48           ` Nicolas Pitre
2009-06-07 22:04             ` Nick Edelen
2009-06-08  0:50               ` Nicolas Pitre
2009-06-08  2:27                 ` 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=4A28D2EF.8040704@viscovery.net \
    --to=j.sixt@viscovery.net \
    --cc=Johannes.Schindelin@gmx.de \
    --cc=christian@couder.net \
    --cc=exon@op5.se \
    --cc=git@vger.kernel.org \
    --cc=peff@peff.net \
    --cc=sam@vilain.net \
    --cc=sirnot@gmail.com \
    --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).