Junio C Hamano <gitster@xxxxxxxxx> writes:
> Thanks, both. Will queue.
>
> I have a couple of questions and comments, though.
>
>> +test_expect_success 'setup' '
>> + test_commit README &&
>> +...
>> + tree=$(git write-tree) &&
>> + head=$(git rev-parse HEAD) &&
>> + rev=$(git rev-parse --verify files-master^0) &&
>> + commit=$(git commit-tree -p $head -p $rev -m "Add subproject master" $tree) &&
>
> I think at this point, your index and working tree states match that
> of $commit. So the next command ...
>
>> + git reset $commit &&
>
> ... made me wonder what its significance was. I think you are doing
> this solely to move the HEAD pointer to point at $commit, but then
> it would be much better and more readable to use update-ref, i.e.
> making this line to:
>
> git update-ref HEAD $commit &&
>
> instead, as "write-tree && commit-tree && update-ref" is a familiar
> pattern to reimplement "git commit" using the plumbing. Ending that
> three-command sequence with "reset" breaks the pattern.
Ok, that makes sense.
>> + (
>> + cd files_subtree &&
>> + test_commit master4
>> + ) &&
>> + test_commit files_subtree/master5
>
> I understand that you are creating these two commits both in the
> top-level repository (the one the history initially created in
> "files" repository gets merged into), but you are creating them
> slightly differently. Is that significant? I am not complaining
> about the style of writing tests, but I am wondering if having these
> two commits created differently has any effect on the bug you
> observed, which may be a good starting point for anybody who wants
> to fix it to start digging from. IOW, would the resulting history
> different if you did this instead?
>
> test_commit files_subtree/master4 &&
> test_commit files_subtree/master5
That is a good question. I originally created the test to see if making
these two commits differently would cause any problems with
git-subtree's split command. Obviously, I didn't get that far. :)
So I think it makes sense for me to at least test and see what happens.
I will add another test to this set if it makes a difference.
> I also notice that files_subtree/master4 does not appear in any of
> the verification in the three tests that use the history being
> prepared here, i.e. if master4 is silently dropped while master5 is
> kept, such a bug won't be caught by them.
Ah, good catch. I should add a test for that.
Let me do a re-roll of this since I think you bring up some excellent
points. Might be a few days due to work obbligations.
-David
--
To unsubscribe from this list: send the line "unsubscribe git" in
the body of a message to majordomo@xxxxxxxxxxxxxxx
More majordomo info at http://vger.kernel.org/majordomo-info.html