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

Re: [PATCH] grep: follow conventions for printing paths w/ unusual chars

From
Johannes Schindelin <johannes.schindelin@gmx.de>
Date
Apr 18, 2020, 14:56 UTC
Message-ID
<nycvar.QRO.7.76.6.2004181655300.46@tvgsbejvaqbjf.bet>
In-Reply-To
<nycvar.QRO.7.76.6.2004181509350.46@tvgsbejvaqbjf.bet>
Hi Matheus,
On Sat, 18 Apr 2020, Johannes Schindelin wrote:
Show 31 quoted lines
> On Fri, 17 Apr 2020, Matheus Tavares wrote:
>
> > grep does not follow the conventions used by other Git commands when
> > printing paths that contain unusual characters (as double-quotes or
> > newlines). Commands such as ls-files, commit, status and diff will:
> >
> > - Quote and escape unusual pathnames, by default.
> > - Print names verbatim and unquoted when "-z" is used.
> >
> > But grep *never* quotes/escapes absolute paths with unusual chars and
> > *always* quotes/escapes relative ones, even with "-z". Besides being
> > inconsistent in its own output, the deviation from other Git commands
> > can be confusing. So let's make it follow the two rules above and add
> > some tests for this new behavior. Note that, making grep quote/escape
> > all unusual paths by default, also make it fully compliant with the
> > core.quotePath configuration, which is currently ignored for absolute
> > paths.
> >
> > Reported-by: Greg Hurrell <greg@hurrell.net>
> > Signed-off-by: Matheus Tavares <matheus.bernardino@usp.br>
> > ---
> >  Documentation/git-grep.txt |  6 +++--
> >  builtin/grep.c             | 46 ++++++++++++++++++++++++++++----------
> >  t/t7810-grep.sh            | 44 ++++++++++++++++++++++++++++++++++++
> >  3 files changed, 82 insertions(+), 14 deletions(-)
>
> Unfortunately, this causes eight test failures on Windows:
> https://dev.azure.com/gitgitgadget/git/_build/results?buildId=38023&view=ms.vss-test-web.build-test-results-tab
>
> Could you please have a look? I suspect that this has something to do with
> those new tests needing the `FUNNYNAMES` prereq.

I need this commit to fix it: https://github.com/git-for-windows/git/commit/7ca815e1ab89d6ffdb1a17b3cbacdf22a508d33c

I'll paste it here, for your convenience:
-- snipsnap --
From 7ca815e1ab89d6ffdb1a17b3cbacdf22a508d33c Mon Sep 17 00:00:00 2001
From: Johannes Schindelin <johannes.schindelin@gmx.de>
Date: Sat, 18 Apr 2020 16:49:43 +0200
Subject: [PATCH] fixup??? grep: follow conventions for printing paths w/
 unusual chars

It is easy to be fooled by the Bash included in Git for Windows, which leads you to believe that quotes are valid parts of file names.

On Windows, they are not. But Cygwin (which is the base of MSYS2, which is the POSIX emulation layer used by that Bash) only _pretends_ that it is a valid file name character. In reality, it will map the character into the private Unicode page. Cygwin knows about this. The rest of Windows applications (including Git for Windows), however, does not.

As a consequence, `>\"with\ quotes\"` will claim to succeed, but the file on disk will have Unicode characters in place of those quotes that literally no application but Cygwin ones can handle, and this leads to those beautiful new tests to fail.

Let's just use the prereq we introduced to guard precisely against this problem: `FUNNYNAMES`.

Signed-off-by: Johannes Schindelin <johannes.schindelin@gmx.de>
---
 t/t7810-grep.sh | 15 +++++++++------
 1 file changed, 9 insertions(+), 6 deletions(-)
diff --git a/t/t7810-grep.sh b/t/t7810-grep.sh
index ab495dba28a7..991d5bd9c03f 100755
--- a/t/t7810-grep.sh
+++ b/t/t7810-grep.sh
@@ -72,8 +72,11 @@ test_expect_success setup '
 	# Still a no-op.
 	function dummy() {}
 	EOF
-	echo unusual >"\"unusual\" pathname" &&
-	echo unusual >"t/nested \"unusual\" pathname" &&
+	if test_have_prereq FUNNYNAMES
+	then
+		echo unusual >"\"unusual\" pathname" &&
+		echo unusual >"t/nested \"unusual\" pathname"
+	fi &&
 	git add . &&
 	test_tick &&
 	git commit -m initial
@@ -484,7 +487,7 @@ do
 		test_cmp expected actual
 	'

-	test_expect_success "grep $L should quote unusual pathnames" '
+	test_expect_success FUNNYNAMES "grep $L should quote unusual pathnames" '
 		cat >expected <<-EOF &&
 		${HC}"\"unusual\" pathname":unusual
 		${HC}"t/nested \"unusual\" pathname":unusual
@@ -493,7 +496,7 @@ do
 		test_cmp expected actual
 	'

-	test_expect_success "grep $L in subdir should quote unusual relative pathnames" '
+	test_expect_success FUNNYNAMES "grep $L in subdir should quote unusual relative pathnames" '
 		cat >expected <<-EOF &&
 		${HC}"nested \"unusual\" pathname":unusual
 		EOF
@@ -504,7 +507,7 @@ do
 		test_cmp expected actual
 	'

-	test_expect_success "grep -z $L with unusual pathnames" '
+	test_expect_success FUNNYNAMES "grep -z $L with unusual pathnames" '
 		cat >expected <<-EOF &&
 		${HC}"unusual" pathname:unusual
 		${HC}t/nested "unusual" pathname:unusual
@@ -514,7 +517,7 @@ do
 		test_cmp expected actual-replace-null
 	'

-	test_expect_success "grep -z $L in subdir with unusual relative pathnames" '
+	test_expect_success FUNNYNAMES "grep -z $L in subdir with unusual relative pathnames" '
 		cat >expected <<-EOF &&
 		${HC}nested "unusual" pathname:unusual
 		EOF
--
2.26.1.windows.1
Previous: Johannes SchindelinNext: Matheus Tavares Bernardino
Message 11 of 13 in “git-grep's "-z" option misbehaves in subdirectory”
  1. Greg HurrellApr 13, 2020
  2. Junio C HamanoApr 13, 2020
  3. Matheus Tavares, Apr 14, 2020
  4. Matheus Tavares BernardinoApr 16, 2020
  5. Junio C HamanoApr 16, 2020
  6. grep: follow conventions for printing paths w/ unusual charsMatheus Tavares, Apr 17, 2020
  7. Junio C HamanoApr 17, 2020
  8. Matheus Tavares BernardinoApr 17, 2020
  9. Junio C HamanoApr 17, 2020
  10. Johannes SchindelinApr 18, 2020
  11. Johannes SchindelinApr 18, 2020
  12. Matheus Tavares BernardinoApr 19, 2020
  13. grep: follow conventions for printing paths w/ unusual charsMatheus Tavares, Apr 19, 2020

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.