[v6,1/4] repository: add a helper function to perform repository format upgrade
diff mbox series

Message ID 20200605091004.208668-2-delphij@google.com
State New
Headers show
Series
  • fetch: allow adding a filter after initial clone
Related show

Commit Message

Xin Li June 5, 2020, 9:10 a.m. UTC
In version 1 of repository format, "extensions" gained special meaning
and it is safer to avoid upgrading when there are pre-existing
extensions.

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(-)

Comments

Junio C Hamano June 5, 2020, 7:12 p.m. UTC | #1
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;

Patch
diff mbox series

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"));
 
 		/* Add promisor config for the remote */
 		cfg_name = xstrfmt("remote.%s.promisor", remote);
diff --git a/repository.h b/repository.h
index 6534fbb7b3..3c1f7d54bd 100644
--- a/repository.h
+++ b/repository.h
@@ -196,4 +196,10 @@  void repo_update_index_if_able(struct repository *, struct lock_file *);
 
 void prepare_repo_settings(struct repository *r);
 
+/*
+ * Return 1 if upgrade repository format to target_version succeeded,
+ * 0 if no upgrade is necessary, and -1 when upgrade is not possible.
+ */
+int upgrade_repository_format(int target_version);
+
 #endif /* REPOSITORY_H */
diff --git a/setup.c b/setup.c
index 65fe5ecefb..597b41b822 100644
--- a/setup.c
+++ b/setup.c
@@ -455,6 +455,7 @@  static int check_repo_format(const char *var, const char *value, void *vdata)
 	if (strcmp(var, "core.repositoryformatversion") == 0)
 		data->version = git_config_int(var, value);
 	else if (skip_prefix(var, "extensions.", &ext)) {
+		data->has_extensions = 1;
 		/*
 		 * record any known extensions here; otherwise,
 		 * we fall through to recording it as unknown, and
@@ -538,6 +539,34 @@  static int check_repository_format_gently(const char *gitdir, struct repository_
 	return 0;
 }
 
+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)) {
+		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;