Re: [PATCH v3 3/9] t6111: new TREESAME test set
Kevin Bracey ke...@bracey.fi writes: On 06/05/2013 23:36, Junio C Hamano wrote: Kevin Bracey ke...@bracey.fi 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. + ... ... +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? Because it's the simplest answer to the question what happened in M since G, which is what G..M is supposed to mean. ... This all comes about because the formal graph definition doesn't match the user interface. The question B..C currently generates a graph of all commits in C since B, and the connections between those commits. It turns out to be problematic that the graph doesn't include the connection to B itself. It would be fine if only worrying about nodes in the graph. But it's not fine when you start doing graph operations that care about edge connections to parents. OK, that makes sense. ... What I'm effectively doing is extending the graph to actually include the unshown bottom. I think it just makes more sense. Yup, and this is a good summary. ... I assume you mean: That is, -a-p F..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 E..M makes E the sole parent of G, and G does not change the path from E, so it should not be shown. Yes. Which is the way the logic works - we treat F and E as interesting/priority parents when they're specified as a bottom in each case. Without doing that, G would have 2 differing and equally (un)important parents in each case, and thus would be shown in both cases. In this case, the same logic says that G is treated as an interesting parent of K because it is the specified bottom. Which then enables the default following to follow that path direct to G, rather than having to go down the IJ path (which leads to G anyway). OK. -- To unsubscribe from this list: send the line unsubscribe git in the body of a message to majord...@vger.kernel.org More majordomo info at http://vger.kernel.org/majordomo-info.html
Re: [PATCH v3 3/9] t6111: new TREESAME test set
Kevin Bracey ke...@bracey.fi writes:
Some side branching and odd merging to illustrate various flaws in
revision list scans, particularly when limiting the list.
Many expected failures, which will be gone by the end of the history
traversal refinements series.
Signed-off-by: Kevin Bracey ke...@bracey.fi
---
t/t6111-rev-list-treesame.sh | 180
+++
1 file changed, 180 insertions(+)
create mode 100755 t/t6111-rev-list-treesame.sh
diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh
new file mode 100755
index 000..602c02d
--- /dev/null
+++ b/t/t6111-rev-list-treesame.sh
@@ -0,0 +1,180 @@
+#!/bin/sh
+#
+#,---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.
... and because J is a revert of I, K ends up merging the same and
being the same with both G and J.
+test_description='TREESAME and limiting'
+
+. ./test-lib.sh
+
+note () {
+ git tag $1
+}
+
+unnote () {
+ git name-rev --tags --stdin | sed -e s|$_x40 (tags/\([^)]*\)) |\1 |g
+}
+
+test_expect_success setup '
+ test_commit Initial file file Hi there A
+ git branch other-branch
+
+ test_commit file=Hello file Hello B
+ git branch third-branch
+
+ git checkout other-branch
+ test_commit Added other other Hello C
+
+ git checkout master
+ test_merge D other-branch
+
+ git checkout third-branch
+ test_commit Third file third Nothing E
+
+ git checkout master
+ test_commit file=Blah file Blah F
+
+ test_tick git merge --no-commit third-branch
+ git checkout third-branch file
+ git commit
+ note G
+ git branch fiddler-branch
+
+ git checkout -b part2-branch
+ test_commit file=Part 2 file Part 2 H
+
+ git checkout fiddler-branch
+ test_commit Bad commit file Silly I
+
+ test_tick git revert I note J
+
+ git checkout master
+ test_tick git merge --no-ff fiddler-branch
+ note K
+
+ test_commit file=Part 1 file Part 1 L
+
+ test_tick test_must_fail git merge part2-branch
+ test_commit M file Parts 1+2
+'
+
+FMT='tformat:%P %H | %s'
+
+# could we soup this up to optionally check parents? So (BA)C would check
+# that C is shown and has parents B A.
Sounds like a good thing to do, especially given that some of the
earlier issues you brought up triggered only when --parents is not
used, so...
Perhaps something like the attached.
+check_outcome () {
+ outcome=$1
+ shift
+ for c in $1
+ do
+ echo $c
+ done expect
Maybe
printf %s\n $1 expect
is more fashionable these days?
+# 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'
I'll read the expectation and comment on them in a separate message.
t/t6111-rev-list-treesame.sh | 30 +-
1 file changed, 21 insertions(+), 9 deletions(-)
diff --git a/t/t6111-rev-list-treesame.sh b/t/t6111-rev-list-treesame.sh
index 602c02d..f4c9cac 100755
--- a/t/t6111-rev-list-treesame.sh
+++ b/t/t6111-rev-list-treesame.sh
@@ -20,7 +20,7 @@ note () {
}
unnote () {
- git name-rev --tags --stdin | sed -e s|$_x40 (tags/\([^)]*\)) |\1 |g
+ git name-rev --tags --stdin | sed -e s|$_x40 (tags/\([^)]*\))\([
]\)|\1\2|g
}
test_expect_success setup '
@@ -66,23 +66,34 @@ test_expect_success setup '
test_commit M file Parts 1+2
'
-FMT='tformat:%P%H | %s'
-
# could we soup this up to optionally check parents? So (BA)C would check
# that C is shown and has parents B A.
check_outcome () {
outcome=$1
shift
- for c in $1
- do
- echo $c
- done expect
- shift
+
+ case $1 in
+ *(*)
+ FMT=%P %H | %s
+ munge_actual=
+ s/^\([^ ]*\)\([^ ]*\) .*/(\1)\2/
+ s/ //g
+ s/()//
+
+ ;;
+ *)
+ FMT=%H | %s
+ munge_actual=s/^\([^ ]*\) .*/\1/
+ ;;
+ esac
+ printf %s\n $1 expect
+ shift
+
param=$*
test_expect_$outcome log $param '
git log --format=$FMT $param |
unnote actual
- sed -e s/^.* \([^ ]*\) .*/\1/
Re: [PATCH v3 3/9] t6111: new TREESAME test set
Kevin Bracey ke...@bracey.fi 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.
