Re: [PATCH v2 06/30] loose: compatibilty short name support

From: Patrick Steinhardt <ps@pks.im>
Date: Feb 15, 2024, 11:22 UTC
Message-ID: <Zc3z4YWYybC0Xi2u@tanuki>
In-Reply-To: <20231002024034.2611-6-ebiederm@gmail.com>

On Sun, Oct 01, 2023 at 09:40:10PM -0500, Eric W. Biederman wrote:

Show 38 quoted lines

> From: "Eric W. Biederman" <ebiederm@xmission.com>
> 
> Update loose_objects_cache when udpating the loose objects map.  This
> oidtree is used to discover which oids are possibilities when
> resolving short names, and it can support a mixture of sha1
> and sha256 oids.
> 
> With this any oid recorded objects/loose-objects-idx is usable
> for resolving an oid to an object.
> 
> To make this maintainable a helper insert_loose_map is factored
> out of load_one_loose_object_map and repo_add_loose_object_map,
> and then modified to also update the loose_objects_cache.
> 
> Signed-off-by: "Eric W. Biederman" <ebiederm@xmission.com>
> ---
>  loose.c | 37 +++++++++++++++++++++++++------------
>  1 file changed, 25 insertions(+), 12 deletions(-)
> 
> diff --git a/loose.c b/loose.c
> index 6ba73cc84dca..f6faa6216a08 100644
> --- a/loose.c
> +++ b/loose.c
> @@ -7,6 +7,7 @@
>  #include "gettext.h"
>  #include "loose.h"
>  #include "lockfile.h"
> +#include "oidtree.h"
>  
>  static const char *loose_object_header = "# loose-object-idx\n";
>  
> @@ -42,6 +43,21 @@ static int insert_oid_pair(kh_oid_map_t *map, const struct object_id *key, const
>  	return 1;
>  }
>  
> +static int insert_loose_map(struct object_directory *odb,
> +			    const struct object_id *oid,
> +			    const struct object_id *compat_oid)

I think it would've been nice to fold this into the preceding patch already. At least I wanted to propose adding such a function to avoid the duplication down below.

Patrick

Show 61 quoted lines

> +{
> +	struct loose_object_map *map = odb->loose_map;
> +	int inserted = 0;
> +
> +	inserted |= insert_oid_pair(map->to_compat, oid, compat_oid);
> +	inserted |= insert_oid_pair(map->to_storage, compat_oid, oid);
> +	if (inserted)
> +		oidtree_insert(odb->loose_objects_cache, compat_oid);
> +
> +	return inserted;
> +}
> +
>  static int load_one_loose_object_map(struct repository *repo, struct object_directory *dir)
>  {
>  	struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
> @@ -49,15 +65,14 @@ static int load_one_loose_object_map(struct repository *repo, struct object_dire
>  
>  	if (!dir->loose_map)
>  		loose_object_map_init(&dir->loose_map);
> +	if (!dir->loose_objects_cache) {
> +		ALLOC_ARRAY(dir->loose_objects_cache, 1);
> +		oidtree_init(dir->loose_objects_cache);
> +	}
>  
> -	insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->empty_tree, repo->compat_hash_algo->empty_tree);
> -	insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->empty_tree, repo->hash_algo->empty_tree);
> -
> -	insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);
> -	insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->empty_blob, repo->hash_algo->empty_blob);
> -
> -	insert_oid_pair(dir->loose_map->to_compat, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);
> -	insert_oid_pair(dir->loose_map->to_storage, repo->compat_hash_algo->null_oid, repo->hash_algo->null_oid);
> +	insert_loose_map(dir, repo->hash_algo->empty_tree, repo->compat_hash_algo->empty_tree);
> +	insert_loose_map(dir, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);
> +	insert_loose_map(dir, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);
>  
>  	strbuf_git_common_path(&path, repo, "objects/loose-object-idx");
>  	fp = fopen(path.buf, "rb");
> @@ -77,8 +92,7 @@ static int load_one_loose_object_map(struct repository *repo, struct object_dire
>  		    parse_oid_hex_algop(p, &compat_oid, &p, repo->compat_hash_algo) ||
>  		    p != buf.buf + buf.len)
>  			goto err;
> -		insert_oid_pair(dir->loose_map->to_compat, &oid, &compat_oid);
> -		insert_oid_pair(dir->loose_map->to_storage, &compat_oid, &oid);
> +		insert_loose_map(dir, &oid, &compat_oid);
>  	}
>  
>  	strbuf_release(&buf);
> @@ -197,8 +211,7 @@ int repo_add_loose_object_map(struct repository *repo, const struct object_id *o
>  	if (!should_use_loose_object_map(repo))
>  		return 0;
>  
> -	inserted |= insert_oid_pair(repo->objects->odb->loose_map->to_compat, oid, compat_oid);
> -	inserted |= insert_oid_pair(repo->objects->odb->loose_map->to_storage, compat_oid, oid);
> +	inserted = insert_loose_map(repo->objects->odb, oid, compat_oid);
>  	if (inserted)
>  		return write_one_object(repo, oid, compat_oid);
>  	return 0;
> -- 
> 2.41.0
>