Re: [PATCH v2 2/8] setup: stop applying repository format twice

[Justin Tobler <jltobler@gmail.com>]

On 26/06/15 03:56PM, Patrick Steinhardt wrote:
> When discovering the repository in "setup.c" we apply the final
> repository format multiple times:
> 
>   - Once via `repository_format_configure()`, where we apply the hash
>     algorithm and ref storage format to both `struct repository_format`
>     and `struct repository`.
> 
>   - And once via `apply_repository_format()`, where we apply these two
>     settings from `struct repository_format` to `struct repository`.
> 
> With the current flow both of these are in fact necessary. But this is
> only because we call `repository_format_configure()` after we have
> called `apply_repository_format()`. Consequently, if we only changed the
> repository format in `repository_format_configure()` it would never
> propagate to the repository.

Ok, so because `repository_format_configure()` is invoked after the
repository format was already applied, it had to explictly configure the
repository as well.

> Refactor the code so that we first configure the repository format
> before applying it to the repository so that we can stop setting the
> hash and reference storage format multiple times.

Makes sense. Sounds like a good change.

> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  setup.c | 9 ++-------
>  1 file changed, 2 insertions(+), 7 deletions(-)
> 
> diff --git a/setup.c b/setup.c
> index a9db1f2c23..2748155964 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -2710,8 +2710,7 @@ static int read_default_format_config(const char *key, const char *value,
>  	return ret;
>  }
>  
> -static void repository_format_configure(struct repository *repo,
> -					struct repository_format *repo_fmt,
> +static void repository_format_configure(struct repository_format *repo_fmt,
>  					int hash, enum ref_storage_format ref_format)

We now only care about configuring the repository format and will let
`apply_repository_format()` handle setting the repository. Looks good.

[snip]
> @@ -2830,10 +2825,10 @@ int init_db(struct repository *repo,
>  	 * is an attempt to reinitialize new repository with an old tool.
>  	 */
>  	check_repository_format_gently(repo_get_git_dir(repo), &repo_fmt, NULL);
> +	repository_format_configure(&repo_fmt, hash, ref_storage_format);
>  	if (apply_repository_format(repo, &repo_fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
>  		die("%s", err.buf);
>  	startup_info->have_repository = 1;
> -	repository_format_configure(repo, &repo_fmt, hash, ref_storage_format);

`apply_repository_format()` already has the logic to set the hash algo
and ref storage format from the repository format, so change changing
the order here is ok and a good change.

-Justin