On Sat, Feb 24, 2018 at 10:34:27AM +0700, Nguyễn Thái Ngọc Duy wrote:
> After 454253f059 (builtin/index-pack: improve hash function abstraction
> - 2018-02-01), index-pack uses the_hash_algo for hashing. If "git
> index-pack" is executed without a repository, we do not know what hash
> algorithm to be used and the_hash_algo in theory could be undefined.
> 
> Since there should be some information about the hash algorithm in the
> input pack file, we can initialize the correct hash algorithm with that
> if the_hash_algo is not yet initialized. This assumes that pack files
> with new hash algorithm MUST step up pack version.
> 
> While at there, make sure the hash algorithm requested by the pack file
> and configured by the repository (if we're running with a repo) are
> consistent.
> 
> Signed-off-by: Nguyễn Thái Ngọc Duy <pclouds@gmail.com>
> ---
>  builtin/index-pack.c | 26 +++++++++++++++++++++++++-
>  1 file changed, 25 insertions(+), 1 deletion(-)
> 
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 7e3e1a461c..1144458140 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -326,10 +326,31 @@ static const char *open_pack_file(const char *pack_name)
>  		output_fd = -1;
>  		nothread_data.pack_fd = input_fd;
>  	}
> -	the_hash_algo->init_fn(&input_ctx);
>  	return pack_name;
>  }
>  
> +static void prepare_hash_algo(uint32_t pack_version)
> +{
> +	const struct git_hash_algo *pack_algo;
> +
> +	switch (pack_version) {
> +	case 2:
> +	case 3:
> +		pack_algo = &hash_algos[GIT_HASH_SHA1];
> +		break;
> +	default:
> +		die("BUG: how to determine hash algo for new version?");
> +	}
> +
> +	if (!the_hash_algo)	/* running without repo */
> +		the_hash_algo = pack_algo;
> +
> +	if (the_hash_algo != pack_algo)
> +		die(_("incompatible hash algorithm, "
> +		      "configured for %s but the pack file needs %s"),
> +		    the_hash_algo->name, pack_algo->name);

I like this.  It's a nice improvement and it should be easy for us to
pass additional information into the function when our pack format
understands multiple algorithms.

I might have done the comparison using the format_id members instead of
the pointers themselves, but that's more a personal preference than
anything.
-- 
brian m. carlson / brian with sandals: Houston, Texas, US
https://www.crustytoothpaste.net/~bmc | My opinion only
OpenPGP: https://keybase.io/bk2204