Re: [PATCH] t7800-difftool: don't accidentally match tmp dirs

SZEDER Gábor [off-list ref] writes:

On Wed, Jan 06, 2021 at 10:24:27PM -0800, Junio C Hamano wrote:

quoted

SZEDER Gábor [off-list ref] writes:

quoted

diff --git a/t/t7800-difftool.sh b/t/t7800-difftool.sh
index a578b35761..fe02fe1688 100755
--- a/t/t7800-difftool.sh
+++ b/t/t7800-difftool.sh

@@ -439,73 +439,104 @@ run_dir_diff_test () {
 }
 
 run_dir_diff_test 'difftool -d' '
+	cat >expect <<-\EOF &&
+	file
+	file2
+
+	file
+	file2
+	sub
+	EOF
 	git difftool -d $symlinks --extcmd ls branch >output &&
-	grep sub output &&
-	grep file output
+	grep -v ^/ output >actual &&

This unfortunately would not catch full paths on certain platforms.

See https://github.com/git/git/runs/1660588243?check_suite_focus=true#step:7:4186
for an example X-<.

Hrm, one has to log in to view those CI logs?  Really?! :(

Yup, it sucks.  I am curious (but not strongly interested enough to
demand) to learn the reason why from GitHub folks.

Anyway, I (apparently falsely) assumed that the output these tests
look at come from Git itself, and therefore we can rely on difftool's
temporary directories being normalized UNIX-style absolute paths...
But it seems they don't actually come from Git but from 'ls', because
that's what those '--extcmd ls' options do, and I now going to assume
that 'ls' prints those absolute paths with drive letter prefixes and
whatnot on Windows.

The initial version of this patch just tightened all potentially
problematic 'grep' patterns, e.g. 'grep ^sub$ output && grep ^file$
output'.  That should work on Windows as well, shouldn't it.  Will see
whether I can dig it out from the reflogs.

I only see 'file', 'file2', 'b', 'c', etc. used in the tests; do we
ever use any path that ends with a colon?

I wonder if would it be more robust to use something like

	sed -e 's|^.*/\([a-z]*\)/:$|/directory-path-\1/:|'

in place of "grep -v /" to redact the temporary directory names
difftool creates.  It would give you /directory-path-left/: or
/directory-path-right/: instead of removing these lines and it would
clarify what these two series of file names are, which may be an
added bonus.