On Mon, Oct 30, 2017 at 11:57 PM, Junio C Hamano <gitster@xxxxxxxxx> wrote:
> Stefan Beller <sbeller@xxxxxxxxxx> writes:
>
>> 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.)
>> +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.
