From: brian m. carlson <sandals@crustytoothpaste.net>
Date: Wed, 19 Dec 2018 00:18:01 GMT
Subject: Re: [PATCH 2/3] setup: do not use invalid `repository_format`
Message-ID: <20181219001801.GA520988@genre.crustytoothpaste.net>
In-Reply-To: <20181218072528.3870492-3-martin.agren@gmail.com>

On Tue, Dec 18, 2018 at 08:25:27AM +0100, Martin Ågren wrote:
>  I fully admit to not understanding all of this setup code, neither in
>  its current incarnation, nor in terms of an ideal end game. This check
>  seems like a good thing to do though.

It's definitely complex.

> diff --git a/setup.c b/setup.c
> index 27747af7a3..52c3c9d31f 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1138,7 +1138,7 @@ const char *setup_git_directory_gently(int *nongit_ok)
>  				gitdir = DEFAULT_GIT_DIR_ENVIRONMENT;
>  			setup_git_env(gitdir);
>  		}
> -		if (startup_info->have_repository)
> +		if (startup_info->have_repository && repo_fmt.version > -1)
>  			repo_set_hash_algo(the_repository, repo_fmt.hash_algo);
>  	}

I think this change is fine, because we initialize the value in
the_repository elsewhere, and if there's no repository, this should
never have a value other than the default anyway.

I looked at the other patches in the series and thought they looked sane
as well.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204