Re: [PATCH] index-pack: correctly initialize appended objects

["Shawn O. Pearce" <spearce@spearce.org>]

Bjjjrn Steinbrink <B.Steinbrink@gmx.de> wrote:
> When index-pack completes a thin pack it appends objects to the pack.
> Since the commit 92392b4(index-pack: Honor core.deltaBaseCacheLimit when
> resolving deltas) such an object can be pruned in case of memory pressure.
> 
> To be able to re-read the object later, a few more fields have to be set.
> 
> Noticed by Pierre Habouzit.
> 
> Signed-off-by: Björn Steinbrink <B.Steinbrink@gmx.de>
> Acked-by: Nicolas Pitre <nico@cam.org>

Acked-by: Shawn O. Pearce <spearce@spearce.org>

>     On 2008.07.25 15:15:48 +0200, Johannes Schindelin wrote:
>     > So, let's add the comment as Nico suggested, and set real_type,
>     > too?
> 
>     OK, I hope the comment is what was expected. My lack of knowledge
>     made we wonder what to write... :-/

The commit message makes sense to me.  :)
 
>     > (And it would be smashing if you could verify that the type is
>     > indeed correctly set to non-delta...)
> 
>     Hm, we get the object via read_sha1_file, can that return a delta? I
>     would not expect it to.  Sorry, never looked at those code paths
>     (and don't have the time to investigate at the moment).

read_sha1_file() _never_ returns a delta.  It always reutrns the
whole object, even if the object was stored as a delta in a pack
somewhere.  The function is widely used within git to read an object
for processing, without the caller needing to worry about the types
of compression used to store the object.


> diff --git a/index-pack.c b/index-pack.c
> index ac20a46..d757b07 100644
> --- a/index-pack.c
> +++ b/index-pack.c
> @@ -699,6 +699,12 @@ static struct object_entry *append_obj_to_pack(
>  	write_or_die(output_fd, header, n);
>  	obj[0].idx.crc32 = crc32(0, Z_NULL, 0);
>  	obj[0].idx.crc32 = crc32(obj[0].idx.crc32, header, n);
> +	// This object comes from outside the thin pack, so we need to
> +	// initialize the size and type fields
> +	obj[0].hdr_size = n;
> +	obj[0].size = size;
> +	obj[0].type = type;
> +	obj[0].real_type = type;
>  	obj[1].idx.offset = obj[0].idx.offset + n;
>  	obj[1].idx.offset += write_compressed(output_fd, buf, size, &obj[0].idx.crc32);
>  	hashcpy(obj->idx.sha1, sha1);

-- 
Shawn.