From: Junio C Hamano <gitster@pobox.com>
Date: Thu, 15 Jun 2017 17:00:39 GMT
Subject: Re: [PATCH v2 2/4] sha1_file: move delta base cache code up
Message-ID: <xmqqbmpp2nk8.fsf@gitster.mtv.corp.google.com>
In-Reply-To: <36b7984ee85d22800b974a04952c50eef043fa67.1497387714.git.jonathantanmy@google.com>

Jonathan Tan <jonathantanmy@google.com> writes:

> In a subsequent patch, packed_object_info() will be modified to use the
> delta base cache, so move the relevant code to before
> packed_object_info().
>
> Signed-off-by: Jonathan Tan <jonathantanmy@google.com>
> ---
>  sha1_file.c | 226 +++++++++++++++++++++++++++++++-----------------------------
>  1 file changed, 116 insertions(+), 110 deletions(-)

Hmph, is this meant to be just moving two whole functions?

> diff --git a/sha1_file.c b/sha1_file.c
> index a52b27541..a158907d1 100644
> --- a/sha1_file.c
> +++ b/sha1_file.c
> @@ -2239,116 +2239,6 @@ static enum ...
> ...
> -int packed_object_info(struct packed_git *p, off_t obj_offset,
> -		       struct object_info *oi)
> -{
> -...
> -	if (oi->delta_base_sha1) {
> -		if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
> -			const unsigned char *base;
> -
> -			base = get_delta_base_sha1(p, &w_curs, curpos,
> -						   type, obj_offset);
> -			if (!base) {
> -				type = OBJ_BAD;
> -				goto out;
> -			}
> -
> -			hashcpy(oi->delta_base_sha1, base);
> -		} else
> -			hashclr(oi->delta_base_sha1);
> -	}
> -
> -out:
> -	unuse_pack(&w_curs);
> -	return type;
> -}
> -...

The above is what was removed, while ...

> @@ -2486,6 +2376,122 @@ static void ...
> ...
> +int packed_object_info(struct packed_git *p, off_t obj_offset,
> +		       struct object_info *oi)
> +{
> +...
> +	if (oi->delta_base_sha1) {
> +		if (type == OBJ_OFS_DELTA || type == OBJ_REF_DELTA) {
> +			const unsigned char *base;
> +
> +			base = get_delta_base_sha1(p, &w_curs, curpos,
> +						   type, obj_offset);
> +			if (!base) {
> +				type = OBJ_BAD;
> +				goto out;
> +			}
> +
> +			hashcpy(oi->delta_base_sha1, base);
> +		} else
> +			hashclr(oi->delta_base_sha1);
> +	}
> +
> +	oi->whence = OI_PACKED;
> +	oi->u.packed.offset = obj_offset;
> +	oi->u.packed.pack = p;
> +	oi->u.packed.is_delta = (type == OBJ_REF_DELTA ||
> +				 type == OBJ_OFS_DELTA);
> +
> +out:
> +	unuse_pack(&w_curs);
> +	return type;
> +}

... we somehow gained code to update *oi that used to be (and still
is) done by its sole caller, sha1_object_info_extended().

Perhaps this is a rebase-gotcha?