From: Junio C Hamano <gitster@pobox.com>
Date: Wed, 04 Nov 2026 17:47:49 GMT
Subject: Re: [PATCH 2/2] odb/source-loose: fix existence checks for newly written objects
Message-ID: <xmqqfqxg32uy.fsf@gitster.g>
In-Reply-To: <20261104-pks-odb-loose-stale-cache-v1-2-a00d5cdb5e7d@pks.im>

Patrick Steinhardt <ps@pks.im> writes:

> @@ -824,8 +837,13 @@ static int write_loose_object(struct odb_source_loose *loose,
>  			warning_errno(_("failed utime() on %s"), tmp_file.buf);
>  	}
>  
> -	return finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
> -					  FOF_SKIP_COLLISION_CHECK);
> +	ret = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
> +					 FOF_SKIP_COLLISION_CHECK);
> +	if (ret < 0)
> +		return ret;
> +
> +	loose_cache_mark_seen(loose, oid);
> +	return 0;
>  }
>  
>  static int odb_source_loose_write_object(struct odb_source *source,
> @@ -967,8 +985,14 @@ static int odb_source_loose_write_object_stream(struct odb_source *source,
>  
>  	err = finalize_object_file_flags(loose->base.odb->repo, tmp_file.buf, filename.buf,
>  					 FOF_SKIP_COLLISION_CHECK);
> -	if (!err && compat)
> +	if (err < 0)
> +		goto cleanup;
> +
> +	loose_cache_mark_seen(loose, oid);
> +
> +	if (compat)
>  		err = repo_add_loose_object_map(loose, oid, &compat_oid);
> +
>  cleanup:
>  	strbuf_release(&tmp_file);
>  	strbuf_release(&filename);

Not about this change but the lack of "if (compat)" stuff in the
previous hunk puzzled me.  It turns out that the logic to write a
single loose object is so big that write_loose_object(), which is
the subject of the previous hunk, helps another function,
odb_source_loose_write_object(), by being its helper function, and
this caller has the corresponding "if (compat)" logic to produce
the hash interoperability data.

Looking very good.