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

[Junio C Hamano <gitster@pobox.com>]

SZEDER Gábor <szeder.dev@gmail.com> writes:

> On Wed, Jan 06, 2021 at 10:24:27PM -0800, Junio C Hamano wrote:
>> SZEDER Gábor <szeder.dev@gmail.com> writes:
>> 
>> > 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.