Re: [PATCH 03/18] odb/source-loose: start converting to a proper `struct odb_source`

[Junio C Hamano <gitster@pobox.com>]

Patrick Steinhardt <ps@pks.im> writes:

> Start converting `struct odb_source_loose` into a proper pluggable
> `struct odb_source` by embedding the base struct and assigning it the
> new `ODB_SOURCE_LOOSE` type. Furthermore, wire up lifecycle management
> of this source by implementing the `free` callback and taking ownership
> of the chdir notifications.
>
> Note that the loose source is not yet functional as a standalone `struct
> odb_source`, as it's missing all of the callback implementations. These
> will be wired up in subsequent commits.
>
> Signed-off-by: Patrick Steinhardt <ps@pks.im>
> ---
>  object-file.c      | 17 -----------------
>  object-file.h      |  2 --
>  odb/source-files.c |  2 +-
>  odb/source-loose.c | 45 +++++++++++++++++++++++++++++++++++++++++++++
>  odb/source-loose.h | 14 ++++++++++++++
>  odb/source.h       |  3 +++
>  6 files changed, 63 insertions(+), 20 deletions(-)
>
> diff --git a/object-file.c b/object-file.c
> index 7a1908bfc0..977d959d33 100644
> --- a/object-file.c
> +++ b/object-file.c
> @@ -2041,14 +2041,6 @@ static struct oidtree *odb_source_loose_cache(struct odb_source *source,
>  	return files->loose->cache;
>  }
>  
> -static void odb_source_loose_clear_cache(struct odb_source_loose *loose)
> -{
> -	oidtree_clear(loose->cache);
> -	FREE_AND_NULL(loose->cache);
> -	memset(&loose->subdir_seen, 0,
> -	       sizeof(loose->subdir_seen));
> -}
> -
>  void odb_source_loose_reprepare(struct odb_source *source)
>  {
>  	struct odb_source_files *files = odb_source_files_downcast(source);
> @@ -2205,15 +2197,6 @@ struct odb_transaction *odb_transaction_files_begin(struct odb_source *source)
>  	return &transaction->base;
>  }
>  
> -void odb_source_loose_free(struct odb_source_loose *loose)
> -{
> -	if (!loose)
> -		return;
> -	odb_source_loose_clear_cache(loose);
> -	loose_object_map_clear(&loose->map);
> -	free(loose);
> -}
> -
>  struct odb_loose_read_stream {
>  	struct odb_read_stream base;
>  	git_zstream z;
> diff --git a/object-file.h b/object-file.h
> index 1d8312cf7f..02c9680980 100644
> --- a/object-file.h
> +++ b/object-file.h
> @@ -21,8 +21,6 @@ struct object_info;
>  struct odb_read_stream;
>  struct odb_source;
>  
> -void odb_source_loose_free(struct odb_source_loose *loose);
> -
>  /* Reprepare the loose source by emptying the loose object cache. */
>  void odb_source_loose_reprepare(struct odb_source *source);
>  
> diff --git a/odb/source-files.c b/odb/source-files.c
> index 185cc6903e..ccc637311b 100644
> --- a/odb/source-files.c
> +++ b/odb/source-files.c
> @@ -27,7 +27,7 @@ static void odb_source_files_free(struct odb_source *source)
>  {
>  	struct odb_source_files *files = odb_source_files_downcast(source);
>  	chdir_notify_unregister(NULL, odb_source_files_reparent, files);
> -	odb_source_loose_free(files->loose);
> +	odb_source_free(&files->loose->base);
>  	packfile_store_free(files->packed);
>  	odb_source_release(&files->base);
>  	free(files);
> diff --git a/odb/source-loose.c b/odb/source-loose.c
> index c9e7414814..92e18f5adb 100644
> --- a/odb/source-loose.c
> +++ b/odb/source-loose.c
> @@ -1,10 +1,55 @@
>  #include "git-compat-util.h"
> +#include "abspath.h"
> +#include "chdir-notify.h"
> +#include "loose.h"
> +#include "odb.h"
> +#include "odb/source-files.h"
>  #include "odb/source-loose.h"
> +#include "oidtree.h"
> +
> +void odb_source_loose_clear_cache(struct odb_source_loose *loose)
> +{
> +	oidtree_clear(loose->cache);
> +	FREE_AND_NULL(loose->cache);
> +	memset(&loose->subdir_seen, 0,
> +	       sizeof(loose->subdir_seen));
> +}
> +
> +static void odb_source_loose_reparent(const char *name UNUSED,
> +				      const char *old_cwd,
> +				      const char *new_cwd,
> +				      void *cb_data)
> +{
> +	struct odb_source_loose *loose = cb_data;
> +	char *path = reparent_relative_path(old_cwd, new_cwd,
> +					    loose->base.path);
> +	free(loose->base.path);
> +	loose->base.path = path;
> +}
> +
> +static void odb_source_loose_free(struct odb_source *source)
> +{
> +	struct odb_source_loose *loose = odb_source_loose_downcast(source);
> +	odb_source_loose_clear_cache(loose);
> +	loose_object_map_clear(&loose->map);
> +	chdir_notify_unregister(NULL, odb_source_loose_reparent, loose);
> +	odb_source_release(&loose->base);
> +	free(loose);
> +}
>  
>  struct odb_source_loose *odb_source_loose_new(struct odb_source_files *files)
>  {
>  	struct odb_source_loose *loose;
> +
>  	CALLOC_ARRAY(loose, 1);
> +	odb_source_init(&loose->base, files->base.odb, ODB_SOURCE_LOOSE,
> +			files->base.path, files->base.local);
>  	loose->files = files;
> +
> +	loose->base.free = odb_source_loose_free;
> +
> +	if (!is_absolute_path(loose->base.path))
> +		chdir_notify_register(NULL, odb_source_loose_reparent, loose);
> +
>  	return loose;
>  }
> diff --git a/odb/source-loose.h b/odb/source-loose.h
> index bf61e767c8..441da9e418 100644
> --- a/odb/source-loose.h
> +++ b/odb/source-loose.h
> @@ -12,6 +12,7 @@ struct oidtree;
>   * file per object. This source is part of the files source.
>   */
>  struct odb_source_loose {
> +	struct odb_source base;
>  	struct odb_source_files *files;
>  
>  	/*
> @@ -32,4 +33,17 @@ struct odb_source_loose {
>  
>  struct odb_source_loose *odb_source_loose_new(struct odb_source_files *files);
>  
> +/*
> + * Cast the given object database source to the loose backend. This will cause
> + * a BUG in case the source uses doesn't use this backend.

"uses doesn't use"???

> + */
> +static inline struct odb_source_loose *odb_source_loose_downcast(struct odb_source *source)
> +{
> +	if (source->type != ODB_SOURCE_LOOSE)
> +		BUG("trying to downcast source of type '%d' to loose", source->type);
> +	return container_of(source, struct odb_source_loose, base);
> +}
> +
> +void odb_source_loose_clear_cache(struct odb_source_loose *loose);
> +
>  #endif
> diff --git a/odb/source.h b/odb/source.h
> index 0a440884e4..8bcb67787e 100644
> --- a/odb/source.h
> +++ b/odb/source.h
> @@ -14,6 +14,9 @@ enum odb_source_type {
>  	/* The "files" backend that uses loose objects and packfiles. */
>  	ODB_SOURCE_FILES,
>  
> +	/* The "loose" backend that uses loose objects, only. */
> +	ODB_SOURCE_LOOSE,
> +
>  	/* The "in-memory" backend that stores objects in memory. */
>  	ODB_SOURCE_INMEMORY,
>  };