odb: handle `OBJECT_INFO_DIE_IF_CORRUPT` generically

When a lookup with `OBJECT_INFO_DIE_IF_CORRUPT` fails we want to die in
case the object exists, but cannot be read. This flag is handled in two
different spots right now:

  - `do_oid_object_info_extended()` calls `has_packed_and_bad()` to
    check whether the object is known to be corrupt in any packfile.
    This function reaches into the internals of the packed source and
    thus breaks the abstraction provided by our object sources.

  - The loose source handles the flag itself and dies directly in
    `read_object_info_from_path()`, which means that we die even in
    cases where another source may still have a good copy of the
    object.

Besides being inconsistent, it also ties us to the specific backend used
by the database sources because `has_packed_and_bad()` assumes that they
use the "files" backend. Any other backend will instead cause us to die
when calling `odb_source_files_downcast()`, even if the object was
simply nonexistent.

In the preceding commits we've carved out the infrastructure to make
this mechanism fully generic. On the one hand, all backends now tell us
whether the object is missing or corrupt via their return values. And
on the other hand, they have been taught to provide a readable error
message to the caller.

Adapt `do_oid_object_info_extended()` to use those new mechanisms. This
means that we won't die immediately anymore when a loose object is
corrupt, and we properly handle backends other than the "files" backend.

Signed-off-by: Patrick Steinhardt <ps@pks.im>
Signed-off-by: Junio C Hamano <gitster@pobox.com>
main
Patrick Steinhardt 2026-08-19 14:17:23 +02:00 committed by Junio C Hamano
parent 63a3257352
commit 2135b14863
5 changed files with 52 additions and 40 deletions

46
odb.c
View File

@ -15,7 +15,6 @@
#include "object-name.h"
#include "odb.h"
#include "odb/source-inmemory.h"
#include "packfile.h"
#include "path.h"
#include "promisor-remote.h"
#include "quote.h"
@ -551,8 +550,11 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *
const struct object_id *oid,
struct object_info *oi, unsigned flags)
{
struct strbuf corrupt_err = STRBUF_INIT;
const struct object_id *real = oid;
enum odb_read_status ret;
int already_retried = 0;
bool corrupt = false;

if (flags & OBJECT_INFO_LOOKUP_REPLACE)
real = lookup_replace_object(odb->repo, oid);
@ -568,9 +570,14 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *
while (1) {
struct odb_source *source;

for (source = odb->sources; source; source = source->next)
if (!odb_source_read_object_info(source, real, oi, flags, NULL))
return 0;
for (source = odb->sources; source; source = source->next) {
ret = odb_source_read_object_info(source, real, oi, flags,
corrupt_err.len ? NULL : &corrupt_err);
if (!ret)
goto out;
if (ret != ODB_READ_NOT_FOUND)
corrupt = true;
}

/*
* When the object hasn't been found we try a second read and
@ -578,11 +585,15 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *
* caches or reload on-disk state.
*/
if (!(flags & OBJECT_INFO_QUICK)) {
for (source = odb->sources; source; source = source->next)
if (!odb_source_read_object_info(source, real, oi,
flags | OBJECT_INFO_SECOND_READ,
NULL))
return 0;
for (source = odb->sources; source; source = source->next) {
ret = odb_source_read_object_info(source, real, oi,
flags | OBJECT_INFO_SECOND_READ,
corrupt_err.len ? NULL : &corrupt_err);
if (!ret)
goto out;
if (ret != ODB_READ_NOT_FOUND)
corrupt = true;
}
}

/*
@ -605,16 +616,23 @@ static enum odb_read_status do_oid_object_info_extended(struct object_database *
}

if (flags & OBJECT_INFO_DIE_IF_CORRUPT) {
const struct packed_git *p;
if ((flags & OBJECT_INFO_LOOKUP_REPLACE) && !oideq(real, oid))
die(_("replacement %s not found for %s"),
oid_to_hex(real), oid_to_hex(oid));
if ((p = has_packed_and_bad(odb->repo, real)))
die(_("packed object %s (stored in %s) is corrupt"),
oid_to_hex(real), p->pack_name);
if (corrupt) {
if (corrupt_err.len)
die("%s", corrupt_err.buf);
die(_("object %s is corrupt"), oid_to_hex(real));
}
}
return -1;

ret = corrupt ? ODB_READ_ERROR : ODB_READ_NOT_FOUND;
goto out;
}

out:
strbuf_release(&corrupt_err);
return ret;
}

static int oid_object_info_convert(struct repository *r,

View File

@ -192,15 +192,9 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
ret = 0;

out:
if (ret && ret != ODB_READ_NOT_FOUND) {
if ((flags & OBJECT_INFO_DIE_IF_CORRUPT))
die(_("loose object %s (stored in %s) is corrupt"),
if (ret && ret != ODB_READ_NOT_FOUND && errmsg)
strbuf_addf(errmsg, _("loose object %s (stored in %s) is corrupt"),
oid_to_hex(oid), path);
if (errmsg)
strbuf_addf(errmsg, _("loose object %s (stored in %s) is corrupt"),
oid_to_hex(oid), path);
}

if (stream_to_end)
git_inflate_end(stream_to_end);
if (map)

View File

@ -985,23 +985,6 @@ void mark_bad_packed_object(struct packed_git *p, const struct object_id *oid)
oidset_insert(&p->bad_objects, oid);
}

const struct packed_git *has_packed_and_bad(struct repository *r,
const struct object_id *oid)
{
struct odb_source *source;

for (source = r->objects->sources; source; source = source->next) {
struct odb_source_files *files = odb_source_files_downcast(source);
struct packfile_list_entry *e;

for (e = files->packed->packs.head; e; e = e->next)
if (oidset_contains(&e->pack->bad_objects, oid))
return e->pack;
}

return NULL;
}

off_t get_delta_base(struct packed_git *p,
struct pack_window **w_curs,
off_t *curpos,

View File

@ -329,7 +329,6 @@ int packed_object_info_with_index_pos(struct odb_source_packed *source,
uint32_t *maybe_index_pos, struct object_info *oi);

void mark_bad_packed_object(struct packed_git *, const struct object_id *);
const struct packed_git *has_packed_and_bad(struct repository *, const struct object_id *);

int has_object_pack(struct repository *r, const struct object_id *oid);
int has_object_kept_pack(struct repository *r, const struct object_id *oid,

View File

@ -145,4 +145,22 @@ test_expect_success 'partial clone of corrupted repository' '
test_must_fail git -C corrupt-partial checkout --force
'

test_expect_success 'corrupted loose commit can be read from alternate' '
git init repo-a &&
tree=$(git -C repo-a write-tree) &&
commit=$(git -C repo-a commit-tree $tree </dev/null) &&

cp -r repo-a repo-b &&
(
cd repo-b &&
echo ../../../repo-a/.git/objects >.git/objects/info/alternates &&
corrupt_byte "$commit" 1
) &&

git -C repo-a cat-file -p "$commit" >expect &&
git -C repo-b cat-file -p "$commit" >actual 2>err &&
test_cmp expect actual &&
test_grep "inflate: data stream error" err
'

test_done