Re: [PATCH 2/7] revision.h: introduce blob/tree walking in order of the commits

On Mon, Oct 30, 2017 at 11:57 PM, Junio C Hamano [off-list ref] wrote:

Stefan Beller [off-list ref] writes:

quoted

diff --git a/list-objects.c b/list-objects.c
index bf46f80dff..5e114c9a8a 100644
--- a/list-objects.c
+++ b/list-objects.c

@@ -237,6 +237,8 @@ void traverse_commit_list(struct rev_info *revs,
              if (commit->tree)
                      add_pending_tree(revs, commit->tree);
              show_commit(commit, data);
+             if (revs->tree_blobs_in_commit_order)
+                     traverse_trees_and_blobs(revs, &base_path, show_object, data);
      }
      traverse_trees_and_blobs(revs, &base_path, show_object, data);

This one is interesting.  While we walk the ancestry chain, we may
add the tree for each commit that we discover to the "pending.  We
used to sweep the pending array at the end after we run out of the
commits, but now we do the sweeping as we process each commit.

Would this make the last call to traverse_trees_and_blobs() always a
no-op?

That is my understanding of the code, the important part of the previous patch
was the move of 'object_array_clear(&revs->pending);' into the
`traverse_trees_and_blobs` function.

 I am not suggesting to add a new conditional in front of it;
I am just asking for curiosity's sake.

Thanks for being clear on that. I have the same taste for this.

(Though for a split second I wondered if we can pull some esoteric trick
to skip this, but I could not find any that is sane as well as readable.)

quoted

+test_expect_success 'setup' '
+     for x in one two three four
+     do
+             echo $x >$x &&
+             git add $x &&
+             git commit -m "add file $x"
+     done &&
+     for x in four three
+     do
+             git rm $x
+             git commit -m "remove $x"
+     done &&

There is break in &&-chain in the second loop, but in any case, for
loops and &&-chains do not mix very well unless done carefully.
Imagine what happens if "git commit" in the first loop failed when
x is two; it won't stop and keep going to create three and four and
nobody would noice any error.

I'll fix that.

Even though we do not have --stdin for rev-parse, you can still do:

        git cat-file --batch-check='%(objectname)' >expect <<-\EOF &&
        HEAD^{commit}
        HEAD^{tree}
        HEAD:one
        HEAD:two
        ...
        EOF

sounds better than the current implementation.