[PATCH v2 0/3] Don't lazy-fetch commits when parsing them
- From
- Jonathan Tan <jonathantanmy@google.com>
- Date
- Dec 7, 2022, 00:40 UTC
- Message-ID
- <cover.1670373420.git.jonathantanmy@google.com>
- In-Reply-To
- <cover.1669839849.git.jonathantanmy@google.com>
Thanks everyone for taking a look. In the end, I thought it best to bite the bullet and move all the corruption diagnostics to where they are detected instead of re-checking (and thus relying on errno to be preserved) after we have found out that we couldn't read the object. This does mean a reworking of all the earlier patches, but overall I think that this puts the code in a better state.
I have also verified that this passes CI [1].
[1] https://github.com/jonathantanmy/git/actions/runs/3634359088
Jonathan Tan (3): object-file: don't exit early if skipping loose object-file: emit corruption errors when detected commit: don't lazy-fetch commits
commit.c | 15 +++++++-- object-file.c | 84 ++++++++++++++++++++++++++------------------------ object-store.h | 3 ++ 3 files changed, 59 insertions(+), 43 deletions(-)
Range-diff against v1:
1: 3a00bc45fd < -: ---------- object-file: reread object with exact same args
2: 9999e127a0 < -: ---------- object-file: refactor corrupt object diagnosis
3: 28c7ee2f8c < -: ---------- object-file: refactor replace object lookup
-: ---------- > 1: 9ad34a1dce object-file: don't exit early if skipping loose
-: ---------- > 2: 9ddfff3585 object-file: emit corruption errors when detected
4: 0607fa67d1 ! 3: c5fe42deb0 commit: don't lazy-fetch commits
@@ commit.c: int repo_parse_commit_internal(struct repository *r,
enum object_type type;
void *buffer;
unsigned long size;
-+ const struct object_id *real_oid;
+ struct object_info oi = {
+ .typep = &type,
+ .sizep = &size,
+ .contentp = &buffer,
-+ .real_oidp = &real_oid,
+ };
++ /*
++ * Git does not support partial clones that exclude commits, so set
++ * OBJECT_INFO_SKIP_FETCH_OBJECT to fail fast when an object is missing.
++ */
++ int flags = OBJECT_INFO_LOOKUP_REPLACE | OBJECT_INFO_SKIP_FETCH_OBJECT |
++ OBJECT_INFO_DIE_IF_CORRUPT;
int ret;
if (!item)
@@ commit.c: int repo_parse_commit_internal(struct repository *r,
- buffer = repo_read_object_file(r, &item->object.oid, &type, &size);
- if (!buffer)
+
-+ /*
-+ * Git does not support partial clones that exclude commits, so set
-+ * OBJECT_INFO_SKIP_FETCH_OBJECT to fail fast when an object is missing.
-+ */
-+ if (oid_object_info_extended(r, &item->object.oid, &oi,
-+ OBJECT_INFO_LOOKUP_REPLACE | OBJECT_INFO_SKIP_FETCH_OBJECT) < 0) {
-+ die_if_corrupt(r, &item->object.oid, real_oid);
++ if (oid_object_info_extended(r, &item->object.oid, &oi, flags) < 0)
return quiet_on_missing ? -1 :
error("Could not read %s",
oid_to_hex(&item->object.oid));
-+ }
- if (type != OBJ_COMMIT) {
- free(buffer);
- return error("Object %s not a commit",-- 2.39.0.rc0.267.gcb52ba06e7-goog