From 9a0edb79f2f00c79b9dced22f67d226f6bb9c741 Mon Sep 17 00:00:00 2001 From: Dmitry Ivankov Date: Mon, 15 Aug 2011 00:32:23 +0600 Subject: [PATCH 1/2] fast-import: add a test for tree delta base corruption fast-import is able to write imported tree objects in delta format. It holds a tree structure in memory where each tree entry may have a delta base sha1 assigned. When delta base data is needed it is reconstructed from this in-memory structure. Though sometimes the delta base data doesn't match the delta base sha1 so wrong or even corrupt pack is produced. Add a small test that produces a corrupt pack. It uses just tree copy and file modification commands aside from the very basic commit and blob commands. Signed-off-by: Dmitry Ivankov Signed-off-by: Junio C Hamano --- t/t9300-fast-import.sh | 41 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 41 insertions(+) diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 2a53640c5b..c88ab467f8 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -734,6 +734,47 @@ test_expect_success \ git diff-tree --abbrev --raw L^ L >output && test_cmp expect output' +cat >input < 1112912473 -0700 +data < 1112912473 -0700 +data <expect +g/b/f +g/b/h +EOF + +test_expect_failure \ + 'L: nested tree copy does not corrupt deltas' \ + 'git fast-import tmp && + cat tmp | cut -f 2 >actual && + test_cmp expect actual && + git fsck `git rev-parse L2`' + +git update-ref -d refs/heads/L2 + ### ### series M ### From 8fb3ad76b1782a5439343b9601f73a9287a245cf Mon Sep 17 00:00:00 2001 From: Dmitry Ivankov Date: Mon, 15 Aug 2011 00:32:24 +0600 Subject: [PATCH 2/2] fast-import: prevent producing bad delta To produce deltas for tree objects fast-import tracks two versions of tree's entries - base and current one. Base version stands both for a delta base of this tree, and for a entry inside a delta base of a parent tree. So care should be taken to keep it in sync. tree_content_set cuts away a whole subtree and replaces it with a new one (or NULL for lazy load of a tree with known sha1). It keeps a base sha1 for this subtree (needed for parent tree). And here is the problem, 'subtree' tree root doesn't have the implied base version entries. Adjusting the subtree to include them would mean a deep rewrite of subtree. Invalidating the subtree base version would mean recursive invalidation of parents' base versions. So just mark this tree as do-not-delta me. Abuse setuid bit for this purpose. tree_content_replace is the same as tree_content_set except that is is used to replace the root, so just clearing base sha1 here (instead of setting the bit) is fine. [di: log message] Signed-off-by: Jonathan Nieder Signed-off-by: Dmitry Ivankov Signed-off-by: Junio C Hamano --- fast-import.c | 35 ++++++++++++++++++++++++++++++----- t/t9300-fast-import.sh | 2 +- 2 files changed, 31 insertions(+), 6 deletions(-) diff --git a/fast-import.c b/fast-import.c index 78d978684d..6dad9ff4db 100644 --- a/fast-import.c +++ b/fast-import.c @@ -170,6 +170,11 @@ Format of STDIN stream: #define DEPTH_BITS 13 #define MAX_DEPTH ((1<entries[i]; if (!e->versions[v].mode) continue; - strbuf_addf(b, "%o %s%c", (unsigned int)e->versions[v].mode, - e->name->str_dat, '\0'); + strbuf_addf(b, "%o %s%c", + (unsigned int)(e->versions[v].mode & ~NO_DELTA), + e->name->str_dat, '\0'); strbuf_add(b, e->versions[v].sha1, 20); } } @@ -1425,7 +1431,7 @@ static void store_tree(struct tree_entry *root) struct tree_content *t = root->tree; unsigned int i, j, del; struct last_object lo = { STRBUF_INIT, 0, 0, /* no_swap */ 1 }; - struct object_entry *le; + struct object_entry *le = NULL; if (!is_null_sha1(root->versions[1].sha1)) return; @@ -1435,7 +1441,8 @@ static void store_tree(struct tree_entry *root) store_tree(t->entries[i]); } - le = find_object(root->versions[0].sha1); + if (!(root->versions[0].mode & NO_DELTA)) + le = find_object(root->versions[0].sha1); if (S_ISDIR(root->versions[0].mode) && le && le->pack_id == pack_id) { mktree(t, 0, &old_tree); lo.data = old_tree; @@ -1469,6 +1476,7 @@ static void tree_content_replace( { if (!S_ISDIR(mode)) die("Root cannot be a non-directory"); + hashclr(root->versions[0].sha1); hashcpy(root->versions[1].sha1, sha1); if (root->tree) release_tree_content_recursive(root->tree); @@ -1513,6 +1521,23 @@ static int tree_content_set( if (e->tree) release_tree_content_recursive(e->tree); e->tree = subtree; + + /* + * We need to leave e->versions[0].sha1 alone + * to avoid modifying the preimage tree used + * when writing out the parent directory. + * But after replacing the subdir with a + * completely different one, it's not a good + * delta base any more, and besides, we've + * thrown away the tree entries needed to + * make a delta against it. + * + * So let's just explicitly disable deltas + * for the subtree. + */ + if (S_ISDIR(e->versions[0].mode)) + e->versions[0].mode |= NO_DELTA; + hashclr(root->versions[1].sha1); return 1; } @@ -2927,7 +2952,7 @@ static void print_ls(int mode, const unsigned char *sha1, const char *path) /* mode SP type SP object_name TAB path LF */ strbuf_reset(&line); strbuf_addf(&line, "%06o %s %s\t", - mode, type, sha1_to_hex(sha1)); + mode & ~NO_DELTA, type, sha1_to_hex(sha1)); quote_c_style(path, &line, NULL, 0); strbuf_addch(&line, '\n'); } diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index c88ab467f8..b33bc8246d 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -765,7 +765,7 @@ g/b/f g/b/h EOF -test_expect_failure \ +test_expect_success \ 'L: nested tree copy does not corrupt deltas' \ 'git fast-import tmp &&