From 5edde510181357b0d0376d5542ddfa51a7e7ba12 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 17 Oct 2010 20:03:38 -0500 Subject: fast-import: filemodify after M 040000 "" crashes Until M 040000 "" syntax was introduced in commit 2794ad5 (fast-import: Allow filemodify to set the root, 2010-10-10), it was impossible for the root entry to refer to an unloaded tree. Update various functions to take that possibility into account. Otherwise M 040000 "" M 100644 :1 "foo" and similar commands (using D, C, or R after resetting the root tree) segfault. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- fast-import.c | 20 ++++++++++++++++---- t/t9300-fast-import.sh | 37 +++++++++++++++++++++++++++++++++++++ 2 files changed, 53 insertions(+), 4 deletions(-) diff --git a/fast-import.c b/fast-import.c index 8f68a89234..aaf47c5745 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1444,7 +1444,7 @@ static int tree_content_set( const uint16_t mode, struct tree_content *subtree) { - struct tree_content *t = root->tree; + struct tree_content *t; const char *slash1; unsigned int i, n; struct tree_entry *e; @@ -1468,6 +1468,9 @@ static int tree_content_set( if (!slash1 && !S_ISDIR(mode) && subtree) die("Non-directories cannot have subtrees"); + if (!root->tree) + load_tree(root); + t = root->tree; for (i = 0; i < t->entry_count; i++) { e = t->entries[i]; if (e->name->str_len == n && !strncmp(p, e->name->str_dat, n)) { @@ -1523,7 +1526,7 @@ static int tree_content_remove( const char *p, struct tree_entry *backup_leaf) { - struct tree_content *t = root->tree; + struct tree_content *t; const char *slash1; unsigned int i, n; struct tree_entry *e; @@ -1534,6 +1537,9 @@ static int tree_content_remove( else n = strlen(p); + if (!root->tree) + load_tree(root); + t = root->tree; for (i = 0; i < t->entry_count; i++) { e = t->entries[i]; if (e->name->str_len == n && !strncmp(p, e->name->str_dat, n)) { @@ -1581,7 +1587,7 @@ static int tree_content_get( const char *p, struct tree_entry *leaf) { - struct tree_content *t = root->tree; + struct tree_content *t; const char *slash1; unsigned int i, n; struct tree_entry *e; @@ -1592,6 +1598,9 @@ static int tree_content_get( else n = strlen(p); + if (!root->tree) + load_tree(root); + t = root->tree; for (i = 0; i < t->entry_count; i++) { e = t->entries[i]; if (e->name->str_len == n && !strncmp(p, e->name->str_dat, n)) { @@ -2056,13 +2065,16 @@ static uintmax_t do_change_note_fanout( char *fullpath, unsigned int fullpath_len, unsigned char fanout) { - struct tree_content *t = root->tree; + struct tree_content *t; struct tree_entry *e, leaf; unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len; uintmax_t num_notes = 0; unsigned char sha1[20]; char realpath[60]; + if (!root->tree); + load_tree(root); + t = root->tree; for (i = 0; t && i < t->entry_count; i++) { e = t->entries[i]; tmp_hex_sha1_len = hex_sha1_len + e->name->str_len; diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 3c0cf0509d..1df11adc90 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -928,6 +928,43 @@ test_expect_success \ git diff-tree -C --find-copies-harder -r N5^^ N5 >actual && compare_diff_raw expect actual' +test_expect_success \ + 'N: copy to root by id and modify' \ + 'echo "hello, world" >expect.foo && + echo hello >expect.bar && + git fast-import <<-SETUP_END && + commit refs/heads/N7 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data < $GIT_COMMITTER_DATE + data <actual.foo && + git show N8:foo/bar >actual.bar && + test_cmp expect.foo actual.foo && + test_cmp expect.bar actual.bar' + ### ### series O ### -- cgit v1.2.3 From 3421578393a5983db6fa76a203c9dee95c74801d Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Sun, 17 Oct 2010 20:08:53 -0500 Subject: fast-import: tighten M 040000 syntax When tree_content_set() is asked to modify the path "foo/bar/", it first recurses like so: tree_content_set(root, "foo/bar/", sha1, S_IFDIR) -> tree_content_set(root:foo, "bar/", ...) -> tree_content_set(root:foo/bar, "", ...) And as a side-effect of 2794ad5 (fast-import: Allow filemodify to set the root, 2010-10-10), this last call is accepted and changes the tree entry for root:foo/bar to refer to the specified tree. That seems safe enough but let's reject the new syntax (we never meant to support it) and make it harder for frontends to introduce pointless incompatibilities with git fast-import 1.7.3. Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- fast-import.c | 34 +++++++++++++++++++++++++--------- t/t9300-fast-import.sh | 30 ++++++++++++++++++++++++++++++ 2 files changed, 55 insertions(+), 9 deletions(-) diff --git a/fast-import.c b/fast-import.c index aaf47c5745..cb947c10d7 100644 --- a/fast-import.c +++ b/fast-import.c @@ -1437,6 +1437,20 @@ static void store_tree(struct tree_entry *root) t->entry_count -= del; } +static void tree_content_replace( + struct tree_entry *root, + const unsigned char *sha1, + const uint16_t mode, + struct tree_content *newtree) +{ + if (!S_ISDIR(mode)) + die("Root cannot be a non-directory"); + hashcpy(root->versions[1].sha1, sha1); + if (root->tree) + release_tree_content_recursive(root->tree); + root->tree = newtree; +} + static int tree_content_set( struct tree_entry *root, const char *p, @@ -1454,15 +1468,6 @@ static int tree_content_set( n = slash1 - p; else n = strlen(p); - if (!slash1 && !n) { - if (!S_ISDIR(mode)) - die("Root cannot be a non-directory"); - hashcpy(root->versions[1].sha1, sha1); - if (root->tree) - release_tree_content_recursive(root->tree); - root->tree = subtree; - return 1; - } if (!n) die("Empty path component found in input"); if (!slash1 && !S_ISDIR(mode) && subtree) @@ -2230,6 +2235,10 @@ static void file_change_m(struct branch *b) command_buf.buf); } + if (!*p) { + tree_content_replace(&b->branch_tree, sha1, mode, NULL); + return; + } tree_content_set(&b->branch_tree, p, sha1, mode, NULL); } @@ -2288,6 +2297,13 @@ static void file_change_cr(struct branch *b, int rename) tree_content_get(&b->branch_tree, s, &leaf); if (!leaf.versions[1].mode) die("Path %s not in branch", s); + if (!*d) { /* C "path/to/subdir" "" */ + tree_content_replace(&b->branch_tree, + leaf.versions[1].sha1, + leaf.versions[1].mode, + leaf.tree); + return; + } tree_content_set(&b->branch_tree, d, leaf.versions[1].sha1, leaf.versions[1].mode, diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index 1df11adc90..ce094577e0 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -928,6 +928,20 @@ test_expect_success \ git diff-tree -C --find-copies-harder -r N5^^ N5 >actual && compare_diff_raw expect actual' +test_expect_success \ + 'N: reject foo/ syntax' \ + 'subdir=$(git rev-parse refs/heads/branch^0:file2) && + test_must_fail git fast-import <<-INPUT_END + commit refs/heads/N5B + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data <expect.foo && @@ -965,6 +979,22 @@ test_expect_success \ test_cmp expect.foo actual.foo && test_cmp expect.bar actual.bar' +test_expect_success \ + 'N: extract subtree' \ + 'branch=$(git rev-parse --verify refs/heads/branch^{tree}) && + cat >input <<-INPUT_END && + commit refs/heads/N9 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data < Date: Sun, 17 Oct 2010 20:44:04 -0500 Subject: t9300 (fast-import): another test for the "replace root" feature Another test for the replace root feature. One can imagine an implementation for which R "some/subdir" "" would free some state associated to the subdir and leave fast-import confused. Luckily, git's is not such an implementation. While at it, change the previous test to use C "some/subdir" "" instead of R (i.e., test both syntaxes). Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- t/t9300-fast-import.sh | 43 ++++++++++++++++++++++++++++++++++++++++++- 1 file changed, 42 insertions(+), 1 deletion(-) diff --git a/t/t9300-fast-import.sh b/t/t9300-fast-import.sh index ce094577e0..dd90a0933e 100755 --- a/t/t9300-fast-import.sh +++ b/t/t9300-fast-import.sh @@ -990,11 +990,52 @@ test_expect_success \ COMMIT M 040000 $branch "" - R "newdir" "" + C "newdir" "" INPUT_END git fast-import expect.baz && + echo hello, world >expect.qux && + git fast-import <<-SETUP_END && + commit refs/heads/N10 + committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE + data < $GIT_COMMITTER_DATE + data <actual.baz && + git show N11:bar/qux >actual.qux && + git show N11:bar/quux >actual.quux && + test_cmp expect.baz actual.baz && + test_cmp expect.qux actual.qux && + test_cmp expect.qux actual.quux' + ### ### series O ### -- cgit v1.2.3 From b21241253bf1216bd31d24d27446895ea17b1a08 Mon Sep 17 00:00:00 2001 From: Jonathan Nieder Date: Wed, 20 Oct 2010 15:25:58 -0500 Subject: fast-import: do not clear notes in do_change_note_fanout() Commit 5edde51 (fast-import: filemodify after M 040000 "" crashes, 2010-10-17) taught fast-import to load trees from the object db as needed when it is time to access them. But it went too far. In change_note_fanout(), an empty, not-loaded tree is not meant to destroy notes, so calling load_tree() at that point is exactly the wrong thing to do. Kudos to Johan Herland for t9301, which caught this failure. Reported-by: Thomas Rast Signed-off-by: Jonathan Nieder Signed-off-by: Junio C Hamano --- fast-import.c | 5 +---- 1 file changed, 1 insertion(+), 4 deletions(-) diff --git a/fast-import.c b/fast-import.c index cb947c10d7..d881630b01 100644 --- a/fast-import.c +++ b/fast-import.c @@ -2070,16 +2070,13 @@ static uintmax_t do_change_note_fanout( char *fullpath, unsigned int fullpath_len, unsigned char fanout) { - struct tree_content *t; + struct tree_content *t = root->tree; struct tree_entry *e, leaf; unsigned int i, tmp_hex_sha1_len, tmp_fullpath_len; uintmax_t num_notes = 0; unsigned char sha1[20]; char realpath[60]; - if (!root->tree); - load_tree(root); - t = root->tree; for (i = 0; t && i < t->entry_count; i++) { e = t->entries[i]; tmp_hex_sha1_len = hex_sha1_len + e->name->str_len; -- cgit v1.2.3