TODO: tests!

do_change_note_fanout() recursively traverses a "struct tree_entry" data
structure. Before recursively traversing into a subtree, we make sure to
call load_tree() on that subtree. However, for the initial/root struct
tree_entry object, we assumed that load_tree() had already been invoked by
our caller. This is true, except in the following case:

If we load a previously written notes tree using 'filemodify' instead of
'from' [1], then we do not pass the notes ref to load_branch() (which
takes care of calling load_tree() on the root tree_entry).

When load_tree(root) has not been called before the first 'notemodify'
command is parsed, the for loop in do_change_note_fanout() does nothing,
and we end up not counting the existing notes, which ends up producing
a notes tree with mixed/insufficient fanout.

Fix this by always making sure to call load_tree() at the start of
do_change_note_fanout(). Since load_tree() is now called before the for loop,
we no longer need to call load_tree() on a subtree before recursing into it,
the recursive call will take care of its own load_tree().

[1]: Usually, when adding notes to an existing notes tree, one would use a
     'from' command like this:

         from refs/notes/foo^0

     However, if one does not want to retain notes history (i.e. one wants
     the current refs/notes/foo commit to be build without any parents),
     then one can replace the 'from' command with the following 'filemodify'
     "hack" to pre-load the existing notes tree without using it as a parent:

         M 040000 refs/notes/foo^{tree} \n

Discovered-by: Mike Hommey <m...@glandium.org>
Signed-off-by: Johan Herland <jo...@herland.net>
---
 fast-import.c | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/fast-import.c b/fast-import.c
index 04dfd50..aa7b64e 100644
--- a/fast-import.c
+++ b/fast-import.c
@@ -2159,6 +2159,9 @@ static uintmax_t do_change_note_fanout(
        unsigned char sha1[20];
        char realpath[60];
 
+       if (!root->tree)
+               load_tree(root);
+
        for (i = 0; root->tree && i < root->tree->entry_count; i++) {
                e = root->tree->entries[i];
                tmp_hex_sha1_len = hex_sha1_len + e->name->str_len;
@@ -2210,8 +2213,6 @@ static uintmax_t do_change_note_fanout(
                                leaf.tree);
                } else if (S_ISDIR(e->versions[1].mode)) {
                        /* This is a subdir that may contain note entries */
-                       if (!e->tree)
-                               load_tree(e);
                        num_notes += do_change_note_fanout(orig_root, e,
                                hex_sha1, tmp_hex_sha1_len,
                                fullpath, tmp_fullpath_len, fanout);
-- 
2.1.1.392.g062cc5d

--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to