[PATCH v2 1/7] t1414: document some reflog-walk oddities

From: Jeff King <peff@peff.net>
Date: Jul 7, 2017, 09:06 UTC
Message-ID: <20170707090610.x2vodxgc7yr6o5pi@sigill.intra.peff.net>
In-Reply-To: <20170707090507.ko2ygry7j4zv7t3s@sigill.intra.peff.net>

Since its inception, the general strategy of the reflog-walk code has been to start with the tip commit for the ref, and as we traverse replace each commit's parent pointers with fake parents pointing to the previous reflog entry.

This lets us traverse the reflog as if it were a real history, but it has some user-visible oddities. Namely:

  1. The fake parents are used for commit selection and
     display. So for example, "--merges" or "--no-merges"
     are not useful, because the history appears as a linear
     string of commits. Likewise, pathspec limiting is based
     on the diff between adjacent entries, not the changes
     actually introduced by a commit.

     These are often the same (e.g., because the entry was
     just running "git commit" and the adjacent entry _is_
     the true parent), but it may not be in several common
     cases. For instance, using "git reset" to jump around
     history, or "git checkout" to move HEAD.

  2. We reverse-map each commit back to its reflog. So when
     it comes time to show commit X, we say "a-ha, we added
     X because it was at the tip of the 'foo' reflog, so
     let's show the foo reflog". But this leads to nonsense
     results when you ask to traverse multiple reflogs: if
     two reflogs have the same tip commit, we only map back
     to one of them.  Instead, we should show both.

  3. If the tip of the reflog and the ref tip disagree on
     the current value, we show the ref tip but give no
     indication of the value in the reflog.  This situation
     isn't supposed to happen (since any ref update should
     touch the reflog). But if it does, given that the
     requested operation is to show the reflog, it makes
     sense to prefer that.

This commit adds a new script with several expect_failure tests to demonstrate the problems. This could be part of the existing t1411, but it's a bit easier to start from a fresh state, where we know exactly what will be in the log.

Since the new multiple-reflog tests are checking the actual output, we can drop the "make sure we don't segfault" tests from t1411, which are a strict subset of what we're doing here.

Signed-off-by: Jeff King <peff@peff.net>
---
 t/t1411-reflog-show.sh |  10 -----
 t/t1414-reflog-walk.sh | 105 +++++++++++++++++++++++++++++++++++++++++++++++++
 2 files changed, 105 insertions(+), 10 deletions(-)
 create mode 100755 t/t1414-reflog-walk.sh

diff --git a/t/t1411-reflog-show.sh b/t/t1411-reflog-show.sh
index b9cb76654b..6ac7734d79 100755
--- a/t/t1411-reflog-show.sh
+++ b/t/t1411-reflog-show.sh
@@ -171,14 +171,4 @@ test_expect_success 'reflog exists works' '
 	! git reflog exists refs/heads/nonexistent
 '
 
-# The behavior with two reflogs is buggy and the output is in flux; for now
-# we're just checking that the program works at all without segfaulting.
-test_expect_success 'showing multiple reflogs works' '
-	git log -g HEAD HEAD >actual
-'
-
-test_expect_success 'showing multiple reflogs with an old date' '
-	git log -g HEAD@{1979-01-01} HEAD >actual
-'
-
 test_done
diff --git a/t/t1414-reflog-walk.sh b/t/t1414-reflog-walk.sh
new file mode 100755
index 0000000000..6426829352
--- /dev/null
+++ b/t/t1414-reflog-walk.sh
@@ -0,0 +1,105 @@
+#!/bin/sh
+
+test_description='various tests of reflog walk (log -g) behavior'
+. ./test-lib.sh
+
+test_expect_success 'set up some reflog entries' '
+	test_commit one &&
+	test_commit two &&
+	git checkout -b side HEAD^ &&
+	test_commit three &&
+	git merge --no-commit master &&
+	echo evil-merge-content >>one.t &&
+	test_tick &&
+	git commit --no-edit -a
+'
+
+do_walk () {
+	git log -g --format="%gd %gs" "$@"
+}
+
+sq="'"
+test_expect_success 'set up expected reflog' '
+	cat >expect.all <<-EOF
+	HEAD@{0} commit (merge): Merge branch ${sq}master${sq} into side
+	HEAD@{1} commit: three
+	HEAD@{2} checkout: moving from master to side
+	HEAD@{3} commit: two
+	HEAD@{4} commit (initial): one
+	EOF
+'
+
+test_expect_success 'reflog walk shows expected logs' '
+	do_walk >actual &&
+	test_cmp expect.all actual
+'
+
+test_expect_failure 'reflog can limit with --no-merges' '
+	grep -v merge expect.all >expect &&
+	do_walk --no-merges >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'reflog can limit with pathspecs' '
+	grep two expect.all >expect &&
+	do_walk -- two.t >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'pathspec limiting handles merges' '
+	# we pick up:
+	#   - the initial commit of one
+	#   - the checkout back to commit one
+	#   - the evil merge which touched one
+	sed -n "1p;3p;5p" expect.all >expect &&
+	do_walk -- one.t >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure '--parents shows true parents' '
+	# convert newlines to spaces
+	echo $(git rev-parse HEAD HEAD^1 HEAD^2) >expect &&
+	git rev-list -g --parents -1 HEAD >actual &&
+	test_cmp expect actual
+'
+
+test_expect_failure 'walking multiple reflogs shows all' '
+	# We expect to see all entries for all reflogs, but interleaved by
+	# date, with order no the command line breaking ties. We
+	# can use "sort" on the separate lists to generate this,
+	# but note two tricks:
+	#
+	#   1. We use "{" as the delimiter, which lets us skip to the reflog
+	#      date specifier as our second field, and then our "-n" numeric
+	#      sort ignores the bits after the timestamp.
+	#
+	#   2. POSIX leaves undefined whether this is a stable sort or not. So
+	#      we use "-k 1" to ensure that we see HEAD before master before
+	#      side when breaking ties.
+	{
+		do_walk --date=unix HEAD &&
+		do_walk --date=unix side &&
+		do_walk --date=unix master
+	} >expect.raw &&
+	sort -t "{" -k 2nr -k 1 <expect.raw >expect &&
+	do_walk --date=unix HEAD master side >actual &&
+	test_cmp expect actual
+'
+
+test_expect_success 'date-limiting does not interfere with other logs' '
+	do_walk HEAD@{1979-01-01} HEAD >actual &&
+	test_cmp expect.all actual
+'
+
+test_expect_failure 'walk prefers reflog to ref tip' '
+	head=$(git rev-parse HEAD) &&
+	one=$(git rev-parse one) &&
+	ident="$GIT_COMMITTER_NAME <$GIT_COMMITTER_EMAIL> $GIT_COMMITTER_DATE" &&
+	echo "$head $one $ident	broken reflog entry" >>.git/logs/HEAD &&
+
+	echo $one >expect &&
+	git log -g --format=%H -1 >actual &&
+	test_cmp expect actual
+'
+
+test_done

-- 
2.13.2.1000.g8590c1af5d