Re: [PATCH 3/5] setup: defer object database creation

Patrick Steinhardt <[email protected]> writes:

> In a subsequent commit we'll make the creation of the on-disk data
> structures of an object database pluggable. This will lead to an
> in-between state where we have already configured the repository's
> object database, but it's not usable yet until we eventually call
> `create_object_directory()`.
>
> Defer the object database creation so that we handle both steps in the
> same function.
>
> Signed-off-by: Patrick Steinhardt <[email protected]>
> ---
>  setup.c | 35 +++++++++++++++++++++++++++--------
>  setup.h |  9 +++++++++
>  2 files changed, 36 insertions(+), 8 deletions(-)
>
> diff --git a/setup.c b/setup.c
> index 825572f5f1..a7b1b9eaef 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -1760,6 +1760,13 @@ enum discovery_result discover_git_directory_reason(struct strbuf *commondir,
>  	return result;
>  }
>  
> +static void get_object_directories(char **object_directory,
> +				   char **alternate_object_directories)
> +{
> +	*object_directory = xstrdup_or_null(getenv(DB_ENVIRONMENT));
> +	*alternate_object_directories = xstrdup_or_null(getenv(ALTERNATE_DB_ENVIRONMENT));
> +}
> +
>  int apply_repository_format(struct repository *repo,
>  			    const struct repository_format *format,
>  			    enum apply_repository_format_flags flags,
> @@ -1779,8 +1786,9 @@ int apply_repository_format(struct repository *repo,
>  	if (flags & APPLY_REPOSITORY_FORMAT_HONOR_ENV) {
>  		const char *shallow_file;
>  
> -		object_directory = xstrdup_or_null(getenv(DB_ENVIRONMENT));
> -		alternate_object_directories = xstrdup_or_null(getenv(ALTERNATE_DB_ENVIRONMENT));
> +		get_object_directories(&object_directory,
> +				       &alternate_object_directories);
> +
>  		shallow_file = getenv(GIT_SHALLOW_FILE_ENVIRONMENT);
>  		if (shallow_file)
>  			set_alternate_shallow_file(repo, shallow_file);

HONOR_ENV still means we read the environment variable to learn where
the object directory (which is admittedly a files backend specific
concept) and alternate object directories (ditto) are.

> @@ -1803,8 +1811,9 @@ int apply_repository_format(struct repository *repo,
>  	repo->repository_format_precious_objects =
>  		format->precious_objects;
>  
> -	repo->objects = odb_new(repo, object_directory,
> -				alternate_object_directories);
> +	if (!(flags & APPLY_REPOSITORY_FORMAT_SKIP_ODB_CREATION))
> +		repo->objects = odb_new(repo, object_directory,
> +					alternate_object_directories);

And SKIP_ODB_CREATION can tell apply_repository_format() not to
create an odb there.

> -static void create_object_directory(struct repository *repo)
> +static void create_object_database(struct repository *repo)
>  {
> +	char *object_directory, *alternate_object_directories;
>  	struct strbuf path = STRBUF_INIT;
>  	size_t baselen;
>  
> +	get_object_directories(&object_directory, &alternate_object_directories);
> +	repo->objects = odb_new(repo, object_directory,
> +				alternate_object_directories);
> +
>  	strbuf_addstr(&path, repo_get_object_directory(repo));
>  	baselen = path.len;
>  
> @@ -2672,6 +2686,8 @@ static void create_object_directory(struct repository *repo)
>  	strbuf_addstr(&path, "/info");
>  	safe_create_dir(repo, path.buf, 1);
>  
> +	free(alternate_object_directories);
> +	free(object_directory);
>  	strbuf_release(&path);
>  }
>  


> @@ -2867,9 +2883,10 @@ int init_db(struct repository *repo,
>  	 */
>  	read_and_verify_repository_format(&repo_fmt, repo_get_git_dir(repo), NULL);
>  	repository_format_configure(&repo_fmt, hash, ref_storage_format);
> -	if (apply_repository_format(repo, &repo_fmt, APPLY_REPOSITORY_FORMAT_HONOR_ENV, &err) < 0)
> +	if (apply_repository_format(repo, &repo_fmt,
> +				    APPLY_REPOSITORY_FORMAT_HONOR_ENV |
> +				    APPLY_REPOSITORY_FORMAT_SKIP_ODB_CREATION, &err) < 0)
>  		die("%s", err.buf);
> -	startup_info->have_repository = 1;

Early in initialization, we no longer recreate the ODB when calling
apply_repository_format(), and we defer declaring that we have a
repository until we call create_object_database().

> @@ -2885,7 +2902,9 @@ int init_db(struct repository *repo,
>  
>  	if (!(flags & INIT_DB_SKIP_REFDB))
>  		create_reference_database(repo, initial_branch, flags & INIT_DB_QUIET);
> -	create_object_directory(repo);
> +	create_object_database(repo);
> +
> +	startup_info->have_repository = 1;

Instead we call create_object_database() rather late, after we
finish creating leading directories and default files and processing
the configuration.  I guess this is a prelude to specifying "no, we
are not doing the files backend but are using this new thing" in the
global configuration?

> diff --git a/setup.h b/setup.h
> index 654f10e059..e55d647b70 100644
> --- a/setup.h
> +++ b/setup.h
> @@ -241,6 +241,15 @@ enum apply_repository_format_flags {
>  	 * relate to the object database.
>  	 */
>  	APPLY_REPOSITORY_FORMAT_HONOR_ENV = (1 << 0),
> +
> +	/*
> +	 * Usually, the object database is created after the repository format
> +	 * was applied. This step is skipped if this flag is set, which leaves
> +	 * us with a partially-working repository.
> +	 *
> +	 * This is useful when initializing a new repository.
> +	 */
> +	APPLY_REPOSITORY_FORMAT_SKIP_ODB_CREATION = (1 << 1),
>  };

OK.