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 <pclo...@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

Attachment: signature.asc
Description: PGP signature

Reply via email to