Re: t7005-editor.sh failure

[Alexander Pyhalov <apyhalov@gmail.com>]

[-- Attachment #1: Type: text/plain, Size: 2973 bytes --]

Hi.
As for sign-off, do I understand correctly that you just want to know
that I'm the original author of the code? Yes, it's so.

I see this on OpenIndiana in
https://github.com/OpenIndiana/oi-userland/pull/4456 , when running
test suite.
Not sure why it wasn't noticed earlier, as 'trash directory' is used in path.

When I use truss to look at failure, I see

2653:   execve("/export/home/alp/srcs/oi-userland/components/developer/git/build/amd64/git",
0x00564EC8, 0x00566008)  argc = 3
2653:    argv:
2653:     /export/home/alp/srcs/oi-userland/components/developer/git/build/amd64/git
2653:     commit --amend
hint: Waiting for your editor to close the file... 2655:
execve("/bin/bash", 0x007EA898, 0x007EA960)  argc = 5
2655:    argv: /bin/bash -c ./e\ space.sh "$@" ./e\ space.sh
2655:     /export/home/alp/srcs/oi-userland/components/developer/git/build/amd64/t/trash
directory.t7005-editor/.git/COMMIT_EDITMSG
2655:   execve("./e space.sh", 0x005655C8, 0x00564008)  Err#8 ENOEXEC
./e space.sh: line 1: $1: ambiguous redirect
2653:       Received signal #18, SIGCLD, in waitid() [default]
2653:         siginfo: SIGCLD CLD_EXITED pid=2655 status=0x0001
error: There was a problem with the editor './e\ space.sh'.
Please supply the message using either -m or -F option.

Shell is bash, as you can see (GNU bash, version 4.4.23(1)-release
(i386-pc-solaris2.11))


2018-09-26 10:59 GMT+03:00 Martin Ågren <martin.agren@gmail.com>:
> Hi Alexander,
>
> Welcome to the list!
>
> On Wed, 26 Sep 2018 at 08:54, Alexander Pyhalov <apyhalov@gmail.com> wrote:
>> On updating git to 2.19 we've suddenly got t7005-editor.sh test failures.
>> The issue seems to be that generated "e space.sh" file can't handle
>> files with spaces.
>> Instead of 'echo space >$1' it should be 'echo space > "$1"' or git
>> editor fails when gets file with spaces in name.
>
> Thanks for finding, analysing and reporting. I haven't bisected, but I'm
> guessing this comes from 4362da078e (t7005-editor: get rid of the
> SPACES_IN_FILENAMES prereq, 2018-05-14), which only happens to have to
> do with spaces in filenames. But in rewriting the test, it introduced
> /another/ instance of spaces-matter-here and didn't quote $1 properly.
> Cute. :-)
>
> We try to snuggle the filename to the >redirector, so it would be 'echo
> space >"$1"' and similar.
>
> Could we have your sign-off for this? Please see [1] for what that
> means. If you want to re-submit as a "proper" patch with commit message
> and all, great. If not, I could do it for you, with you as "Author:", if
> you just let me know.
>
> By the way, could you say something about which shell or which
> environment this bug triggered in? Just so we can better understand how
> this snuck past us.
>
> [1] https://github.com/git/git/blob/master/Documentation/SubmittingPatches
>
> Thanks
> Martin



-- 
С уважением,
Александр Пыхалов

[-- Attachment #2: 0001-t7005-editor.sh-e-space.sh-should-properly-escape-fi.patch --]
[-- Type: text/x-patch, Size: 809 bytes --]

From 171e2404a5e19b1b9b5112ad408a5cc1cf963289 Mon Sep 17 00:00:00 2001
From: Alexander Pyhalov <apyhalov@gmail.com>
Date: Wed, 26 Sep 2018 11:54:58 +0300
Subject: [PATCH] t7005-editor.sh: "e space.sh" should properly escape file
 path

Signed-off-by: Alexander Pyhalov <apyhalov@gmail.com>
---
 t/t7005-editor.sh | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/t/t7005-editor.sh b/t/t7005-editor.sh
index b2ca77b33..66243c43c 100755
--- a/t/t7005-editor.sh
+++ b/t/t7005-editor.sh
@@ -112,7 +112,7 @@ do
 done
 
 test_expect_success 'editor with a space' '
-	echo "echo space >\$1" >"e space.sh" &&
+	echo "echo space > \"\$1\"" > "e space.sh" &&
 	chmod a+x "e space.sh" &&
 	GIT_EDITOR="./e\ space.sh" git commit --amend &&
 	test space = "$(git show -s --pretty=format:%s)"
-- 
2.19.0