Re: [PATCH 2/2] odb/source-loose: fix existence checks for newly written objects
- From
Junio C Hamano <gitster@pobox.com>
- Date
- Nov 4, 2026, 17:47 UTC
- 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:
Show 32 quoted lines
> @@ -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.