Re: [PATCH 36/44] builtin/index-pack: add option to specify hash algorithm

["Martin Ågren" <martin.agren@gmail.com>]

On Wed, 13 May 2020 at 02:56, brian m. carlson
<sandals@crustytoothpaste.net> wrote:
>
> git index-pack is usually run in a repository, but need not be. Since
> packs don't contains information on the algorithm in use, instead
> relying on context, add an option to index-pack to tell it which one
> we're using in case someone runs it outside of a repository.
>
> Signed-off-by: brian m. carlson <sandals@crustytoothpaste.net>
> ---
>  builtin/index-pack.c | 5 +++++
>  1 file changed, 5 insertions(+)
>
> diff --git a/builtin/index-pack.c b/builtin/index-pack.c
> index 7bea1fba52..89f4962a00 100644
> --- a/builtin/index-pack.c
> +++ b/builtin/index-pack.c
> @@ -1760,6 +1760,11 @@ int cmd_index_pack(int argc, const char **argv, const char *prefix)
>                                         die(_("bad %s"), arg);
>                         } else if (skip_prefix(arg, "--max-input-size=", &arg)) {
>                                 max_input_size = strtoumax(arg, NULL, 10);
> +                       } else if (skip_prefix(arg, "--object-format=", &arg)) {
> +                               int hash_algo = hash_algo_by_name(arg);
> +                               if (hash_algo == GIT_HASH_UNKNOWN)
> +                                       die(_("unknown hash algorithm '%s'"), arg);
> +                               repo_set_hash_algo(the_repository, hash_algo);
>                         } else

Patch 27 added `--hash` to `git show-index` and I almost commented on
"hash" vs "object-format". In the end I figured the object format was a
more technical (protocol) term. But now I wonder. Should we try to align
such options from the start? Or is there perhaps a reason for those
different approaches?

Similar to an earlier patch where we modify `the_hash_algo` like this, I
feel a bit nervous. What happens if you pass in a "wrong" algo here,
i.e., SHA-1 in a SHA-256 repo? Or, given the motivation in the commit
message, should this only be allowed if we really *are* outside a repo?


Martin