Re: [PATCH v6 1/4] repository: add a helper function to perform repository format upgrade

[Junio C Hamano <gitster@pobox.com>]

Xin Li <delphij@google.com> writes:

> In version 1 of repository format, "extensions" gained special meaning
> and it is safer to avoid upgrading when there are pre-existing
> extensions.

I am tempted to suggest s/upgrading/& from version 0/ but if we
assume everybody knows there are only v0 and v1, that may be
unnecessary.  I dunno.

> Make list-objects-filter to use the helper function instead of setting
> repository version directly as a prerequisite of exposing the upgrade
> capability.
>
> Signed-off-by: Xin Li <delphij@google.com>
> ---
>  cache.h                       |  1 +
>  list-objects-filter-options.c |  3 ++-
>  repository.h                  |  6 ++++++
>  setup.c                       | 29 +++++++++++++++++++++++++++++
>  4 files changed, 38 insertions(+), 1 deletion(-)
>
> diff --git a/cache.h b/cache.h
> index 0f0485ecfe..e5885cc9ea 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -1042,6 +1042,7 @@ struct repository_format {
>  	int worktree_config;
>  	int is_bare;
>  	int hash_algo;
> +	int has_extensions;
>  	char *work_tree;
>  	struct string_list unknown_extensions;
>  };
> diff --git a/list-objects-filter-options.c b/list-objects-filter-options.c
> index 256bcfbdfe..3553ad7b0a 100644
> --- a/list-objects-filter-options.c
> +++ b/list-objects-filter-options.c
> @@ -326,7 +326,8 @@ void partial_clone_register(
>  
>  	/* Check if it is already registered */
>  	if (!promisor_remote_find(remote)) {
> -		git_config_set("core.repositoryformatversion", "1");
> +		if (upgrade_repository_format(1) < 0)
> +			die(_("unable to upgrade repository format to support partial clone"));

The idea is that builtin/fetch.c calls this before the actual fetch
operation happens, which sounds sensible.


The other caller of this function is in builtin/clone.c; at that
point, we have set up our $GIT_DIR/config file enough to be able to
write the refspec into it, so we should be able to tell what version
number we wrote to the repository.  Could we have written version 0
there in today's code, though (just being curious, as we should be
able to upgrade it to version 1 with this code)?

> +int upgrade_repository_format(int target_version)
> +{
> +	struct strbuf sb = STRBUF_INIT;
> +	struct strbuf err = STRBUF_INIT;
> +	struct strbuf repo_version = STRBUF_INIT;
> +	struct repository_format repo_fmt = REPOSITORY_FORMAT_INIT;
> +
> +	strbuf_git_common_path(&sb, the_repository, "config");
> +	read_repository_format(&repo_fmt, sb.buf);
> +	strbuf_release(&sb);
> +
> +	if (repo_fmt.version >= target_version)
> +		return 0;
> +
> +	if (verify_repository_format(&repo_fmt, &err) < 0 ||
> +	    (!repo_fmt.version && repo_fmt.has_extensions)) {

"If we do not understand the extensions, or if the original is v0
and has any extensions, we shouldn't be upgrading"---makes sense.

> +		warning("unable to upgrade repository format from %d to %d: %s",
> +			repo_fmt.version, target_version, err.buf);
> +		strbuf_release(&err);
> +		return -1;
> +	}
> +
> +	strbuf_addf(&repo_version, "%d", target_version);
> +	git_config_set("core.repositoryformatversion", repo_version.buf);
> +	strbuf_release(&repo_version);
> +	return 1;
> +}
> +
>  static void init_repository_format(struct repository_format *format)
>  {
>  	const struct repository_format fresh = REPOSITORY_FORMAT_INIT;