[PATCH 7/7] 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 tought 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 <[email protected]>
---
 odb.c                        | 46 ++++++++++++++++++++++++++++++--------------
 odb/source-loose.c           |  5 -----
 packfile.c                   | 17 ----------------
 packfile.h                   |  1 -
 t/t1060-object-corruption.sh | 18 +++++++++++++++++
 5 files changed, 50 insertions(+), 37 deletions(-)

diff --git a/odb.c b/odb.c
index 6cb0a9534b..206988f39b 100644
--- a/odb.c
+++ b/odb.c
@@ -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 int do_oid_object_info_extended(struct object_database *odb,
 				       const struct object_id *oid,
 				       struct object_info *oi, unsigned flags)
 {
+	struct strbuf corrupt_err = STRBUF_INIT;
 	const struct object_id *real = oid;
 	int already_retried = 0;
+	bool corrupt = false;
+	int ret;
 
 	if (flags & OBJECT_INFO_LOOKUP_REPLACE)
 		real = lookup_replace_object(odb->repo, oid);
@@ -568,9 +570,14 @@ static int do_oid_object_info_extended(struct object_database *odb,
 	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 < 0)
+				corrupt = true;
+		}
 
 		/*
 		 * When the object hasn't been found we try a second read and
@@ -578,11 +585,15 @@ static int do_oid_object_info_extended(struct object_database *odb,
 		 * 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 < 0)
+					corrupt = true;
+			}
 		}
 
 		/*
@@ -605,16 +616,23 @@ static int do_oid_object_info_extended(struct object_database *odb,
 		}
 
 		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 = -1;
+		goto out;
 	}
+
+out:
+	strbuf_release(&corrupt_err);
+	return ret;
 }
 
 static int oid_object_info_convert(struct repository *r,
diff --git a/odb/source-loose.c b/odb/source-loose.c
index 3cee012a6d..8ca5a78858 100644
--- a/odb/source-loose.c
+++ b/odb/source-loose.c
@@ -195,11 +195,6 @@ static int read_object_info_from_path(struct odb_source_loose *loose,
 	if (ret < 0 && errmsg)
 		strbuf_addf(errmsg, _("loose object %s (stored in %s) is corrupt"),
 			    oid_to_hex(oid), path);
-
-	if (ret < 0 && (flags & OBJECT_INFO_DIE_IF_CORRUPT))
-		die(_("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)
diff --git a/packfile.c b/packfile.c
index 3cde39a01c..cd38be088d 100644
--- a/packfile.c
+++ b/packfile.c
@@ -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,
diff --git a/packfile.h b/packfile.h
index 3229a6ed47..573fe003d0 100644
--- a/packfile.h
+++ b/packfile.h
@@ -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,
diff --git a/t/t1060-object-corruption.sh b/t/t1060-object-corruption.sh
index 502a5ea1c5..d2ef468b45 100755
--- a/t/t1060-object-corruption.sh
+++ b/t/t1060-object-corruption.sh
@@ -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

-- 
2.55.0.822.g20453c30eb.dirty