Re: [PATCH 1/5] loose: load loose object map for the correct source

On 26/07/24 05:48AM, Patrick Steinhardt wrote:
> When loading the loose object map via `load_one_loose_object_map()` we
> pass in both a repository and the corresponding source. We ultimately
> don't really respect the passed-in source though as we instead always
> load the map via the common directory. This doesn't make any sense
> though, as the function is called in a loop through all sources, and as
> such the expectation is that we'll load the map that belongs to the
> given source.
> 
> Fix this bug by instead loading the map via the loose source's path.

IIUC the primary source is always being used, does this mean that
repositories using a compat hash and alternates are currently broken?

> Signed-off-by: Patrick Steinhardt <[email protected]>
> ---
>  loose.c | 18 ++++++++++--------
>  1 file changed, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/loose.c b/loose.c
> index bf01d3e42d..9dad75373b 100644
> --- a/loose.c
> +++ b/loose.c
> @@ -61,9 +61,11 @@ static int insert_loose_map(struct odb_source_loose *loose,
>  	return inserted;
>  }
>  
> -static int load_one_loose_object_map(struct repository *repo, struct odb_source_loose *loose)
> +static int load_one_loose_object_map(struct odb_source_loose *loose)
>  {
> -	struct strbuf buf = STRBUF_INIT, path = STRBUF_INIT;
> +	struct repository *repo = loose->base.odb->repo;

Ok, we really only need the repository to know the hash algo, but we can
get this from the loose source.

> +	struct strbuf buf = STRBUF_INIT;
> +	char *path;
>  	FILE *fp;
>  	int ret = -1;
>  
> @@ -78,10 +80,10 @@ static int load_one_loose_object_map(struct repository *repo, struct odb_source_
>  	insert_loose_map(loose, repo->hash_algo->empty_blob, repo->compat_hash_algo->empty_blob);
>  	insert_loose_map(loose, repo->hash_algo->null_oid, repo->compat_hash_algo->null_oid);
>  
> -	repo_common_path_replace(repo, &path, "objects/loose-object-idx");
> -	fp = fopen(path.buf, "rb");
> +	path = xstrfmt("%s/loose-object-idx", loose->base.path);

Now we use the correct path per source. Looks good.

-Justin