On Wed, Mar 20, 2013 at 2:48 AM, Johannes Sixt <j.sixt@xxxxxxxxxxxxx> wrote:
> Am 2/21/2013 5:03, schrieb David Aguilar:
>> test_expect_success PERL 'difftool -d' '
>> - diff=$(git difftool -d --extcmd ls branch) &&
>> - echo "$diff" | stdin_contains sub &&
>> - echo "$diff" | stdin_contains file
>> + git difftool -d --extcmd ls branch >output &&
>> + stdin_contains sub <output &&
>> + stdin_contains file <output
>> '
>
> This test is broken on Windows. There is this code in git-difftool.perl
>
> for my $file (@worktree) {
> ...
> copy("$b/$file", "$workdir/$file") or
> exit_cleanup($tmpdir, 1);
> ...
> }
>
> @worktree is populated with all files in the worktree. At this point,
> "output" is among them. Then follows an attempt to copy a file over
> "$workdir/$file". I guess that is some link+remove magic going on behind
> the scenes. At any rate, this fails on Windows with
> "D:/Src/mingw-git/t/trash directory.t7800-difftool/../../git-difftool line
> 408: Bad file number", because files that are open cannot be written from
> outside (the file is open due to the redirection in the test snippet).
>
> What is going on here? Why can this ever succeed even on Unix?
Thanks for the report. Yes, these do pass on Unix.
Hmm I wonder what's going on here?
I started digging in and the @worktree_files (aka @worktree above)
is populated from the output of "git diff --raw ...".
Seeing the "output" filename in "diff --raw" implies that one of the
tests added "output" to the index somehow. I do not see that
happening anywhere, though, so I do not know how it would end up in
the @worktree array if it is not reported by "diff --raw".
My current understanding of how it could possibly be open twice:
1. via the >output redirect
2. via the copy() perl code which is fed by @worktree
So I'm confused. Why would we get different results on Windows?
I just re-ran these tests from "next" to check my sanity and they
passed on both Linux and OS X.
> Same for some later tests.
Ditto.
> BTW, while debugging this, I found the use of the helper function
> stdin_contains() highly unhelpful; it just resolves to a 'grep' that on
> top of all hides stdout. Please don't do that. Just use unadorned grep
> like we do everywhere else.
I'm not too opposed to that.
The one small advantage to the helper is that you can tweak the redirect
in one central place, so it's not all for naught.
Sitaram, you added this back in:
ba959de1 git-difftool: allow skipping file by typing 'n' at prompt
Do you have any thoughts?
It seems like removing the stdout redirect could be helpful for debugging,
and if we did that then there's really no point in having the helper
(aside from the indirection which can sometimes help during debugging).
I don't really feel too strongly either way, but it did bother you
while debugging the test script, so unadorned grep seems like the way to go.
I'll wait and see if anybody else has any Windows-specific clues that
we can use to narrow down this problem.
In lieu of an immediate fix, are there any test prerequisite we can
use to skip these tests on windows? One or both of NOT_CYGWIN,NOT_MINGW?
--
David
--
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