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 <jrnieder@gmail.com> Signed-off-by: Dmitry Ivankov <divanorama@gmail.com> Signed-off-by: Junio C Hamano <gitster@pobox.com>

Dmitry Ivankov committed Aug 15, 2011 at 00:32 UTC 8fb3ad76b1782a5439343b9601f73a9287a245cf
2 files changed +31 -6
fast-import.c
+30 -5
@@ -170,6 +170,11 @@ Format of STDIN stream:
170 #define DEPTH_BITS 13
171 #define MAX_DEPTH ((1<<DEPTH_BITS)-1)
172
173 +/*
174 + * We abuse the setuid bit on directories to mean "do not delta".
175 + */
176 +#define NO_DELTA S_ISUID
177 +
178 struct object_entry {
179 struct pack_idx_entry idx;
180 struct object_entry *next;
@@ -1414,8 +1419,9 @@ static void mktree(struct tree_content *t, int v, struct strbuf *b)
1419 struct tree_entry *e = t->entries[i];
1420 if (!e->versions[v].mode)
1421 continue;
1417 - strbuf_addf(b, "%o %s%c", (unsigned int)e->versions[v].mode,
1418 - e->name->str_dat, '\0');
1422 + strbuf_addf(b, "%o %s%c",
1423 + (unsigned int)(e->versions[v].mode & ~NO_DELTA),
1424 + e->name->str_dat, '\0');
1425 strbuf_add(b, e->versions[v].sha1, 20);
1426 }
1427 }
@@ -1425,7 +1431,7 @@ static void store_tree(struct tree_entry *root)
1431 struct tree_content *t = root->tree;
1432 unsigned int i, j, del;
1433 struct last_object lo = { STRBUF_INIT, 0, 0, /* no_swap */ 1 };
1428 - struct object_entry *le;
1434 + struct object_entry *le = NULL;
1435
1436 if (!is_null_sha1(root->versions[1].sha1))
1437 return;
@@ -1435,7 +1441,8 @@ static void store_tree(struct tree_entry *root)
1441 store_tree(t->entries[i]);
1442 }
1443
1438 - le = find_object(root->versions[0].sha1);
1444 + if (!(root->versions[0].mode & NO_DELTA))
1445 + le = find_object(root->versions[0].sha1);
1446 if (S_ISDIR(root->versions[0].mode) && le && le->pack_id == pack_id) {
1447 mktree(t, 0, &old_tree);
1448 lo.data = old_tree;
@@ -1469,6 +1476,7 @@ static void tree_content_replace(
1476 {
1477 if (!S_ISDIR(mode))
1478 die("Root cannot be a non-directory");
1479 + hashclr(root->versions[0].sha1);
1480 hashcpy(root->versions[1].sha1, sha1);
1481 if (root->tree)
1482 release_tree_content_recursive(root->tree);
@@ -1513,6 +1521,23 @@ static int tree_content_set(
1521 if (e->tree)
1522 release_tree_content_recursive(e->tree);
1523 e->tree = subtree;
1524 +
1525 + /*
1526 + * We need to leave e->versions[0].sha1 alone
1527 + * to avoid modifying the preimage tree used
1528 + * when writing out the parent directory.
1529 + * But after replacing the subdir with a
1530 + * completely different one, it's not a good
1531 + * delta base any more, and besides, we've
1532 + * thrown away the tree entries needed to
1533 + * make a delta against it.
1534 + *
1535 + * So let's just explicitly disable deltas
1536 + * for the subtree.
1537 + */
1538 + if (S_ISDIR(e->versions[0].mode))
1539 + e->versions[0].mode |= NO_DELTA;
1540 +
1541 hashclr(root->versions[1].sha1);
1542 return 1;
1543 }
@@ -2927,7 +2952,7 @@ static void print_ls(int mode, const unsigned char *sha1, const char *path)
2952 /* mode SP type SP object_name TAB path LF */
2953 strbuf_reset(&line);
2954 strbuf_addf(&line, "%06o %s %s\t",
2930 - mode, type, sha1_to_hex(sha1));
2955 + mode & ~NO_DELTA, type, sha1_to_hex(sha1));
2956 quote_c_style(path, &line, NULL, 0);
2957 strbuf_addch(&line, '\n');
2958 }
t/t9300-fast-import.sh
+1 -1
@@ -765,7 +765,7 @@ g/b/f
765 g/b/h
766 EOF
767
768 -test_expect_failure \
768 +test_expect_success \
769 'L: nested tree copy does not corrupt deltas' \
770 'git fast-import <input &&
771 git ls-tree L2 g/b/ >tmp &&