threads / patch / 61506

patchSwitch grep from non-portable BRE to portable ERE

Subject: [PATCH] Switch grep from non-portable BRE to portable ERE

## tl;dr

4 messages between May 17, 2024 and May 17, 2024. Diffs are folded; open one to read it.

replies: 3people: 2as markdown or json

Marcel Telka· May 17, 2024, 17:10 UTC · lore

This makes the grep usage fully POSIX compliant.

Signed-off-by: Marcel Telka <marcel@telka.sk>
---
 mergetools/vimdiff           | 2 +-
 t/t1404-update-ref-errors.sh | 2 +-
 2 files changed, 2 insertions(+), 2 deletions(-)

Show changes to 2 files +2 −2

mergetools/vimdiff, t/t1404-update-ref-errors.sh

diff --git a/mergetools/vimdiff b/mergetools/vimdiff
index 734d15a03b..f8ad6b35d4 100644
--- a/mergetools/vimdiff
+++ b/mergetools/vimdiff
@@ -325,7 +325,7 @@ gen_cmd () {
 		fi
 
 		# If this is a single window diff with all the buffers
-		if ! echo "$tab" | grep ",\|/" >/dev/null
+		if ! echo "$tab" | grep -E ",|/" >/dev/null
 		then
 			CMD="$CMD | silent execute 'bufdo diffthis'"
 		fi
diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
index 98e9158bd2..67ebd81a4c 100755
--- a/t/t1404-update-ref-errors.sh
+++ b/t/t1404-update-ref-errors.sh
@@ -100,7 +100,7 @@ df_test() {
 		printf "%s\n" "delete $delname" "create $addname $D"
 	fi >commands &&
 	test_must_fail git update-ref --stdin <commands 2>output.err &&
-	grep "fatal:\( cannot lock ref $SQ$addname$SQ:\)\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&
+	grep -E "fatal:( cannot lock ref $SQ$addname$SQ:)? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&
 	printf "%s\n" "$C $delref" >expected-refs &&
 	git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
 	test_cmp expected-refs actual-refs

Junio C Hamano· May 17, 2024, 18:02 UTC · re: Marcel Telka · lore

Re: [PATCH] Switch grep from non-portable BRE to portable ERE

Marcel Telka <marcel@telka.sk> writes:

> Subject: Re: [PATCH] Switch grep from non-portable BRE to portable ERE
>
> This makes the grep usage fully POSIX compliant.

May want to mention that using backslash to invoke ERE feature in BRE is a GNU extension?

Thanks for this and other portability patches. You are porting to a new platform, I presume, where either nobody else ported to before or those who ported did not bother reporting test breakages to us?

Show 32 quoted lines

> Signed-off-by: Marcel Telka <marcel@telka.sk>
> ---
>  mergetools/vimdiff           | 2 +-
>  t/t1404-update-ref-errors.sh | 2 +-
>  2 files changed, 2 insertions(+), 2 deletions(-)
>
> diff --git a/mergetools/vimdiff b/mergetools/vimdiff
> index 734d15a03b..f8ad6b35d4 100644
> --- a/mergetools/vimdiff
> +++ b/mergetools/vimdiff
> @@ -325,7 +325,7 @@ gen_cmd () {
>  		fi
>  
>  		# If this is a single window diff with all the buffers
> -		if ! echo "$tab" | grep ",\|/" >/dev/null
> +		if ! echo "$tab" | grep -E ",|/" >/dev/null
>  		then
>  			CMD="$CMD | silent execute 'bufdo diffthis'"
>  		fi
> diff --git a/t/t1404-update-ref-errors.sh b/t/t1404-update-ref-errors.sh
> index 98e9158bd2..67ebd81a4c 100755
> --- a/t/t1404-update-ref-errors.sh
> +++ b/t/t1404-update-ref-errors.sh
> @@ -100,7 +100,7 @@ df_test() {
>  		printf "%s\n" "delete $delname" "create $addname $D"
>  	fi >commands &&
>  	test_must_fail git update-ref --stdin <commands 2>output.err &&
> -	grep "fatal:\( cannot lock ref $SQ$addname$SQ:\)\? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&
> +	grep -E "fatal:( cannot lock ref $SQ$addname$SQ:)? $SQ$delref$SQ exists; cannot create $SQ$addref$SQ" output.err &&
>  	printf "%s\n" "$C $delref" >expected-refs &&
>  	git for-each-ref --format="%(objectname) %(refname)" $prefix/r >actual-refs &&
>  	test_cmp expected-refs actual-refs

Marcel Telka· May 17, 2024, 18:50 UTC · re: Junio C Hamano · lore

Re: [PATCH] Switch grep from non-portable BRE to portable ERE

On Fri, May 17, 2024 at 11:02:28AM -0700, Junio C Hamano wrote:

> Thanks for this and other portability patches.  You are porting to a
> new platform, I presume, where either nobody else ported to before
> or those who ported did not bother reporting test breakages to us?

Unfortunately, the latter is the case. I'm doing this for OpenIndiana (an illumos distro, Solaris descendant) and the testing status before I started the work was:

failed test(s): t0211 t0600 t1404 t1700 t2501 t3404 t4150 t4202 t7609 t9001 t9118 t9210 t9902

fixed 1 success 29241 failed 38 broken 269 total 30158

Now all tests pass.

The work uncovered few bugs in illumos as well. For example, this one helped to find issues with grep's null RE cases in git tests: https://www.illumos.org/issues/16561

Thank you.

-- 
+-------------------------------------------+
| Marcel Telka   e-mail:   marcel@telka.sk  |
|                homepage: http://telka.sk/ |
+-------------------------------------------+

Marcel Telka· May 17, 2024, 19:02 UTC · re: Junio C Hamano · lore

Re: [PATCH] Switch grep from non-portable BRE to portable ERE

On Fri, May 17, 2024 at 11:02:28AM -0700, Junio C Hamano wrote:

Show 7 quoted lines

> Marcel Telka <marcel@telka.sk> writes:
> > Subject: Re: [PATCH] Switch grep from non-portable BRE to portable ERE
> >
> > This makes the grep usage fully POSIX compliant.
> 
> May want to mention that using backslash to invoke ERE feature in
> BRE is a GNU extension?

v2 is flying...

-- 
+-------------------------------------------+
| Marcel Telka   e-mail:   marcel@telka.sk  |
|                homepage: http://telka.sk/ |
+-------------------------------------------+