Merge branch 'jh/fast-import-notes' into maint

* jh/fast-import-notes:
  fast-import: Fix incorrect fanout level when modifying existing notes refs
  t9301: Add 2nd testcase exposing bugs in fast-import's notes fanout handling
  t9301: Fix testcase covering up a bug in fast-import's notes fanout handling

diff --git a/fast-import.c b/fast-import.c
index 8d8ea3c45c0be5481c7b452c27ee0d163d69fb00..f4bfe0f6650fdb2666f031ca1f551134f872e098 100644 (file)
--- a/fast-import.c
+++ b/fast-import.c
@@ -2173,6 +2173,11 @@ static uintmax_t do_change_note_fanout(
 
                if (tmp_hex_sha1_len == 40 && !get_sha1_hex(hex_sha1, sha1)) {
                        /* This is a note entry */
+                       if (fanout == 0xff) {
+                               /* Counting mode, no rename */
+                               num_notes++;
+                               continue;
+                       }
                        construct_path_with_fanout(hex_sha1, fanout, realpath);
                        if (!strcmp(fullpath, realpath)) {
                                /* Note entry is in correct location */
@@ -2379,7 +2384,7 @@ static void file_change_cr(struct branch *b, int rename)
                leaf.tree);
 }
 
-static void note_change_n(struct branch *b, unsigned char old_fanout)
+static void note_change_n(struct branch *b, unsigned char *old_fanout)
 {
        const char *p = command_buf.buf + 2;
        static struct strbuf uq = STRBUF_INIT;
@@ -2390,6 +2395,23 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)
        uint16_t inline_data = 0;
        unsigned char new_fanout;
 
+       /*
+        * When loading a branch, we don't traverse its tree to count the real
+        * number of notes (too expensive to do this for all non-note refs).
+        * This means that recently loaded notes refs might incorrectly have
+        * b->num_notes == 0, and consequently, old_fanout might be wrong.
+        *
+        * Fix this by traversing the tree and counting the number of notes
+        * when b->num_notes == 0. If the notes tree is truly empty, the
+        * calculation should not take long.
+        */
+       if (b->num_notes == 0 && *old_fanout == 0) {
+               /* Invoke change_note_fanout() in "counting mode". */
+               b->num_notes = change_note_fanout(&b->branch_tree, 0xff);
+               *old_fanout = convert_num_notes_to_fanout(b->num_notes);
+       }
+
+       /* Now parse the notemodify command. */
        /* <dataref> or 'inline' */
        if (*p == ':') {
                char *x;
@@ -2450,7 +2472,7 @@ static void note_change_n(struct branch *b, unsigned char old_fanout)
                            typename(type), command_buf.buf);
        }
 
-       construct_path_with_fanout(sha1_to_hex(commit_sha1), old_fanout, path);
+       construct_path_with_fanout(sha1_to_hex(commit_sha1), *old_fanout, path);
        if (tree_content_remove(&b->branch_tree, path, NULL))
                b->num_notes--;
 
@@ -2637,7 +2659,7 @@ static void parse_new_commit(void)
                else if (!prefixcmp(command_buf.buf, "C "))
                        file_change_cr(b, 0);
                else if (!prefixcmp(command_buf.buf, "N "))
-                       note_change_n(b, prev_fanout);
+                       note_change_n(b, &prev_fanout);
                else if (!strcmp("deleteall", command_buf.buf))
                        file_change_deleteall(b);
                else if (!prefixcmp(command_buf.buf, "ls "))

diff --git a/t/t9301-fast-import-notes.sh b/t/t9301-fast-import-notes.sh
index 463254c72734beaf74948b6c424367ef4fea9d1a..83acf68bc3c19770eb690ece139892ca75329216 100755 (executable)
--- a/t/t9301-fast-import-notes.sh
+++ b/t/t9301-fast-import-notes.sh
@@ -505,9 +505,63 @@ test_expect_success 'verify that non-notes are untouched by a fanout change' '
        test_cmp expect_non-note3 actual
 
 '
+
+# Change the notes for the three top commits
+test_tick
+cat >input <<INPUT_END
+commit refs/notes/many_notes
+committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
+data <<COMMIT
+changing notes for the top three commits
+COMMIT
+from refs/notes/many_notes^0
+INPUT_END
+
+rm expect
+i=$num_commits
+j=0
+while test $j -lt 3
+do
+       cat >>input <<INPUT_END
+N inline refs/heads/many_commits~$j
+data <<EOF
+changed note for commit #$i
+EOF
+INPUT_END
+       cat >>expect <<EXPECT_END
+    commit #$i
+    changed note for commit #$i
+EXPECT_END
+       i=$(($i - 1))
+       j=$(($j + 1))
+done
+
+test_expect_success 'change a few existing notes' '
+
+       git fast-import <input &&
+       GIT_NOTES_REF=refs/notes/many_notes git log -n3 refs/heads/many_commits |
+           grep "^    " > actual &&
+       test_cmp expect actual
+
+'
+
+test_expect_success 'verify that changing notes respect existing fanout' '
+
+       # None of the entries in the top-level notes tree should be a full SHA1
+       git ls-tree --name-only refs/notes/many_notes |
+       while read path
+       do
+               if test $(expr length "$path") -ge 40
+               then
+                       return 1
+               fi
+       done
+
+'
+
 remaining_notes=10
 test_tick
-cat >>input <<INPUT_END
+cat >input <<INPUT_END
 commit refs/notes/many_notes
 committer $GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE
 data <<COMMIT
@@ -516,12 +570,11 @@ COMMIT
 from refs/notes/many_notes^0
 INPUT_END
 
-i=$remaining_notes
-while test $i -lt $num_commits
+i=$(($num_commits - $remaining_notes))
+for sha1 in $(git rev-list -n $i refs/heads/many_commits)
 do
-       i=$(($i + 1))
        cat >>input <<INPUT_END
-N 0000000000000000000000000000000000000000 :$i
+N 0000000000000000000000000000000000000000 $sha1
 INPUT_END
 done