From: Karthik Nayak <karthik.188@gmail.com>
Date: Fri, 30 Oct 2026 11:55:10 GMT
Subject: Re: [PATCH v3 4/4] tmp-objdir: merge loose object index when migrating tmpdir
Message-ID: <CAOLa=ZR2AmC4EDM7V2A9UainUZ5md=dQT_pnJycxuqV7BhfxxA@mail.gmail.com>
In-Reply-To: <20261029232220.1245827-5-jltobler@gmail.com>

Justin Tobler <jltobler@gmail.com> writes:

> diff --git a/loose.c b/loose.c
> index f2e111c25e..0ce9c7486e 100644
> --- a/loose.c
> +++ b/loose.c
> @@ -311,3 +311,50 @@ void loose_object_map_clear(struct loose_object_map **map)
>  	free(m);
>  	*map = NULL;
>  }
> +
> +int loose_object_idx_merge(struct repository *repo, const char *src,
> +			   const char *dst)
>

Nit: should we name this `loose_object_map_merge()` to stay consistent
with the other `loose_object_map...()` functions?

> +{
> +	struct lock_file lock = LOCK_INIT;
> +	struct loose_object_map *map;
> +	int ret;
> +	int fd;
> +
> +	loose_object_map_init(&map);
> +
> +	if (loose_object_map_read(repo, src, map, NULL) < 0) {
> +		ret = error(_("unable to read loose object index %s"), src);
> +		goto out;
> +	}
> +
> +	fd = repo_hold_lock_file_for_update_timeout(repo, &lock, dst, 0,
> +						    LOOSE_OBJECT_IDX_LOCK_TIMEOUT_MS);
> +	if (fd < 0) {
> +		ret = error_errno(_("unable to lock %s"), dst);
> +		goto out;
> +	}
> +
> +	if (loose_object_map_read(repo, dst, map, NULL) < 0) {
> +		ret = error(_("unable to read loose object index %s"), dst);
> +		goto out;
> +	}
> +
> +	if (loose_object_map_write(repo, map, fd) < 0) {
> +		ret = error_errno(_("unable to write %s"),
> +				  get_lock_file_path(&lock));
> +		goto out;
> +	}
> +
> +	if (commit_lock_file(&lock) < 0) {
> +		ret = error_errno(_("unable to write %s"), dst);

Nit: Shouldn't this be unable to commit/rename? This will differentiate
from the above write error

> +		goto out;
> +	}
> +
> +	ret = 0;
> +
> +out:
> +	loose_object_map_clear(&map);
> +	rollback_lock_file(&lock);
> +
> +	return ret;
> +}

The rest looks to be in order.