Re: [PATCH v2 13/24] builtin/init-db: allow specifying hash algorithm on command line

From: Junio C Hamano <gitster@pobox.com>
To: "brian m. carlson" <sandals@crustytoothpaste.net>
Cc: <git@vger.kernel.org>
Subject: Re: [PATCH v2 13/24] builtin/init-db: allow specifying hash algorithm on command line
Date: Mon, 24 Feb 2020 10:14:33 -0800	[thread overview]
Message-ID: <xmqqh7zfrgxi.fsf@gitster-ct.c.googlers.com> (raw)
In-Reply-To: <20200222201749.937983-14-sandals@crustytoothpaste.net> (brian m. carlson's message of "Sat, 22 Feb 2020 20:17:38 +0000")

"brian m. carlson" <sandals@crustytoothpaste.net> writes:

>  'git init' [-q | --quiet] [--bare] [--template=<template_directory>]
> -	  [--separate-git-dir <git dir>]
> +	  [--separate-git-dir <git dir>] [--object-format=<format]

A missing closing ket> in <bra-ket> pair.

> +#ifndef ENABLE_SHA256
> +	if (fmt->hash_algo != GIT_HASH_SHA1)
> +		die(_("The hash algorithm %s is not supported in this build."), hash_algos[fmt->hash_algo].name);

Could you fold the overlong line here?

>  int init_db(const char *git_dir, const char *real_git_dir,
> -	    const char *template_dir, unsigned int flags)
> +	    const char *template_dir, int hash, unsigned int flags)

Perhaps rename "hash" to "hash_algo"?  I know that it is very
unlikely for a variable whose name is 'hash' to be mistaken as a raw
hash value when its type is "int" (as opposed to say char *), but
still.  I wouldn't be saying this if its type were an "enum
hash_algo" or something like that.

> +	const char *object_format = NULL;
> +	int hash_algo = GIT_HASH_UNKNOWN;

This one _is_ good.

>  	const struct option init_db_options[] = {
>  		OPT_STRING(0, "template", &template_dir, N_("template-directory"),
>  				N_("directory from which templates will be used")),
> @@ -494,6 +526,8 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  		OPT_BIT('q', "quiet", &flags, N_("be quiet"), INIT_DB_QUIET),
>  		OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
>  			   N_("separate git dir from working tree")),
> +		OPT_STRING(0, "object-format", &object_format, N_("hash"),
> +			   N_("specify the hash algorithm to use")),
>  		OPT_END()
>  	};
>  
> @@ -546,6 +580,12 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  		free(cwd);
>  	}
>  
> +	if (object_format) {
> +		hash_algo = hash_algo_by_name(object_format);
> +		if (hash_algo == GIT_HASH_UNKNOWN)
> +			die(_("unknown hash algorithm '%s'"), object_format);
> +	}
> +
>  	if (init_shared_repository != -1)
>  		set_shared_repository(init_shared_repository);
>  
> @@ -597,5 +637,5 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  	UNLEAK(work_tree);
>  
>  	flags |= INIT_DB_EXIST_OK;
> -	return init_db(git_dir, real_git_dir, template_dir, flags);
> +	return init_db(git_dir, real_git_dir, template_dir, hash_algo, flags);
>  }
> diff --git a/cache.h b/cache.h
> index 29ee02a8d4..7a47e023ba 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -627,7 +627,8 @@ int path_inside_repo(const char *prefix, const char *path);
>  #define INIT_DB_EXIST_OK 0x0002
>  
>  int init_db(const char *git_dir, const char *real_git_dir,
> -	    const char *template_dir, unsigned int flags);
> +	    const char *template_dir, int hash_algo,

So is this one.

> +	    unsigned int flags);
>  
>  void sanitize_stdfds(void);
>  int daemonize(void);