Re: [PATCH 14/16] refs: allow ref backend to be set for clone

[Michael Haggerty <mhagger@alum.mit.edu>]

On 12/03/2015 01:35 AM, David Turner wrote:
> Add a new option, --refs-backend-type, to allow the ref backend type to
> be set on new clones.
> 
> Submodules must use the same ref backend as the parent repository, so

^^^ is this verified anywhere? Hint: if not, it probably should be. For
example, imagine a future where an existing repository can be migrated
in place from files -> lmdb. If somebody runs that command in a
submodule, this invariant would be broken and *this* version of git
would have to at least detect the problem and refuse to corrupt things.

I added CC to a couple submodules experts in case they have any
feedback. Are there any other code paths that can lead to submodules
being newly cloned, where this option would also have to be passed?

> we also pass the --refs-backend-type option option when cloning
> submodules.
> 
> Signed-off-by: David Turner <dturner@twopensource.com>
> ---
>  Documentation/git-clone.txt |  4 ++++
>  builtin/clone.c             | 27 +++++++++++++++++++++++++--
>  builtin/submodule--helper.c |  5 ++++-
>  cache.h                     |  1 +
>  4 files changed, 34 insertions(+), 3 deletions(-)
> 
> diff --git a/Documentation/git-clone.txt b/Documentation/git-clone.txt
> index 6bf000d..431575b 100644
> --- a/Documentation/git-clone.txt
> +++ b/Documentation/git-clone.txt
> @@ -14,6 +14,7 @@ SYNOPSIS
>  	  [-o <name>] [-b <name>] [-u <upload-pack>] [--reference <repository>]
>  	  [--dissociate] [--separate-git-dir <git dir>]
>  	  [--depth <depth>] [--[no-]single-branch]
> +	  [--refs-backend-type=<name>]
>  	  [--recursive | --recurse-submodules] [--] <repository>
>  	  [<directory>]
>  
> @@ -221,6 +222,9 @@ objects from the source repository into a pack in the cloned repository.
>  	The result is Git repository can be separated from working
>  	tree.
>  
> +--refs-backend-type=<name>::
> +	Type of refs backend. Default is to use the original files based
> +	backend.
>  
>  <repository>::
>  	The (possibly remote) repository to clone from.  See the
> diff --git a/builtin/clone.c b/builtin/clone.c
> index caae43e..a53f341 100644
> --- a/builtin/clone.c
> +++ b/builtin/clone.c
> @@ -92,11 +92,13 @@ static struct option builtin_clone_options[] = {
>  		   N_("separate git dir from working tree")),
>  	OPT_STRING_LIST('c', "config", &option_config, N_("key=value"),
>  			N_("set config inside the new repository")),
> +	OPT_STRING(0, "refs-backend-type", &refs_backend_type,
> +		   N_("name"), N_("name of backend type to use")),
>  	OPT_END()
>  };
>  
>  static const char *argv_submodule[] = {
> -	"submodule", "update", "--init", "--recursive", NULL
> +	"submodule", "update", "--init", "--recursive", NULL, NULL
>  };
>  
>  static const char *get_repo_path_1(struct strbuf *path, int *is_bundle)
> @@ -724,8 +726,24 @@ static int checkout(void)
>  	err |= run_hook_le(NULL, "post-checkout", sha1_to_hex(null_sha1),
>  			   sha1_to_hex(sha1), "1", NULL);
>  
> -	if (!err && option_recursive)
> +	if (!err && option_recursive) {
> +		const char **backend_arg = argv_submodule;
> +		char *new_backend_arg = NULL;
> +		if (refs_backend_type) {
> +			while (*backend_arg)
> +				++backend_arg;
> +
> +			new_backend_arg = xmalloc(21 + strlen(refs_backend_type));
> +			sprintf(new_backend_arg, "--refs-backend-type=%s",
> +				refs_backend_type);
> +			*backend_arg = new_backend_arg;
> +		}
>  		err = run_command_v_opt(argv_submodule, RUN_GIT_CMD);
> +		if (refs_backend_type) {
> +			free(new_backend_arg);
> +			*backend_arg = NULL;
> +		}
> +	}

I sent a separate email about this hunk:


http://mid.gmane.org/c21eb4a5d3a3a4886c45da0abe307fe1772e932e.1450869637.git.mhagger@alum.mit.edu

>  
>  	return err;
>  }
> @@ -744,6 +762,11 @@ static void write_config(struct string_list *config)
>  					       write_one_config, NULL) < 0)
>  			die("unable to write parameters to config file");
>  	}
> +
> +	if (refs_backend_type &&
> +	    write_one_config("core.refsBackendType",
> +			     refs_backend_type, NULL) < 0)
> +		die("unable to write backend parameter to config file");
>  }
>  
>  static void write_refspec_config(const char *src_ref_prefix,
> diff --git a/builtin/submodule--helper.c b/builtin/submodule--helper.c
> index f4c3eff..5c9ca4e 100644
> --- a/builtin/submodule--helper.c
> +++ b/builtin/submodule--helper.c
> @@ -140,7 +140,10 @@ static int clone_submodule(const char *path, const char *gitdir, const char *url
>  		argv_array_pushl(&cp.args, "--reference", reference, NULL);
>  	if (gitdir && *gitdir)
>  		argv_array_pushl(&cp.args, "--separate-git-dir", gitdir, NULL);
> -
> +	if (refs_backend_type && *refs_backend_type) {
> +		argv_array_push(&cp.args, "--refs-backend-type");
> +		argv_array_push(&cp.args, refs_backend_type);
> +	}

This could be shortened to

    argv_array_pushf(&cp.args, "--refs-backend-type=%s", refs_backend_type);

>  	argv_array_push(&cp.args, url);
>  	argv_array_push(&cp.args, path);
>  
> diff --git a/cache.h b/cache.h
> index d1534db..8f2ca55 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -695,6 +695,7 @@ enum object_creation_mode {
>  extern enum object_creation_mode object_creation_mode;
>  
>  extern char *notes_ref_name;
> +extern const char *refs_backend_type;
>  
>  extern const char *refs_backend_type;

Michael

-- 
Michael Haggerty
mhagger@alum.mit.edu