patch, 13 partsprevent try_delta from using objects not in pack

> Subject: [PATCH 08/13] prevent try_delta from using objects not in pack
>
> ---
>  builtin-pack-objects.c |    4 ++++
>  1 files changed, 4 insertions(+), 0 deletions(-)
>
> diff --git a/builtin-pack-objects.c b/builtin-pack-objects.c
> index 9eb5fd6..37b0150 100644
> --- a/builtin-pack-objects.c
> +++ b/builtin-pack-objects.c
> @@ -1251,6 +1251,10 @@ static int try_delta(struct unpacked *trg, struct unpacked *src,
>  	if (trg_entry->type != src_entry->type)
>  		return -1;
>  
> +	/* Don't try deltas involving already/non written objects */
> +	if (trg_entry->no_write || src_entry->no_write)
> +		return -1;
> +
>  	/* We do not compute delta to *create* objects we are not
>  	 * going to pack.
>  	 */

> On 4/5/07, Nicolas Pitre <nico@cam.org> wrote:
> > What is the purpose of this patch?
> > 
> > The try_delta() function is called with all objects before any object is
> > written to a pack to find out how to deltify objects upfront.
> 
> I set no_write for 2 different reasons in the patchset.
> (1) When the blob is too big (--blob-limit) and will never be written.
> (2) When the blob has been written to a previous, finished pack.
> 
> You're correct that this patch will never see condition (2).

> I think my repository statistics are a little unusual.
> Perhaps I'm getting ahead of myself here,
> but I also wanted to experiment with writing all blobs to one set
> of packs,  and all trees, commits, and tags to another set
> (but probably just one small pack).
> I would use no_write for that and it would matter here.

> On Thu, 5 Apr 2007, Dana How wrote:
> > On 4/5/07, Nicolas Pitre <nico@cam.org> wrote:
> > > What is the purpose of this patch?
> > >
> > > The try_delta() function is called with all objects before any object is
> > > written to a pack to find out how to deltify objects upfront.
> >
> > I set no_write for 2 different reasons in the patchset.
> > (1) When the blob is too big (--blob-limit) and will never be written.
> > (2) When the blob has been written to a previous, finished pack.
> >
> > You're correct that this patch will never see condition (2).
>
> Given that I proposed another way for big blobs in my previous email,
> then (1) should not be needed either.
>
> > I think my repository statistics are a little unusual.
> > Perhaps I'm getting ahead of myself here,
> > but I also wanted to experiment with writing all blobs to one set
> > of packs,  and all trees, commits, and tags to another set
> > (but probably just one small pack).
> > I would use no_write for that and it would matter here.
>
> Again you should simply _not_ add objects you don't want to the list
> instead of adding them and marking them as unwanted.

> Currently we have get_object_list -> traverse_commit_list ->
> show_{commit,object} -> add_object_entry ,  which is all
> called way before get_object_details -> check_object -> sha1_object_info .
> Can I safely move the sha1_object_info calll earlier into
> add_object_entry so I will know the size for pruning?

> On Thu, 5 Apr 2007, Dana How wrote:
> > Currently we have get_object_list -> traverse_commit_list ->
> > show_{commit,object} -> add_object_entry ,  which is all
> > called way before get_object_details -> check_object -> sha1_object_info .
> > Can I safely move the sha1_object_info calll earlier into
> > add_object_entry so I will know the size for pruning?
>
> I think so, yes.  And actually this call to sha1_object_info() has been
> bothering me for a while.
>
> There are currently two ways to get the list of objects to pack: one is
> from stdin where objects are listed with their SHA1's, the other one
> uses the internal revision walking code.  In the later case we _already_
> have the information that sha1_object_info() later provides making it
> rather wasteful by forcing a second object header parsing.