All of lore.kernel.org
 help / color / mirror / Atom feed
From: "Martin Ågren" <martin.agren@gmail.com>
To: git@vger.kernel.org
Cc: "Alexander Pyhalov" <apyhalov@gmail.com>,
	"SZEDER Gábor" <szeder.dev@gmail.com>,
	"Eric Sunshine" <sunshine@sunshineco.com>
Subject: [PATCH] t7005-editor: quote filename to fix whitespace-issue
Date: Wed, 26 Sep 2018 18:14:11 +0200	[thread overview]
Message-ID: <20180926161411.10697-1-martin.agren@gmail.com> (raw)
In-Reply-To: <20180926121107.GH27036@localhost>

From: Alexander Pyhalov <apyhalov@gmail.com>

Commit 4362da078e (t7005-editor: get rid of the SPACES_IN_FILENAMES
prereq, 2018-05-14) removed code for detecting whether spaces in
filenames work. Since we rely on spaces throughout the test suite
("trash directory.t1234-foo"), testing whether we can use the filename
"e space.sh" was redundant and unnecessary.

In simplifying the code, though, this introduced a regression around how
spaces are handled, not in the /name/ of the editor script, but /in/ the
script itself. The script just does `echo space >$1`, where $1 is for
example "/foo/t/trash directory.t7005-editor/.git/COMMIT_EDITMSG".

With most shells, or with Bash in posix mode, $1 will not be subjected
to field splitting. But if we invoke Bash directly, which will happen if
we build Git with SHELL_PATH=/bin/bash, it will detect and complain
about an "ambiguous redirect". More details can be found in [1], thanks
to SZEDER Gábor.

Make sure that the editor script quotes "$1" to remove the ambiguity.

[1] https://public-inbox.org/git/20180926121107.GH27036@localhost/

Signed-off-by: Alexander Pyhalov <apyhalov@gmail.com>
Commit-message-by: Martin Ågren <martin.agren@gmail.com>
Signed-off-by: Martin Ågren <martin.agren@gmail.com>
---
 SZEDER wrote:
 > Let me put on my POSIX-lawyer hat for a moment to explain this :)

 Thanks for an excellent explanation.

 > Sidenote: this test should use the write_script helper to create this
 > editor script.

 Yes. I've punted on that for now.

 Here's my updated commit message as part of a proper patch. Thanks
 Alexander for the analysis and the diff, and thanks Eric and SZEDER for
 getting me on the right track with the commit message.

 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 b2ca77b338..5fcf281dfb 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.216.g2d3b1c576c


  reply	other threads:[~2018-09-26 16:14 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2018-09-26  6:52 t7005-editor.sh failure Alexander Pyhalov
2018-09-26  7:59 ` Martin Ågren
2018-09-26  9:00   ` Alexander Pyhalov
2018-09-26  9:52     ` Martin Ågren
2018-09-26 10:02       ` Alexander Pyhalov
2018-09-26 11:59       ` Eric Sunshine
2018-09-26 13:23         ` Martin Ågren
2018-09-26 12:11       ` SZEDER Gábor
2018-09-26 16:14         ` Martin Ågren [this message]
2018-09-26 18:14           ` [PATCH] t7005-editor: quote filename to fix whitespace-issue Taylor Blau
2018-09-26 19:21           ` Jeff King
2018-09-26 18:16         ` t7005-editor.sh failure Junio C Hamano
2018-09-26 19:16           ` Junio C Hamano
2018-09-26 19:29             ` Andrei Rybak
2018-09-27 20:53             ` SZEDER Gábor

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20180926161411.10697-1-martin.agren@gmail.com \
    --to=martin.agren@gmail.com \
    --cc=apyhalov@gmail.com \
    --cc=git@vger.kernel.org \
    --cc=sunshine@sunshineco.com \
    --cc=szeder.dev@gmail.com \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.