On 2020-02-06 at 22:55:56, Han-Wen Nienhuys via GitGitGadget wrote:
> @@ -403,6 +417,10 @@ int init_db(const char *git_dir, const char *real_git_dir,
>  		git_config_set("receive.denyNonFastforwards", "true");
>  	}
>  
> +	if (flags & INIT_DB_REFTABLE) {
> +		git_config_set("extensions.refStorage", "reftable");
> +	}

This does seem like the best way to do this.  Can we get an addition to
Documentation/technical/repository-version.txt that documents this
extension and the values it takes?  I presume "reftable" and "files" are
options, but it would be nice it folks didn't have to dig through the
code to find that out.

One side note: we typically omit the braces for a single-line if.

> +
>  	if (!(flags & INIT_DB_QUIET)) {
>  		int len = strlen(git_dir);
>  
> @@ -481,15 +499,18 @@ int cmd_init_db(int argc, const char **argv, const char *prefix)
>  	const char *template_dir = NULL;
>  	unsigned int flags = 0;
>  	const struct option init_db_options[] = {
> -		OPT_STRING(0, "template", &template_dir, N_("template-directory"),
> -				N_("directory from which templates will be used")),
> +		OPT_STRING(0, "template", &template_dir,
> +			   N_("template-directory"),
> +			   N_("directory from which templates will be used")),
>  		OPT_SET_INT(0, "bare", &is_bare_repository_cfg,
> -				N_("create a bare repository"), 1),
> +			    N_("create a bare repository"), 1),
>  		{ OPTION_CALLBACK, 0, "shared", &init_shared_repository,
> -			N_("permissions"),
> -			N_("specify that the git repository is to be shared amongst several users"),
> -			PARSE_OPT_OPTARG | PARSE_OPT_NONEG, shared_callback, 0},
> +		  N_("permissions"),
> +		  N_("specify that the git repository is to be shared amongst several users"),
> +		  PARSE_OPT_OPTARG | PARSE_OPT_NONEG, shared_callback, 0 },
>  		OPT_BIT('q', "quiet", &flags, N_("be quiet"), INIT_DB_QUIET),
> +		OPT_BIT(0, "reftable", &flags, N_("use reftable"),
> +			INIT_DB_REFTABLE),

I wonder if this might be better as --refs-backend={files|reftable}.  If
reftable becomes the default at some point in the future, it would be
easier to let people explicitly specify that they want a non-reftable
version.  If we learn support for a hypothetical reftable v2 in the
future, maybe we'd want to let folks specify that instead.

>  		OPT_STRING(0, "separate-git-dir", &real_git_dir, N_("gitdir"),
>  			   N_("separate git dir from working tree")),
>  		OPT_END()
> diff --git a/cache.h b/cache.h
> index cbfaead23a..929a61e861 100644
> --- a/cache.h
> +++ b/cache.h
> @@ -625,6 +625,7 @@ int path_inside_repo(const char *prefix, const char *path);
>  
>  #define INIT_DB_QUIET 0x0001
>  #define INIT_DB_EXIST_OK 0x0002
> +#define INIT_DB_REFTABLE 0x0004
>  
>  int init_db(const char *git_dir, const char *real_git_dir,
>  	    const char *template_dir, unsigned int flags);
> @@ -1041,6 +1042,7 @@ struct repository_format {
>  	int is_bare;
>  	int hash_algo;
>  	char *work_tree;
> +	char *ref_storage;
>  	struct string_list unknown_extensions;
>  };
>  
> diff --git a/refs.c b/refs.c
> index 1ab0bb54d3..e4e5af8ea0 100644
> --- a/refs.c
> +++ b/refs.c
> @@ -20,7 +20,7 @@
>  /*
>   * List of all available backends
>   */
> -static struct ref_storage_be *refs_backends = &refs_be_files;
> +static struct ref_storage_be *refs_backends = &refs_be_reftable;

I'm not sure why we're making this change.  It doesn't look like the
default is changing.

> @@ -1913,7 +1916,7 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
>  		goto done;
>  
>  	/* assume that add_submodule_odb() has been called */
> -	refs = ref_store_init(submodule_sb.buf,
> +	refs = ref_store_init(submodule_sb.buf, "files", /* XXX */
>  			      REF_STORE_READ | REF_STORE_ODB);
>  	register_ref_store_map(&submodule_ref_stores, "submodule",
>  			       refs, submodule);
> @@ -1927,6 +1930,7 @@ struct ref_store *get_submodule_ref_store(const char *submodule)
>  
>  struct ref_store *get_worktree_ref_store(const struct worktree *wt)
>  {
> +	const char *format = "files"; /* XXX */

If the question is whether we still want to default to "files" as the
default, then yes, I think we do.  Suddenly changing things would be a
problem if for some reason someone needed to downgrade Git versions.

Since we have two instances of "files" above, it might be better to
create a #define like DEFAULT_REF_BACKEND or some such.
-- 
brian m. carlson: Houston, Texas, US
OpenPGP: https://keybase.io/bk2204