ba002f3 (builtin-fsck: move common object checking code to fsck.c) did
more than what it claimed to. Most notably, it wrongly made an empty tree
object an error by pretending to only move code from fsck_tree() in
builtin-fsck.c to fsck_tree() in fsck.c, but in fact adding a bogus check
to barf on an empty tree.
An empty tree object is _unusual_. Recent porcelains try reasonably hard
not to let the user create a commit that contains such a tree. Perhaps
warning about them in git-fsck may have some merit.
However, being unusual and being errorneous are two quite different
things. This is especially true now we seem to use the same
fsck_$object() code in places other than git-fsck itself. For example,
receive-pack should not reject unusual objects, even if it would be a good
idea to tighten it to reject incorrect ones.
Signed-off-by: Junio C Hamano <redacted>
---
* I've wasted a few hours tonight hunting for random breakages in "git
push", the symptom of which is "fatal: unresolved deltas left after
unpacking." I was hoping this patch would fix it, but it seems that
the problem is elsewhere.
I'll revert the following two commits for now:
d5ef408 (unpack-objects: prevent writing of inconsistent objects)
28f72a0 (receive-pack: use strict mode for unpacking objects)
as I have verified that running with receive.fsckobjects set to false
fixes the issues for me, and the repository at the receiving end (both
before and after the push) pass git-fsck without problems. Needless to
say, I am not a happy camper right now.
fsck.c | 2 --
1 files changed, 0 insertions(+), 2 deletions(-)
diff --git a/fsck.c b/fsck.c
index 6883d1b..797e317 100644
--- a/fsck.c
+++ b/fsck.c
@@ -155,8 +155,6 @@ static int fsck_tree(struct tree *item, int strict, fsck_error error_func)
o_mode = 0;
o_name = NULL;
o_sha1 = NULL;
- if (!desc.size)
- return error_func(&item->object, FSCK_ERROR, "empty tree");
while (desc.size) {
unsigned mode;--
1.5.4.3.529.gb25fb
On Tue, 04 Mar 2008 03:21:16 -0800 Junio C Hamano wrote:
* I've wasted a few hours tonight hunting for random breakages in "git
push", the symptom of which is "fatal: unresolved deltas left after
unpacking." I was hoping this patch would fix it, but it seems that
the problem is elsewhere.
I'll revert the following two commits for now:
d5ef408 (unpack-objects: prevent writing of inconsistent objects)
28f72a0 (receive-pack: use strict mode for unpacking objects)
as I have verified that running with receive.fsckobjects set to false
fixes the issues for me, and the repository at the receiving end (both
before and after the push) pass git-fsck without problems. Needless to
say, I am not a happy camper right now.
This part of commit d5ef408 changes is bogus:
quoted hunk
@@ -144,9 +205,36 @@ static void added_object(unsigned nr, enum object_type type,
static void write_object(unsigned nr, enum object_type type,
void *buf, unsigned long size)
{
- if (write_sha1_file(buf, size, typename(type), obj_list[nr].sha1) < 0)
- die("failed to write object");
added_object(nr, type, buf, size);
The write_sha1_file() call here was calculating obj_list[nr].sha1; now
it is removed, but added_object() needs this value:
| static void added_object(unsigned nr, enum object_type type,
| void *data, unsigned long size)
| {
| struct delta_info **p = &delta_list;
| struct delta_info *info;
|
| while ((info = *p) != NULL) {
| if (!hashcmp(info->base_sha1, obj_list[nr].sha1) ||
^^^^^^^^^^^^^^^^^
| info->base_offset == obj_list[nr].offset) {
| *p = info->next;
| p = &delta_list;
| resolve_delta(info->nr, type, data, size,
| info->delta, info->size);
| free(info);
| continue;
| }
| p = &info->next;
| }
| }
However, I do not have time to create a proper test case for this.
+ if (!strict) {
+ if (write_sha1_file(buf, size, typename(type), obj_list[nr].sha1) < 0)
+ die("failed to write object");
+ free(buf);
+ obj_list[nr].obj = 0;
+ } else if (type == OBJ_BLOB) {
+ struct blob *blob;
+ if (write_sha1_file(buf, size, typename(type), obj_list[nr].sha1) < 0)
+ die("failed to write object");
+ free(buf);
+
+ blob = lookup_blob(obj_list[nr].sha1);
+ if (blob)
+ blob->object.flags |= FLAG_WRITTEN;
+ else
+ die("invalid blob object");
+ obj_list[nr].obj = 0;
+ } else {
+ struct object *obj;
+ int eaten;
+ hash_sha1_file(buf, size, typename(type), obj_list[nr].sha1);
+ obj = parse_object_buffer(obj_list[nr].sha1, type, size, buf, &eaten);
+ if (!obj)
+ die("invalid %s", typename(type));
+ /* buf is stored via add_object_buffer and in obj, if its a tree or commit */
+ add_object_buffer(obj, buf, size);
+ obj->flags |= FLAG_OPEN;
+ obj_list[nr].obj = obj;
+ }
}
static void resolve_delta(unsigned nr, enum object_type type,
The simplest way to fix this would be to duplicate the added_object()
call in all branches; invoking hash_sha1_file() unconditionally will
work too, but may be wasteful if we need to call write_sha1_file()
afterwards.
On Tue, Mar 04, 2008 at 03:26:35PM +0300, Sergey Vlasov wrote:
On Tue, 04 Mar 2008 03:21:16 -0800 Junio C Hamano wrote:
The simplest way to fix this would be to duplicate the added_object()
call in all branches; invoking hash_sha1_file() unconditionally will
work too, but may be wasteful if we need to call write_sha1_file()
afterwards.
This is only a part of the problem. Moving added_object only makes
forward reference for deltas work.
unpack_delta_entry checks for OBJ_REF_DELTA, if there is a sha1 file.
This must not be true, if --strict is passed. It needs to check the
cache too.
From 843d84fa52ff546bf88f135522e5739070d712aa Mon Sep 17 00:00:00 2001
From: Martin Koegler <redacted>
Date: Tue, 4 Mar 2008 22:38:21 +0100
Subject: [PATCH] unpack-objects: fix delta handling
Signed-off-by: Martin Koegler <redacted>
---
builtin-unpack-objects.c | 9 +++++++--
1 files changed, 7 insertions(+), 2 deletions(-)
diff --git a/builtin-unpack-objects.c b/builtin-unpack-objects.c
index 1845abc..c0d3c9a 100644
--- a/builtin-unpack-objects.c
+++ b/builtin-unpack-objects.c
@@ -206,16 +206,17 @@ static void added_object(unsigned nr, enum object_type type,
static void write_object(unsigned nr, enum object_type type,
void *buf, unsigned long size)
{
- added_object(nr, type, buf, size);
if (!strict) {
if (write_sha1_file(buf, size, typename(type), obj_list[nr].sha1) < 0)
die("failed to write object");
+ added_object(nr, type, buf, size);
free(buf);
obj_list[nr].obj = 0;
} else if (type == OBJ_BLOB) {
struct blob *blob;
if (write_sha1_file(buf, size, typename(type), obj_list[nr].sha1) < 0)
die("failed to write object");
+ added_object(nr, type, buf, size);
free(buf);
blob = lookup_blob(obj_list[nr].sha1);@@ -228,6 +229,7 @@ static void write_object(unsigned nr, enum object_type type,
struct object *obj;
int eaten;
hash_sha1_file(buf, size, typename(type), obj_list[nr].sha1);
+ added_object(nr, type, buf, size);
obj = parse_object_buffer(obj_list[nr].sha1, type, size, buf, &eaten);
if (!obj)
die("invalid %s", typename(type));@@ -301,7 +303,10 @@ static void unpack_delta_entry(enum object_type type, unsigned long delta_size,
free(delta_data);
return;
}
- if (!has_sha1_file(base_sha1)) {
+ obj = lookup_object(base_sha1);
+ if (obj && lookup_object_buffer(obj))
+ ;
+ else if (!has_sha1_file(base_sha1)) {
hashcpy(obj_list[nr].sha1, null_sha1);
add_delta_to_list(nr, base_sha1, 0, delta_data, delta_size);
return;--
1.5.4.GIT