On Thu, Aug 19, 2010 at 12:02:36PM +0200, Thomas Rast wrote:
> David Aguilar wrote:
> > When mergetool is run without path limiters it loops
> > over each entry in 'git ls-files -u'. This includes
> > autoresolved paths.
> [...]
> > +test_expect_success 'mergetool merges all from subdir' '
> > + cd subdir && (
> > + git config rerere.enabled false &&
> > + test_must_fail git merge master &&
> > + git mergetool --no-prompt &&
> > + test "$(cat ../file1)" = "master updated" &&
> > + test "$(cat ../file2)" = "master new" &&
> > + test "$(cat file3)" = "master new sub" &&
> > + git add ../file1 ../file2 file3 &&
> > + git commit -m "branch2 resolved by mergetool from subdir") &&
> > + cd ..
> > +'
>
> This test never worked in my automatic testing (it fails and bisects
> to this commit).
>
> It might be because the cronjob doesn't have a tty, as I'm seeing the
> output below (note the error at the end). Any insights?
It must be the tty.
> expecting success:
> cd subdir && (
> git config rerere.enabled false &&
> test_must_fail git merge master &&
> git mergetool --no-prompt &&
> test "$(cat ../file1)" = "master updated" &&
> test "$(cat ../file2)" = "master new" &&
> test "$(cat file3)" = "master new sub" &&
> git add ../file1 ../file2 file3 &&
> git commit -m "branch2 resolved by mergetool from subdir") &&
> cd ..
> [...]
> /local/home/trast/git/t/valgrind/bin/git-mergetool: line 302: /dev/tty: No such device
> or address
> /local/home/trast/git/t/valgrind/bin/git-mergetool: line 299: /dev/tty: No such device
> or address
git-mergetool lines 295-307:
files_to_merge |
while IFS= read i
do
if test $last_status -ne 0; then
prompt_after_failed_merge < /dev/tty || exit 1
fi
printf "\n"
merge_file "$i" < /dev/tty > /dev/tty
last_status=$?
if test $last_status -ne 0; then
rollup_status=1
fi
done
The reason the test fails without a tty is that we've never
exercised this code in the past.
This commit did not introduce the "< /dev/tty > /dev/tty"
idiom. It was introduced in b0169d84 by Charles Bailey.
What this commit did do was add test coverage to it,
which is good because it uncovered this problem :-)
Charles, is there another way we can write this?
Is there a reason why we need the tty redirection?
Can we drop it or is there a portability concern?
FWIW, the merge_file call in the else clause that follows
this section does not use tty redirection.
--
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