Re: [PATCH v6 1/4] repository: add a helper function to perform repository format upgrade
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Jun 5, 2020, 19:12 UTC
- Message-ID
- <xmqqy2p1minn.fsf@gitster.c.googlers.com>
- In-Reply-To
- <20200605091004.208668-2-delphij@google.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.
Show 35 quoted lines
> 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)?
Show 16 quoted lines
> +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.
Show 15 quoted lines
> + 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;