Re: [PATCH v3 6/6] odb: make creation of on-disk structures pluggable

Patrick Steinhardt <[email protected]> writes:

> When creating a new "files" object database source we have to create a
> couple of directories. These directories are of course specific to this
> particular backend, and a different backend may require a setup that is
> completely different.
>
> Make the creation of on-disk structures pluggable to accommodate for
> this.
>
> Note that there is one exception though: the "objects" directory must
> exist in a repository regardless of which backend is in use. If it
> doesn't exist then the repository is not treated as a Git repository at
> all. Consequently, we create this directory regardless of the backend.
>
> Signed-off-by: Patrick Steinhardt <[email protected]>
> ---
>  odb/source-files.c | 19 +++++++++++++++++++
>  odb/source.h       | 23 +++++++++++++++++++++++
>  setup.c            | 34 ++++++++++++++++++----------------
>  3 files changed, 60 insertions(+), 16 deletions(-)
>
> diff --git a/odb/source-files.c b/odb/source-files.c
> index 4138758511..0db6e681fe 100644
> --- a/odb/source-files.c
> +++ b/odb/source-files.c
> @@ -9,6 +9,7 @@
>  #include "odb/source-files.h"
>  #include "odb/source-loose.h"
>  #include "packfile.h"
> +#include "path.h"
>  #include "strbuf.h"
>  #include "write-or-die.h"
>  
> @@ -41,6 +42,23 @@ static void odb_source_files_close(struct odb_source *source)
>  	odb_source_close(&files->packed->base);
>  }
>  
> +static int odb_source_files_create_on_disk(struct odb_source *source)
> +{
> +	struct strbuf path = STRBUF_INIT;
> +
> +	safe_create_dir(source->odb->repo, source->path, 1);
> +
> +	strbuf_addf(&path, "%s/pack", source->path);
> +	safe_create_dir(source->odb->repo, path.buf, 1);
> +
> +	strbuf_reset(&path);
> +	strbuf_addf(&path, "%s/info", source->path);
> +	safe_create_dir(source->odb->repo, path.buf, 1);
> +
> +	strbuf_release(&path);
> +	return 0;
> +}
> +
>  static void odb_source_files_prepare(struct odb_source *source,
>  				     enum odb_prepare_flags flags)
>  {
> @@ -271,6 +289,7 @@ struct odb_source_files *odb_source_files_new(struct object_database *odb,
>  
>  	files->base.free = odb_source_files_free;
>  	files->base.close = odb_source_files_close;
> +	files->base.create_on_disk = odb_source_files_create_on_disk;
>  	files->base.prepare = odb_source_files_prepare;
>  	files->base.read_object_info = odb_source_files_read_object_info;
>  	files->base.read_object_stream = odb_source_files_read_object_stream;
> diff --git a/odb/source.h b/odb/source.h
> index ab16d152f4..4abc418bdd 100644
> --- a/odb/source.h
> +++ b/odb/source.h
> @@ -89,6 +89,18 @@ struct odb_source {
>  	 */
>  	void (*close)(struct odb_source *source);
>  
> +	/*
> +	 * This callback is expected to create on-disk data structures that are
> +	 * required for this source to operate.
> +	 *
> +	 * The callback is expected to return 0 on success, a negative error
> +	 * code otherwise.
> +	 *
> +	 * This callback may be NULL in case the source does not need any
> +	 * on-disk setup.
> +	 */
> +	int (*create_on_disk)(struct odb_source *source);
> +
>  	/*
>  	 * This callback is expected to prepare the source so that it becomes
>  	 * ready for use. It optionally clears underlying caches of the object
> @@ -316,6 +328,17 @@ static inline void odb_source_close(struct odb_source *source)
>  	source->close(source);
>  }
>  
> +/*
> + * Create on-disk data structures that are required for this source to operate
> + * correctly. Returns 0 on success, a negative error code otherwise.
> + */
> +static inline int odb_source_create_on_disk(struct odb_source *source)
> +{
> +	if (!source->create_on_disk)
> +		return 0;
> +	return source->create_on_disk(source);
> +}
> +
>  /*
>   * Prepare the object database source and clear any caches. Depending on the
>   * backend used this may have the effect that concurrently-written objects
> diff --git a/setup.c b/setup.c
> index d85171f3b6..af02cd965c 100644
> --- a/setup.c
> +++ b/setup.c
> @@ -2654,25 +2654,27 @@ static int create_default_files(struct repository *repo,
>  
>  static void create_object_database(struct repository *repo)
>  {
> -	struct strbuf path = STRBUF_INIT;
> -	size_t baselen;
> +	/*
> +	 * Create the "objects" directory in the common directory. This is done
> +	 * so that the repository can be discovered regardless of the backend
> +	 * used.
> +	 *
> +	 * Note that we only do this in case the object directory wasn't
> +	 * overwritten via an environment variable. If it _is_ being overridden
> +	 * then we skip this step, as the repository won't be discoverable
> +	 * anyway without the environment variable.
> +	 */
> +	if (!getenv(DB_ENVIRONMENT)) {

It's a bit sad that [PATCH 3/6] removed the use of DB_ENVIRONMENT from
this file, and now we're re-adding it. Although, I don't see how else we
can do this.

> +		struct strbuf objects_dir = STRBUF_INIT;
> +		repo_common_path_append(repo, &objects_dir, "objects");
> +		safe_create_dir(repo, objects_dir.buf, 1);
> +		strbuf_release(&objects_dir);
> +	}
>  
>  	repo->objects = odb_new(repo, ODB_NEW_HONOR_ENV);
>  
> -	strbuf_addstr(&path, repo_get_object_directory(repo));
> -	baselen = path.len;
> -
> -	safe_create_dir(repo, path.buf, 1);
> -
> -	strbuf_setlen(&path, baselen);
> -	strbuf_addstr(&path, "/pack");
> -	safe_create_dir(repo, path.buf, 1);
> -
> -	strbuf_setlen(&path, baselen);
> -	strbuf_addstr(&path, "/info");
> -	safe_create_dir(repo, path.buf, 1);
> -
> -	strbuf_release(&path);
> +	if (odb_source_create_on_disk(repo->objects->sources) < 0)
> +		die("failed creating object database");

This error isn't translatable.

>  }
>  
>  static void separate_git_dir(const char *git_dir, const char *git_link)
>
> -- 
> 2.55.0.679.g6767b8d81c.dirty
>

-- 
Cheers,
Toon