git/list[1] front-page[2] threads[3] people[4] search[5] about
 

[PATCH] t7005-editor: quote filename to fix whitespace-issue

From
MÅMartin Ågren <martin.agren@gmail.com>
Date
Sep 26, 2018, 16:14 UTC
Message-ID
<20180926161411.10697-1-martin.agren@gmail.com>
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
Previous: SZEDER GáborNext: Taylor Blau
Message 9 of 15 in “t7005-editor.sh failure”
  1. Alexander PyhalovSep 26, 2018
  2. Martin ÅgrenSep 26, 2018
  3. Alexander PyhalovSep 26, 2018
  4. Martin ÅgrenSep 26, 2018
  5. Alexander PyhalovSep 26, 2018
  6. Eric SunshineSep 26, 2018
  7. Martin ÅgrenSep 26, 2018
  8. SZEDER GáborSep 26, 2018
  9. t7005-editor: quote filename to fix whitespace-issueMartin Ågren, Sep 26, 2018
  10. Taylor BlauSep 26, 2018
  11. Jeff KingSep 26, 2018
  12. Junio C HamanoSep 26, 2018
  13. Junio C HamanoSep 26, 2018
  14. Andrei RybakSep 26, 2018
  15. SZEDER GáborSep 27, 2018

Read the whole thread, see it on lore, or plain text.

$ cat FOOTERMessages come from the public archive at lore.kernel.org/git, fetched every hour. The front page is chosen and written each morning by an AI editor and can be wrong; the threads themselves are the record. About and API. For agents: an MCP server at https://gitlist.dev/mcp, and any thread, story or person page as Markdown by adding .md to its URL (or sending Accept: text/markdown). Details in /llms.txt.