Re: [PATCH v3 3/9] t6111: new TREESAME test set

[Date Prev][Date Next][Thread Prev][Thread Next][Date Index][Thread Index]

 



Kevin Bracey <kevin@xxxxxxxxx> writes:

> +#        ,---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

> +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.

> +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.

--
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




[Index of Archives]     [Linux Kernel Development]     [Gcc Help]     [IETF Annouce]     [DCCP]     [Netdev]     [Networking]     [Security]     [V4L]     [Bugtraq]     [Yosemite]     [MIPS Linux]     [ARM Linux]     [Linux Security]     [Linux RAID]     [Linux SCSI]     [Fedora Users]