Re: [PATCH v3 3/9] t6111: new TREESAME test set
- From
Junio C Hamano <gitster@pobox.com>
- Date
- May 6, 2013, 20:36 UTC
- Message-ID
- <7v1u9jrgl4.fsf@alter.siamese.dyndns.org>
- In-Reply-To
- <1367767977-14513-4-git-send-email-kevin@bracey.fi>
Kevin Bracey <kevin@bracey.fi> writes:
Show 16 quoted lines
> +# ,---E--. *H----------. * marks !TREESAME parent paths > +# / \ / \* > +# *A--*B---D--*F-*G---------K-*L-*M > +# \ /* \ / > +# `-C-' `-*I-*J > +# > +# A creates "file", B and F change it. > +# Odd merge G takes the old version from B. > +# I changes it, but J reverts it. > +# H and L both change it, and M merges those changes. > + ... > +# Odd merge G drops a change in F. Important that G is listed in all > +# except the most basic list. Achieving this means normal merge D will also be > +# shown in normal full-history, as we can't distinguish unless we do a > +# simplification pass. After simplification, D is dropped but G remains. > +check_result 'M L K J I H G F E D C B A'
OK.
> +check_result 'M H L K J I G E F D C B A' --topo-order
OK.
> +check_result 'M L H B A' -- file
OK.
> +check_result 'M L H B A' --parents -- file
OK.
> +check_outcome failure 'M L J I H G F D B A' --full-history -- file # drops G
This breaks the promise that full history at least gives a connected graph.
> +check_result 'M L K J I H G F D B A' --full-history --parents -- file
And it is saved by --parents; the "(parents)commit" notation may make it easier to read the above two tests.
> +check_outcome failure 'M H L J I G F B A' --simplify-merges -- file # drops G
The hsitory simplify merges gives ought to be a full history that keeps any commit that locally touches the specified path, so keeping I and J even though they do not contribute to the end result is correct. The path in question is the same between G and B, but again, F should be shown because it should be in the full history.
Even though E simplifies down to B, making G a merge between B and F (the former is an ancestor of the latter), G is kept, because...?
Because the path at B and F are different?
I have to wonder what should happen if D makes path different from B, and then F makes path the same as B. B and F should still be kept.
So "because the path at B and F are different" is not the reason why we keep G. How do we decide why G must be kept?
> +check_result 'M L K G F D B A' --first-parent > +check_result 'M L G F B A' --first-parent -- file
OK.
> +# Check that odd merge G remains shown when F is the bottom.
I did not have a chance to say this when you responded in the previous round with that "bottom" thing before you sent this new round, but I am not sure if it is a good idea to pay special attention to the "bottom"-ness. Very often, instead of running "git log -- path" and stopping when seeing enough by killing the pager, people run "git log HEAD~200.. -- path" to pick "recent enough" commits, but the number 200 is very much arbitrary.
> +check_result 'M L K J I H G E' F..M
OK.
> +check_result 'M H L K J I G E' F..M --topo-order
OK. But a note about style: options should come before revision ranges.
> +check_result 'M L H' F..M -- file
OK. It is still one possible history to explain what we have in the path at the end.
> +check_result 'M L H' F..M --parents -- file # L+H's parents rewritten to B, so more useful than it may seem
OK.
> +check_outcome failure 'M L J I H G' F..M --full-history -- file # drops G
G is same as E which is in the graph (F and B are the boundaries of the history, and F is no special than B in that regard). You want to show a merge that is different from at least one parent (as opposed to showing only when it is different from all parents), so you want to show G. OK.
> +check_result 'M L K J I H G' F..M --full-history --parents -- file
OK.
It almost makes me suspect that the --full-history option should flip revs->rewrite_parents on (but not revs->print_parents, unless the --parents option asks us to) when the option is parsed.
> +check_outcome failure 'M H L J I G' F..M --simplify-merges -- file # drops G
The same as the "--full-history without --parents" before.
> +check_result 'M L K J I H G' F..M --ancestry-path > +check_outcome failure 'M L J I H G' F..M --ancestry-path -- file # drops G
With --ancestry-path, what G does relative to E becomes irrelevant, because E is not in the F..M graph. G changes path relative to its remaining parent F, so it should be shown. OK.
> +check_result 'M L K J I H G' F..M --ancestry-path --parents -- file
OK. Again this makes me suspect --ancestry-path should flip revs->rewrite_parents on.
> +check_result 'M H L J I G' F..M --ancestry-path --simplify-merges -- file
Show 6 quoted lines
> +check_result 'M L K G' F..M --first-parent > +check_result 'M L G' F..M --first-parent -- file > + > +# Note that G is pruned when E is the bottom, even if it's the same commit list > +# If we want history since E, then we're quite happy to ignore G that took E. > +check_result 'M L K J I H G' E..M --ancestry-path
OK.
> +check_result 'M L J I H' E..M --ancestry-path -- file
Because F is outside our graph with --ancestry-path, G does not bring anything new to path relative to its remaining parent E and should not be shown. OK.
> +check_outcome failure 'M L K J I H' E..M --ancestry-path --parents -- file
This fails in what way? G is shown, because the !treesame with now irrelevant F comes into play?
> +check_outcome failure 'M H L J I' E..M --ancestry-path --simplify-merges -- file # includes G
Makes me wonder how "--ancestry-path --full-history" would work with this range and with/without the pathspec.
> +# Should still be able to ignore I-J branch in simple log, despite limiting > +# to G. > +check_result 'M L K J I H' G..M > +check_result 'M H L K J I' G..M --topo-order
OK.
> +check_outcome failure 'M L H' G..M -- file # includes J I > +check_outcome failure 'M L H' G..M --parents -- file # includes J I
I am not sure if it should be a failure or your expectation is wrong. G is outside the graph, so as far as the remainder of the graph is concerned, J is the sole remaining parent of K and I and J did change the path in question.
What makes you think I and J should be excluded in these cases?
Shouldn't the same logic as "--ancestry-path [EF]..M" we saw earlier apply here? That is, "-a-p E..M" makes F the sole remaining parent of G and G does change the path from F so it should be shown, while "-a-p F..M" makes E the sole parent of G, and G does not change the path from E, so it should not be shown.
Show 15 quoted lines
> +check_result 'M L J I H' G..M --full-history -- file > +check_result 'M L K J I H' G..M --full-history --parents -- file > +check_result 'M H L J I' G..M --simplify-merges -- file > +check_result 'M L K J I H' G..M --ancestry-path > +check_result 'M L J I H' G..M --ancestry-path -- file > +check_result 'M L K J I H' G..M --ancestry-path --parents -- file > +check_result 'M H L J I' G..M --ancestry-path --simplify-merges -- file > + > +# B..F should be able to simplify the merge D from irrelevant side branch C. > +# Default log should also be free to follow B-D, and ignore C. > +# But --full-history shouldn't drop D on its own - without simplification, > +# we can't decide if the merge from INTERESTING commit C was sensible. > +check_result 'F D C' B..F > +check_result 'F' B..F -- file > +check_outcome failure 'F' B..F --parents -- file # includes D
OK. --parents should not affect the outcome.
> +check_outcome failure 'F D' B..F --full-history -- file # drops D prematurely
OK.
> +check_result 'F D' B..F --full-history --parents -- file
Again, OK.
> +check_result 'F' B..F --simplify-merges -- file > +check_result 'F D' B..F --ancestry-path > +check_result 'F' B..F --ancestry-path -- file
OK.
> +check_outcome failure 'F' B..F --ancestry-path --parents -- file # includes D > +check_outcome failure 'F' B..F --ancestry-path --simplify-merges -- file # includes D
OK. "-a-p B..F" makes C outside our graph, and D does not change the path relative to its sole remaining parent B, so it should not appear in the result.
> +check_result 'F D' B..F --first-parent > +check_result 'F' B..F --first-parent -- file
OK.
> +# Any sort of full history of C..F should show D, as it's the connection to C, > +# and it differs from it. > +check_result 'F D B' C..F > +check_result 'F B' C..F -- file
OK, as A is a boundary and its child B changes the path from it.
> +check_result 'F B' C..F --parents -- file
OK.
> +check_outcome failure 'F D B' C..F --full-history -- file # drops D
OK.
> +check_result 'F D B' C..F --full-history --parents -- file > +check_result 'F D B' C..F --simplify-merges -- file > +check_result 'F D' C..F --ancestry-path > +check_outcome failure 'F D' C..F --ancestry-path -- file # drops D
OK. D should not be dropped merely because it is the same with B, which is now outside our graph "-a-p C..F".
> +check_result 'F D' C..F --ancestry-path --parents -- file > +check_result 'F D' C..F --ancestry-path --simplify-merges -- file
OK.
> +check_result 'F D B' C..F --first-parent > +check_result 'F B' C..F --first-parent -- file
OK. "-f-p C..F" is the same as "-f-p A..F", and C is outside our graph, so these expectations look sensible.