git@vger.kernel.org mailing list mirror (one of many)
 help / color / mirror / code / Atom feed
* [PATCH v2 0/4] allow "grep -E", remove {e,f}grep usage
@ 2022-09-21 13:02 Đoàn Trần Công Danh
  2022-09-21 13:02 ` [PATCH v2 1/4] CodingGuidelines: allow grep -E Đoàn Trần Công Danh
                   ` (3 more replies)
  0 siblings, 4 replies; 6+ messages in thread
From: Đoàn Trần Công Danh @ 2022-09-21 13:02 UTC (permalink / raw)
  To: git
  Cc: Đoàn Trần Công Danh, SZEDER Gábor,
	Phillip Wood, Junio C Hamano

Our CodingGuidelines says that we should avoid "grep -E" and/or 
"grep \{m,n\}". However they're still in use and noone has
complained, yet.

In addition, GNU grep 3.8 started to warn about the continuation
of deprecation process for egrep and fgrep.

This series aim to allow "grep -E" and replace {e,f}grep usage with
"grep -{E,F}"

While there're idea to lift the restriction for \{m,n\}, too.
Their usage are limited and could be replaced with other alternatives.
Let's skip them for now.

Change from v1:
- Change wording in 2/4
- Change regex in 2/4 to be more readable
- Remove '-F' from some regex in 4/4 when the regex doesn't have any special
  characters

Đoàn Trần Công Danh (4):
  CodingGuidelines: allow grep -E
  t: remove \{m,n\} from BRE grep usage
  t: convert egrep usage to "grep -E"
  t: convert fgrep usage to "grep -F"

 Documentation/CodingGuidelines       |  2 --
 t/perf/run                           |  4 ++--
 t/t1304-default-acl.sh               |  4 ++--
 t/t3200-branch.sh                    |  4 ++--
 t/t3305-notes-fanout.sh              |  2 +-
 t/t3404-rebase-interactive.sh        |  6 +++---
 t/t3700-add.sh                       |  2 +-
 t/t3702-add-edit.sh                  |  2 +-
 t/t4014-format-patch.sh              |  8 ++++----
 t/t5320-delta-islands.sh             |  2 +-
 t/t5550-http-fetch-dumb.sh           |  2 +-
 t/t5702-protocol-v2.sh               |  2 +-
 t/t7003-filter-branch.sh             |  4 ++--
 t/t7527-builtin-fsmonitor.sh         | 18 +++++++++---------
 t/t7701-repack-unpack-unreachable.sh |  4 ++--
 t/t9001-send-email.sh                |  8 ++++----
 t/t9133-git-svn-nested-git-repo.sh   |  6 +++---
 t/t9134-git-svn-ignore-paths.sh      |  8 ++++----
 t/t9140-git-svn-reset.sh             |  4 ++--
 t/t9147-git-svn-include-paths.sh     |  8 ++++----
 t/t9814-git-p4-rename.sh             |  2 +-
 t/t9815-git-p4-submit-fail.sh        |  4 ++--
 t/test-lib-functions.sh              |  2 +-
 23 files changed, 53 insertions(+), 55 deletions(-)

Range-diff against v1:
1:  a8dadaf2d1 = 1:  4ad1ac9d9b CodingGuidelines: allow grep -E
2:  9d5fcda278 ! 2:  ebaf6cec07 t: remove \{m,n\} from BRE grep usage
    @@ Metadata
      ## Commit message ##
         t: remove \{m,n\} from BRE grep usage
     
    -    \{m,n\} is a GNU extension to BRE, and it's forbidden by our
    -    CodingGuidelines.
    +    The CodingGuidelines says we should avoid \{m,n\} in BRE usage.
    +    And their usages in our code base is limited, and subjectively
    +    hard to read.
     
    -    Change to fixed strings or ERE.
    +    Replace them with ERE.
    +
    +    Except for "0\{40\}" which would be changed to "$ZERO_OID",
    +    which is a better value for testing with:
    +    GIT_TEST_DEFAULT_HASH=sha256
     
         Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
     
    @@ t/t3200-branch.sh: test_expect_success 'git branch -M baz bam should succeed whe
      	msg="Branch: renamed refs/heads/baz to refs/heads/bam" &&
     -	grep " 0\{40\}.*$msg$" .git/logs/HEAD &&
     -	grep "^0\{40\}.*$msg$" .git/logs/HEAD
    -+	zero="00000000" &&
    -+	zero="$zero$zero$zero$zero$zero" &&
    -+	grep " $zero.*$msg$" .git/logs/HEAD &&
    -+	grep "^$zero.*$msg$" .git/logs/HEAD
    ++	grep " $ZERO_OID.*$msg$" .git/logs/HEAD &&
    ++	grep "^$ZERO_OID.*$msg$" .git/logs/HEAD
      '
      
      test_expect_success 'git branch -M should leave orphaned HEAD alone' '
    @@ t/t3305-notes-fanout.sh: path_has_fanout() {
      	fanout=$2 &&
      	after_last_slash=$(($(test_oid hexsz) - $fanout * 2)) &&
     -	echo $path | grep -q "^\([0-9a-f]\{2\}/\)\{$fanout\}[0-9a-f]\{$after_last_slash\}$"
    -+	echo $path | grep -q -E "^([0-9a-f][0-9a-f]/){$fanout}[0-9a-f]{$after_last_slash}$"
    ++	echo $path | grep -q -E "^([0-9a-f]{2}/){$fanout}[0-9a-f]{$after_last_slash}$"
      }
      
      touched_one_note_with_fanout() {
3:  a131160033 = 3:  b7c0629603 t: convert egrep usage to "grep -E"
4:  50d009b368 ! 4:  b65a3d7749 t: convert fgrep usage to "grep -F"
    @@ t/t7003-filter-branch.sh: test_expect_success 'result is really identical' '
      	(git config core.bare true && cd .git &&
      	 git filter-branch branch > filter-output 2>&1 &&
     -	! fgrep fatal filter-output)
    -+	! grep -F fatal filter-output)
    ++	! grep fatal filter-output)
      '
      git config core.bare false
      test_expect_success 'result is really identical' '
    @@ t/t7003-filter-branch.sh: test_expect_success 'rewrite repository including refs
      	git reset --hard HEAD &&
      	git filter-branch -f -- --all >filter-output 2>&1 &&
     -	! fgrep fatal filter-output
    -+	! grep -F fatal filter-output
    ++	! grep fatal filter-output
      '
      
      test_expect_success 'filter-branch handles ref deletion' '
    @@ t/t9134-git-svn-ignore-paths.sh: test_expect_success 'init+fetch an SVN reposito
      	(
      	    cd g &&
     -	    git config --get svn-remote.svn.ignore-paths | fgrep "www"
    -+	    git config --get svn-remote.svn.ignore-paths | grep -F "www"
    ++	    git config --get svn-remote.svn.ignore-paths | grep www
      	)
      '
      
    @@ t/t9134-git-svn-ignore-paths.sh: test_expect_success 'SVN-side change outside of
      		svn_cmd commit -m "SVN-side change outside of www" &&
      		svn_cmd up &&
     -		svn_cmd log -v | fgrep "SVN-side change outside of www"
    -+		svn_cmd log -v | grep -F "SVN-side change outside of www"
    ++		svn_cmd log -v | grep "SVN-side change outside of www"
      	)
      '
      
    @@ t/t9134-git-svn-ignore-paths.sh: test_expect_success 'SVN-side change in and out
      		svn_cmd commit -m "SVN-side change in and out of ignored www" &&
      		svn_cmd up &&
     -		svn_cmd log -v | fgrep "SVN-side change in and out of ignored www"
    -+		svn_cmd log -v | grep -F "SVN-side change in and out of ignored www"
    ++		svn_cmd log -v | grep "SVN-side change in and out of ignored www"
      	)
      '
      
    @@ t/t9140-git-svn-reset.sh: test_expect_success 'fetch fails on modified hidden fi
      	  test_must_fail git svn fetch 2> ../errors &&
      	  git svn find-rev refs/remotes/git-svn > ../expect2 ) &&
     -	fgrep "not found in commit" errors &&
    -+	grep -F "not found in commit" errors &&
    ++	grep "not found in commit" errors &&
      	test_cmp expect expect2
      '
      
    @@ t/t9140-git-svn-reset.sh: test_expect_success 'refetch succeeds not ignoring any
      	  git svn fetch &&
      	  git svn rebase &&
     -	  fgrep "mod hidden" hid/hid.txt
    -+	  grep -F "mod hidden" hid/hid.txt
    ++	  grep "mod hidden" hid/hid.txt
      	)
      '
      
    @@ t/t9147-git-svn-include-paths.sh: test_expect_success 'init+fetch an SVN reposit
      	(
      	    cd g &&
     -	    git config --get svn-remote.svn.include-paths | fgrep "qqq"
    -+	    git config --get svn-remote.svn.include-paths | grep -F "qqq"
    ++	    git config --get svn-remote.svn.include-paths | grep qqq
      	)
      '
      
    @@ t/t9147-git-svn-include-paths.sh: test_expect_success 'SVN-side change outside o
      		svn_cmd commit -m "SVN-side change outside of www" &&
      		svn_cmd up &&
     -		svn_cmd log -v | fgrep "SVN-side change outside of www"
    -+		svn_cmd log -v | grep -F "SVN-side change outside of www"
    ++		svn_cmd log -v | grep "SVN-side change outside of www"
      	)
      '
      
    @@ t/t9147-git-svn-include-paths.sh: test_expect_success 'SVN-side change inside of
      		svn_cmd commit -m "SVN-side change inside of www/test_www.txt" &&
      		svn_cmd up &&
     -		svn_cmd log -v | fgrep "SVN-side change inside of www/test_www.txt"
    -+		svn_cmd log -v | grep -F "SVN-side change inside of www/test_www.txt"
    ++		svn_cmd log -v | grep "SVN-side change inside of www/test_www.txt"
      	)
      '
      
    @@ t/t9147-git-svn-include-paths.sh: test_expect_success 'SVN-side change in and ou
      		svn_cmd commit -m "SVN-side change in and out of ignored www" &&
      		svn_cmd up &&
     -		svn_cmd log -v | fgrep "SVN-side change in and out of ignored www"
    -+		svn_cmd log -v | grep -F "SVN-side change in and out of ignored www"
    ++		svn_cmd log -v | grep "SVN-side change in and out of ignored www"
      	)
      '
      
-- 
2.38.0.rc0


^ permalink raw reply	[flat|nested] 6+ messages in thread

* [PATCH v2 1/4] CodingGuidelines: allow grep -E
  2022-09-21 13:02 [PATCH v2 0/4] allow "grep -E", remove {e,f}grep usage Đoàn Trần Công Danh
@ 2022-09-21 13:02 ` Đoàn Trần Công Danh
  2022-09-21 13:02 ` [PATCH v2 2/4] t: remove \{m,n\} from BRE grep usage Đoàn Trần Công Danh
                   ` (2 subsequent siblings)
  3 siblings, 0 replies; 6+ messages in thread
From: Đoàn Trần Công Danh @ 2022-09-21 13:02 UTC (permalink / raw)
  To: git
  Cc: Đoàn Trần Công Danh, SZEDER Gábor,
	Phillip Wood, Junio C Hamano

Despite forbidden by CodingGuidelines, our usage of 'grep -E' has been
increased over the years, and noone has come and complained.

Let's lift the restriction.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 Documentation/CodingGuidelines | 2 --
 1 file changed, 2 deletions(-)

diff --git a/Documentation/CodingGuidelines b/Documentation/CodingGuidelines
index 9fca21cc5f..cb7a367ea0 100644
--- a/Documentation/CodingGuidelines
+++ b/Documentation/CodingGuidelines
@@ -162,8 +162,6 @@ For shell scripts specifically (not exhaustive):
 
    - We do not use \{m,n\};
 
-   - We do not use -E;
-
    - We do not use ? or + (which are \{0,1\} and \{1,\}
      respectively in BRE) but that goes without saying as these
      are ERE elements not BRE (note that \? and \+ are not even part
-- 
2.38.0.rc0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 2/4] t: remove \{m,n\} from BRE grep usage
  2022-09-21 13:02 [PATCH v2 0/4] allow "grep -E", remove {e,f}grep usage Đoàn Trần Công Danh
  2022-09-21 13:02 ` [PATCH v2 1/4] CodingGuidelines: allow grep -E Đoàn Trần Công Danh
@ 2022-09-21 13:02 ` Đoàn Trần Công Danh
  2022-09-21 18:06   ` Junio C Hamano
  2022-09-21 13:02 ` [PATCH v2 3/4] t: convert egrep usage to "grep -E" Đoàn Trần Công Danh
  2022-09-21 13:02 ` [PATCH v2 4/4] t: convert fgrep usage to "grep -F" Đoàn Trần Công Danh
  3 siblings, 1 reply; 6+ messages in thread
From: Đoàn Trần Công Danh @ 2022-09-21 13:02 UTC (permalink / raw)
  To: git
  Cc: Đoàn Trần Công Danh, SZEDER Gábor,
	Phillip Wood, Junio C Hamano

The CodingGuidelines says we should avoid \{m,n\} in BRE usage.
And their usages in our code base is limited, and subjectively
hard to read.

Replace them with ERE.

Except for "0\{40\}" which would be changed to "$ZERO_OID",
which is a better value for testing with:
GIT_TEST_DEFAULT_HASH=sha256

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---

 Phillip Wood said:
 > \{m,n\} is valid in a posix BRE[1]. If we're already using it without
 > anyone
 > complaining I think it would be better to update CodingGuidlines to allow
 > it.

 Yes, I agree. However, I think our usage of \{m,n\} is limited.
 Let's skip the lifting for now.

 t/t3200-branch.sh             | 4 ++--
 t/t3305-notes-fanout.sh       | 2 +-
 t/t3404-rebase-interactive.sh | 6 +++---
 t/t5550-http-fetch-dumb.sh    | 2 +-
 t/t5702-protocol-v2.sh        | 2 +-
 5 files changed, 8 insertions(+), 8 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 9723c2827c..b82cffc0b3 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -201,8 +201,8 @@ test_expect_success 'git branch -M baz bam should succeed when baz is checked ou
 
 test_expect_success 'git branch -M baz bam should add entries to .git/logs/HEAD' '
 	msg="Branch: renamed refs/heads/baz to refs/heads/bam" &&
-	grep " 0\{40\}.*$msg$" .git/logs/HEAD &&
-	grep "^0\{40\}.*$msg$" .git/logs/HEAD
+	grep " $ZERO_OID.*$msg$" .git/logs/HEAD &&
+	grep "^$ZERO_OID.*$msg$" .git/logs/HEAD
 '
 
 test_expect_success 'git branch -M should leave orphaned HEAD alone' '
diff --git a/t/t3305-notes-fanout.sh b/t/t3305-notes-fanout.sh
index 22ffe5bcb9..1ec1fb6715 100755
--- a/t/t3305-notes-fanout.sh
+++ b/t/t3305-notes-fanout.sh
@@ -9,7 +9,7 @@ path_has_fanout() {
 	path=$1 &&
 	fanout=$2 &&
 	after_last_slash=$(($(test_oid hexsz) - $fanout * 2)) &&
-	echo $path | grep -q "^\([0-9a-f]\{2\}/\)\{$fanout\}[0-9a-f]\{$after_last_slash\}$"
+	echo $path | grep -q -E "^([0-9a-f]{2}/){$fanout}[0-9a-f]{$after_last_slash}$"
 }
 
 touched_one_note_with_fanout() {
diff --git a/t/t3404-rebase-interactive.sh b/t/t3404-rebase-interactive.sh
index 688b01e3eb..4f5abb5ad2 100755
--- a/t/t3404-rebase-interactive.sh
+++ b/t/t3404-rebase-interactive.sh
@@ -1244,9 +1244,9 @@ test_expect_success 'short commit ID collide' '
 		test $colliding_id = "$(git rev-parse HEAD | cut -c 1-4)" &&
 		grep "^pick $colliding_id " \
 			.git/rebase-merge/git-rebase-todo.tmp &&
-		grep "^pick [0-9a-f]\{$hexsz\}" \
+		grep -E "^pick [0-9a-f]{$hexsz}" \
 			.git/rebase-merge/git-rebase-todo &&
-		grep "^pick [0-9a-f]\{$hexsz\}" \
+		grep -E "^pick [0-9a-f]{$hexsz}" \
 			.git/rebase-merge/git-rebase-todo.backup &&
 		git rebase --continue
 	) &&
@@ -1261,7 +1261,7 @@ test_expect_success 'respect core.abbrev' '
 		set_cat_todo_editor &&
 		test_must_fail git rebase -i HEAD~4 >todo-list
 	) &&
-	test 4 = $(grep -c "pick [0-9a-f]\{12,\}" todo-list)
+	test 4 = $(grep -c -E "pick [0-9a-f]{12,}" todo-list)
 '
 
 test_expect_success 'todo count' '
diff --git a/t/t5550-http-fetch-dumb.sh b/t/t5550-http-fetch-dumb.sh
index d7cf85ffea..8f182a3cbf 100755
--- a/t/t5550-http-fetch-dumb.sh
+++ b/t/t5550-http-fetch-dumb.sh
@@ -234,7 +234,7 @@ test_expect_success 'http-fetch --packfile' '
 		--index-pack-arg=--keep \
 		"$HTTPD_URL"/dumb/repo_pack.git/$p >out &&
 
-	grep "^keep.[0-9a-f]\{16,\}$" out &&
+	grep -E "^keep.[0-9a-f]{16,}$" out &&
 	cut -c6- out >packhash &&
 
 	# Ensure that the expected files are generated
diff --git a/t/t5702-protocol-v2.sh b/t/t5702-protocol-v2.sh
index 5d42a355a8..b33cd4afca 100755
--- a/t/t5702-protocol-v2.sh
+++ b/t/t5702-protocol-v2.sh
@@ -1001,7 +1001,7 @@ test_expect_success 'part of packfile response provided as URI' '
 	do
 		git verify-pack --object-format=$(test_oid algo) --verbose $idx >out &&
 		{
-			grep "^[0-9a-f]\{16,\} " out || :
+			grep -E "^[0-9a-f]{16,} " out || :
 		} >out.objectlist &&
 		if test_line_count = 1 out.objectlist
 		then
-- 
2.38.0.rc0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 3/4] t: convert egrep usage to "grep -E"
  2022-09-21 13:02 [PATCH v2 0/4] allow "grep -E", remove {e,f}grep usage Đoàn Trần Công Danh
  2022-09-21 13:02 ` [PATCH v2 1/4] CodingGuidelines: allow grep -E Đoàn Trần Công Danh
  2022-09-21 13:02 ` [PATCH v2 2/4] t: remove \{m,n\} from BRE grep usage Đoàn Trần Công Danh
@ 2022-09-21 13:02 ` Đoàn Trần Công Danh
  2022-09-21 13:02 ` [PATCH v2 4/4] t: convert fgrep usage to "grep -F" Đoàn Trần Công Danh
  3 siblings, 0 replies; 6+ messages in thread
From: Đoàn Trần Công Danh @ 2022-09-21 13:02 UTC (permalink / raw)
  To: git
  Cc: Đoàn Trần Công Danh, SZEDER Gábor,
	Phillip Wood, Junio C Hamano

Despite POSIX states that:

> The old egrep and fgrep commands are likely to be supported for many
> years to come as implementation extensions, allowing historical
> applications to operate unmodified.

GNU grep 3.8 started to warn[1]:

> The egrep and fgrep commands, which have been deprecated since
> release 2.5.3 (2007), now warn that they are obsolescent and should
> be replaced by grep -E and grep -F.

Prepare for their removal in the future.

[1]: https://lists.gnu.org/archive/html/info-gnu/2022-09/msg00001.html

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/perf/run                           |  4 ++--
 t/t1304-default-acl.sh               |  4 ++--
 t/t3702-add-edit.sh                  |  2 +-
 t/t4014-format-patch.sh              |  8 ++++----
 t/t5320-delta-islands.sh             |  2 +-
 t/t7527-builtin-fsmonitor.sh         | 18 +++++++++---------
 t/t7701-repack-unpack-unreachable.sh |  4 ++--
 t/t9001-send-email.sh                |  8 ++++----
 t/t9814-git-p4-rename.sh             |  2 +-
 t/t9815-git-p4-submit-fail.sh        |  4 ++--
 t/test-lib-functions.sh              |  2 +-
 11 files changed, 29 insertions(+), 29 deletions(-)

diff --git a/t/perf/run b/t/perf/run
index 33da4d2aba..34115edec3 100755
--- a/t/perf/run
+++ b/t/perf/run
@@ -232,10 +232,10 @@ then
 	)
 elif test -n "$GIT_PERF_SUBSECTION"
 then
-	egrep "^$GIT_PERF_SUBSECTION\$" "$TEST_RESULTS_DIR"/run_subsections.names >/dev/null ||
+	grep -E "^$GIT_PERF_SUBSECTION\$" "$TEST_RESULTS_DIR"/run_subsections.names >/dev/null ||
 		die "subsection '$GIT_PERF_SUBSECTION' not found in '$GIT_PERF_CONFIG_FILE'"
 
-	egrep "^$GIT_PERF_SUBSECTION\$" "$TEST_RESULTS_DIR"/run_subsections.names | while read -r subsec
+	grep -E "^$GIT_PERF_SUBSECTION\$" "$TEST_RESULTS_DIR"/run_subsections.names | while read -r subsec
 	do
 		(
 			GIT_PERF_SUBSECTION="$subsec"
diff --git a/t/t1304-default-acl.sh b/t/t1304-default-acl.sh
index 335d3f3211..c69ae41306 100755
--- a/t/t1304-default-acl.sh
+++ b/t/t1304-default-acl.sh
@@ -18,7 +18,7 @@ test_expect_success 'checking for a working acl setup' '
 	if setfacl -m d:m:rwx -m u:root:rwx . &&
 	   getfacl . | grep user:root:rwx &&
 	   touch should-have-readable-acl &&
-	   getfacl should-have-readable-acl | egrep "mask::?rw-"
+	   getfacl should-have-readable-acl | grep -E "mask::?rw-"
 	then
 		test_set_prereq SETFACL
 	fi
@@ -34,7 +34,7 @@ check_perms_and_acl () {
 	getfacl "$1" > actual &&
 	grep -q "user:root:rwx" actual &&
 	grep -q "user:${LOGNAME}:rwx" actual &&
-	egrep "mask::?r--" actual > /dev/null 2>&1 &&
+	grep -E "mask::?r--" actual > /dev/null 2>&1 &&
 	grep -q "group::---" actual || false
 }
 
diff --git a/t/t3702-add-edit.sh b/t/t3702-add-edit.sh
index a1801a8cbd..82bfb2fd2a 100755
--- a/t/t3702-add-edit.sh
+++ b/t/t3702-add-edit.sh
@@ -100,7 +100,7 @@ EOF
 
 echo "#!$SHELL_PATH" >fake-editor.sh
 cat >> fake-editor.sh <<\EOF
-egrep -v '^index' "$1" >orig-patch &&
+grep -E -v '^index' "$1" >orig-patch &&
 mv -f patch "$1"
 EOF
 
diff --git a/t/t4014-format-patch.sh b/t/t4014-format-patch.sh
index ad5c029279..de1da4673d 100755
--- a/t/t4014-format-patch.sh
+++ b/t/t4014-format-patch.sh
@@ -1457,7 +1457,7 @@ append_signoff()
 	C=$(git commit-tree HEAD^^{tree} -p HEAD) &&
 	git format-patch --stdout --signoff $C^..$C >append_signoff.patch &&
 	sed -n -e "1,/^---$/p" append_signoff.patch |
-		egrep -n "^Subject|Sign|^$"
+		grep -E -n "^Subject|Sign|^$"
 }
 
 test_expect_success 'signoff: commit with no body' '
@@ -2274,10 +2274,10 @@ test_expect_success 'format-patch --base with --attach' '
 test_expect_success 'format-patch --attach cover-letter only is non-multipart' '
 	test_when_finished "rm -fr patches" &&
 	git format-patch -o patches --cover-letter --attach=mimemime --base=HEAD~ -1 &&
-	! egrep "^--+mimemime" patches/0000*.patch &&
-	egrep "^--+mimemime$" patches/0001*.patch >output &&
+	! grep -E "^--+mimemime" patches/0000*.patch &&
+	grep -E "^--+mimemime$" patches/0001*.patch >output &&
 	test_line_count = 2 output &&
-	egrep "^--+mimemime--$" patches/0001*.patch >output &&
+	grep -E "^--+mimemime--$" patches/0001*.patch >output &&
 	test_line_count = 1 output
 '
 
diff --git a/t/t5320-delta-islands.sh b/t/t5320-delta-islands.sh
index 124d47603d..406363381f 100755
--- a/t/t5320-delta-islands.sh
+++ b/t/t5320-delta-islands.sh
@@ -134,7 +134,7 @@ test_expect_success 'island core places core objects first' '
 	    repack -adfi &&
 	git verify-pack -v .git/objects/pack/*.pack |
 	cut -d" " -f1 |
-	egrep "$root|$two" >actual &&
+	grep -E "$root|$two" >actual &&
 	test_cmp expect actual
 '
 
diff --git a/t/t7527-builtin-fsmonitor.sh b/t/t7527-builtin-fsmonitor.sh
index 56c0dfffea..1746d30cf6 100755
--- a/t/t7527-builtin-fsmonitor.sh
+++ b/t/t7527-builtin-fsmonitor.sh
@@ -939,9 +939,9 @@ test_expect_success CASE_INSENSITIVE_FS 'case insensitive+preserving' '
 	# directories and files that we touched.  We may or may not get a
 	# trailing slash on modified directories.
 	#
-	egrep "^event: abc/?$"       ./insensitive.trace &&
-	egrep "^event: abc/def/?$"   ./insensitive.trace &&
-	egrep "^event: abc/def/xyz$" ./insensitive.trace
+	grep -E "^event: abc/?$"       ./insensitive.trace &&
+	grep -E "^event: abc/def/?$"   ./insensitive.trace &&
+	grep -E "^event: abc/def/xyz$" ./insensitive.trace
 '
 
 # The variable "unicode_debug" is defined in the following library
@@ -983,20 +983,20 @@ test_expect_success !UNICODE_COMPOSITION_SENSITIVE 'Unicode nfc/nfd' '
 	then
 		# We should have seen NFC event from OS.
 		# We should not have synthesized an NFD event.
-		egrep    "^event: nfc/c_${utf8_nfc}/?$" ./unicode.trace &&
-		egrep -v "^event: nfc/c_${utf8_nfd}/?$" ./unicode.trace
+		grep -E    "^event: nfc/c_${utf8_nfc}/?$" ./unicode.trace &&
+		grep -E -v "^event: nfc/c_${utf8_nfd}/?$" ./unicode.trace
 	else
 		# We should have seen NFD event from OS.
 		# We should have synthesized an NFC event.
-		egrep "^event: nfc/c_${utf8_nfd}/?$" ./unicode.trace &&
-		egrep "^event: nfc/c_${utf8_nfc}/?$" ./unicode.trace
+		grep -E "^event: nfc/c_${utf8_nfd}/?$" ./unicode.trace &&
+		grep -E "^event: nfc/c_${utf8_nfc}/?$" ./unicode.trace
 	fi &&
 
 	# We assume UNICODE_NFD_PRESERVED.
 	# We should have seen explicit NFD from OS.
 	# We should have synthesized an NFC event.
-	egrep "^event: nfd/d_${utf8_nfd}/?$" ./unicode.trace &&
-	egrep "^event: nfd/d_${utf8_nfc}/?$" ./unicode.trace
+	grep -E "^event: nfd/d_${utf8_nfd}/?$" ./unicode.trace &&
+	grep -E "^event: nfd/d_${utf8_nfc}/?$" ./unicode.trace
 '
 
 test_done
diff --git a/t/t7701-repack-unpack-unreachable.sh b/t/t7701-repack-unpack-unreachable.sh
index 937f89ee8c..b7ac4f598a 100755
--- a/t/t7701-repack-unpack-unreachable.sh
+++ b/t/t7701-repack-unpack-unreachable.sh
@@ -35,7 +35,7 @@ test_expect_success '-A with -d option leaves unreachable objects unpacked' '
 	git repack -A -d -l &&
 	# verify objects are packed in repository
 	test 3 = $(git verify-pack -v -- .git/objects/pack/*.idx |
-		   egrep "^($fsha1|$csha1|$tsha1) " |
+		   grep -E "^($fsha1|$csha1|$tsha1) " |
 		   sort | uniq | wc -l) &&
 	git show $fsha1 &&
 	git show $csha1 &&
@@ -49,7 +49,7 @@ test_expect_success '-A with -d option leaves unreachable objects unpacked' '
 	git repack -A -d -l &&
 	# verify objects are retained unpacked
 	test 0 = $(git verify-pack -v -- .git/objects/pack/*.idx |
-		   egrep "^($fsha1|$csha1|$tsha1) " |
+		   grep -E "^($fsha1|$csha1|$tsha1) " |
 		   sort | uniq | wc -l) &&
 	git show $fsha1 &&
 	git show $csha1 &&
diff --git a/t/t9001-send-email.sh b/t/t9001-send-email.sh
index 01c74b8b07..1130ef21b3 100755
--- a/t/t9001-send-email.sh
+++ b/t/t9001-send-email.sh
@@ -1519,7 +1519,7 @@ test_expect_success $PREREQ 'asks about and fixes 8bit encodings' '
 	grep "do not declare a Content-Transfer-Encoding" stdout &&
 	grep email-using-8bit stdout &&
 	grep "Which 8bit encoding" stdout &&
-	egrep "Content|MIME" msgtxt1 >actual &&
+	grep -E "Content|MIME" msgtxt1 >actual &&
 	test_cmp content-type-decl actual
 '
 
@@ -1530,7 +1530,7 @@ test_expect_success $PREREQ 'sendemail.8bitEncoding works' '
 	git send-email --from=author@example.com --to=nobody@example.com \
 			--smtp-server="$(pwd)/fake.sendmail" \
 			email-using-8bit >stdout &&
-	egrep "Content|MIME" msgtxt1 >actual &&
+	grep -E "Content|MIME" msgtxt1 >actual &&
 	test_cmp content-type-decl actual
 '
 
@@ -1545,7 +1545,7 @@ test_expect_success $PREREQ 'sendemail.8bitEncoding in .git/config overrides --g
 	git send-email --from=author@example.com --to=nobody@example.com \
 			--smtp-server="$(pwd)/fake.sendmail" \
 			email-using-8bit >stdout &&
-	egrep "Content|MIME" msgtxt1 >actual &&
+	grep -E "Content|MIME" msgtxt1 >actual &&
 	test_cmp content-type-decl actual
 '
 
@@ -1557,7 +1557,7 @@ test_expect_success $PREREQ '--8bit-encoding overrides sendemail.8bitEncoding' '
 			--smtp-server="$(pwd)/fake.sendmail" \
 			--8bit-encoding=UTF-8 \
 			email-using-8bit >stdout &&
-	egrep "Content|MIME" msgtxt1 >actual &&
+	grep -E "Content|MIME" msgtxt1 >actual &&
 	test_cmp content-type-decl actual
 '
 
diff --git a/t/t9814-git-p4-rename.sh b/t/t9814-git-p4-rename.sh
index 468767cbf4..2a9838f37f 100755
--- a/t/t9814-git-p4-rename.sh
+++ b/t/t9814-git-p4-rename.sh
@@ -216,7 +216,7 @@ test_expect_success 'detect copies' '
 # variable exists, which allows admins to disable the "p4 move" command.
 test_lazy_prereq P4D_HAVE_CONFIGURABLE_RUN_MOVE_ALLOW '
 	p4 configure show run.move.allow >out &&
-	egrep ^run.move.allow: out
+	grep -E ^run.move.allow: out
 '
 
 # If move can be disabled, turn it off and test p4 move handling
diff --git a/t/t9815-git-p4-submit-fail.sh b/t/t9815-git-p4-submit-fail.sh
index 9779dc0d11..0ca9937de6 100755
--- a/t/t9815-git-p4-submit-fail.sh
+++ b/t/t9815-git-p4-submit-fail.sh
@@ -417,8 +417,8 @@ test_expect_success 'cleanup chmod after submit cancel' '
 		! p4 fstat -T action text &&
 		test_path_is_file text+x &&
 		! p4 fstat -T action text+x &&
-		ls -l text | egrep ^-r-- &&
-		ls -l text+x | egrep ^-r-x
+		ls -l text | grep -E ^-r-- &&
+		ls -l text+x | grep -E ^-r-x
 	)
 '
 
diff --git a/t/test-lib-functions.sh b/t/test-lib-functions.sh
index c6479f24eb..527a714500 100644
--- a/t/test-lib-functions.sh
+++ b/t/test-lib-functions.sh
@@ -897,7 +897,7 @@ test_path_is_symlink () {
 test_dir_is_empty () {
 	test "$#" -ne 1 && BUG "1 param"
 	test_path_is_dir "$1" &&
-	if test -n "$(ls -a1 "$1" | egrep -v '^\.\.?$')"
+	if test -n "$(ls -a1 "$1" | grep -E -v '^\.\.?$')"
 	then
 		echo "Directory '$1' is not empty, it contains:"
 		ls -la "$1"
-- 
2.38.0.rc0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* [PATCH v2 4/4] t: convert fgrep usage to "grep -F"
  2022-09-21 13:02 [PATCH v2 0/4] allow "grep -E", remove {e,f}grep usage Đoàn Trần Công Danh
                   ` (2 preceding siblings ...)
  2022-09-21 13:02 ` [PATCH v2 3/4] t: convert egrep usage to "grep -E" Đoàn Trần Công Danh
@ 2022-09-21 13:02 ` Đoàn Trần Công Danh
  3 siblings, 0 replies; 6+ messages in thread
From: Đoàn Trần Công Danh @ 2022-09-21 13:02 UTC (permalink / raw)
  To: git
  Cc: Đoàn Trần Công Danh, SZEDER Gábor,
	Phillip Wood, Junio C Hamano

Despite POSIX states that:

> The old egrep and fgrep commands are likely to be supported for many
> years to come as implementation extensions, allowing historical
> applications to operate unmodified.

GNU grep 3.8 started to warn[1]:

> The egrep and fgrep commands, which have been deprecated since
> release 2.5.3 (2007), now warn that they are obsolescent and should
> be replaced by grep -E and grep -F.

Prepare for their removal in the future.

[1]: https://lists.gnu.org/archive/html/info-gnu/2022-09/msg00001.html

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t3700-add.sh                     | 2 +-
 t/t7003-filter-branch.sh           | 4 ++--
 t/t9133-git-svn-nested-git-repo.sh | 6 +++---
 t/t9134-git-svn-ignore-paths.sh    | 8 ++++----
 t/t9140-git-svn-reset.sh           | 4 ++--
 t/t9147-git-svn-include-paths.sh   | 8 ++++----
 6 files changed, 16 insertions(+), 16 deletions(-)

diff --git a/t/t3700-add.sh b/t/t3700-add.sh
index 8689b48589..51afbd7b24 100755
--- a/t/t3700-add.sh
+++ b/t/t3700-add.sh
@@ -291,7 +291,7 @@ test_expect_success BSLASHPSPEC "git add 'fo\\[ou\\]bar' ignores foobar" '
 	git reset --hard &&
 	touch fo\[ou\]bar foobar &&
 	git add '\''fo\[ou\]bar'\'' &&
-	git ls-files fo\[ou\]bar | fgrep fo\[ou\]bar &&
+	git ls-files fo\[ou\]bar | grep -F fo\[ou\]bar &&
 	! ( git ls-files foobar | grep foobar )
 '
 
diff --git a/t/t7003-filter-branch.sh b/t/t7003-filter-branch.sh
index e18a218952..f6aebe92ff 100755
--- a/t/t7003-filter-branch.sh
+++ b/t/t7003-filter-branch.sh
@@ -49,7 +49,7 @@ test_expect_success 'result is really identical' '
 test_expect_success 'rewrite bare repository identically' '
 	(git config core.bare true && cd .git &&
 	 git filter-branch branch > filter-output 2>&1 &&
-	! fgrep fatal filter-output)
+	! grep fatal filter-output)
 '
 git config core.bare false
 test_expect_success 'result is really identical' '
@@ -506,7 +506,7 @@ test_expect_success 'rewrite repository including refs that point at non-commit
 	git tag -a -m "tag to a tree" treetag $new_tree &&
 	git reset --hard HEAD &&
 	git filter-branch -f -- --all >filter-output 2>&1 &&
-	! fgrep fatal filter-output
+	! grep fatal filter-output
 '
 
 test_expect_success 'filter-branch handles ref deletion' '
diff --git a/t/t9133-git-svn-nested-git-repo.sh b/t/t9133-git-svn-nested-git-repo.sh
index f894860867..d8d536269c 100755
--- a/t/t9133-git-svn-nested-git-repo.sh
+++ b/t/t9133-git-svn-nested-git-repo.sh
@@ -35,7 +35,7 @@ test_expect_success 'SVN-side change outside of .git' '
 		echo b >> a &&
 		svn_cmd commit -m "SVN-side change outside of .git" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change outside of .git"
+		svn_cmd log -v | grep -F "SVN-side change outside of .git"
 	)
 '
 
@@ -59,7 +59,7 @@ test_expect_success 'SVN-side change inside of .git' '
 		svn_cmd add --force .git &&
 		svn_cmd commit -m "SVN-side change inside of .git" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change inside of .git"
+		svn_cmd log -v | grep -F "SVN-side change inside of .git"
 	)
 '
 
@@ -82,7 +82,7 @@ test_expect_success 'SVN-side change in and out of .git' '
 		git commit -m "add a inside an SVN repo" &&
 		svn_cmd commit -m "SVN-side change in and out of .git" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change in and out of .git"
+		svn_cmd log -v | grep -F "SVN-side change in and out of .git"
 	)
 '
 
diff --git a/t/t9134-git-svn-ignore-paths.sh b/t/t9134-git-svn-ignore-paths.sh
index 4a77eb9f60..3188400226 100755
--- a/t/t9134-git-svn-ignore-paths.sh
+++ b/t/t9134-git-svn-ignore-paths.sh
@@ -43,7 +43,7 @@ test_expect_success 'init+fetch an SVN repository with ignored www directory' '
 test_expect_success 'verify ignore-paths config saved by clone' '
 	(
 	    cd g &&
-	    git config --get svn-remote.svn.ignore-paths | fgrep "www"
+	    git config --get svn-remote.svn.ignore-paths | grep www
 	)
 '
 
@@ -53,7 +53,7 @@ test_expect_success 'SVN-side change outside of www' '
 		echo b >> qqq/test_qqq.txt &&
 		svn_cmd commit -m "SVN-side change outside of www" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change outside of www"
+		svn_cmd log -v | grep "SVN-side change outside of www"
 	)
 '
 
@@ -85,7 +85,7 @@ test_expect_success 'SVN-side change inside of ignored www' '
 		echo zaq >> www/test_www.txt &&
 		svn_cmd commit -m "SVN-side change inside of www/test_www.txt" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change inside of www/test_www.txt"
+		svn_cmd log -v | grep -F "SVN-side change inside of www/test_www.txt"
 	)
 '
 
@@ -118,7 +118,7 @@ test_expect_success 'SVN-side change in and out of ignored www' '
 		echo ygg >> qqq/test_qqq.txt &&
 		svn_cmd commit -m "SVN-side change in and out of ignored www" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change in and out of ignored www"
+		svn_cmd log -v | grep "SVN-side change in and out of ignored www"
 	)
 '
 
diff --git a/t/t9140-git-svn-reset.sh b/t/t9140-git-svn-reset.sh
index e855904629..a420b2a87a 100755
--- a/t/t9140-git-svn-reset.sh
+++ b/t/t9140-git-svn-reset.sh
@@ -43,7 +43,7 @@ test_expect_success 'fetch fails on modified hidden file' '
 	  git svn find-rev refs/remotes/git-svn > ../expect &&
 	  test_must_fail git svn fetch 2> ../errors &&
 	  git svn find-rev refs/remotes/git-svn > ../expect2 ) &&
-	fgrep "not found in commit" errors &&
+	grep "not found in commit" errors &&
 	test_cmp expect expect2
 '
 
@@ -59,7 +59,7 @@ test_expect_success 'refetch succeeds not ignoring any files' '
 	( cd g &&
 	  git svn fetch &&
 	  git svn rebase &&
-	  fgrep "mod hidden" hid/hid.txt
+	  grep "mod hidden" hid/hid.txt
 	)
 '
 
diff --git a/t/t9147-git-svn-include-paths.sh b/t/t9147-git-svn-include-paths.sh
index 257fc8f2f8..63fa0b6732 100755
--- a/t/t9147-git-svn-include-paths.sh
+++ b/t/t9147-git-svn-include-paths.sh
@@ -45,7 +45,7 @@ test_expect_success 'init+fetch an SVN repository with included qqq directory' '
 test_expect_success 'verify include-paths config saved by clone' '
 	(
 	    cd g &&
-	    git config --get svn-remote.svn.include-paths | fgrep "qqq"
+	    git config --get svn-remote.svn.include-paths | grep qqq
 	)
 '
 
@@ -55,7 +55,7 @@ test_expect_success 'SVN-side change outside of www' '
 		echo b >> qqq/test_qqq.txt &&
 		svn_cmd commit -m "SVN-side change outside of www" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change outside of www"
+		svn_cmd log -v | grep "SVN-side change outside of www"
 	)
 '
 
@@ -87,7 +87,7 @@ test_expect_success 'SVN-side change inside of ignored www' '
 		echo zaq >> www/test_www.txt &&
 		svn_cmd commit -m "SVN-side change inside of www/test_www.txt" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change inside of www/test_www.txt"
+		svn_cmd log -v | grep "SVN-side change inside of www/test_www.txt"
 	)
 '
 
@@ -120,7 +120,7 @@ test_expect_success 'SVN-side change in and out of included qqq' '
 		echo ygg >> qqq/test_qqq.txt &&
 		svn_cmd commit -m "SVN-side change in and out of ignored www" &&
 		svn_cmd up &&
-		svn_cmd log -v | fgrep "SVN-side change in and out of ignored www"
+		svn_cmd log -v | grep "SVN-side change in and out of ignored www"
 	)
 '
 
-- 
2.38.0.rc0


^ permalink raw reply related	[flat|nested] 6+ messages in thread

* Re: [PATCH v2 2/4] t: remove \{m,n\} from BRE grep usage
  2022-09-21 13:02 ` [PATCH v2 2/4] t: remove \{m,n\} from BRE grep usage Đoàn Trần Công Danh
@ 2022-09-21 18:06   ` Junio C Hamano
  0 siblings, 0 replies; 6+ messages in thread
From: Junio C Hamano @ 2022-09-21 18:06 UTC (permalink / raw)
  To: Đoàn Trần Công Danh
  Cc: git, SZEDER Gábor, Phillip Wood

Đoàn Trần Công Danh  <congdanhqx@gmail.com> writes:

> The CodingGuidelines says we should avoid \{m,n\} in BRE usage.
> And their usages in our code base is limited, and subjectively
> hard to read.
>
> Replace them with ERE.

OK.  I do not personally mind allowing \{0,1\} in BRE (which would
give us a portable way to express '?'), but we are not forbidding
ERE in any way, so I am OK with the direction.

> Except for "0\{40\}" which would be changed to "$ZERO_OID",
> which is a better value for testing with:
> GIT_TEST_DEFAULT_HASH=sha256

Absolutely.  This alone is a change worth doing regardless of the
portability issues.

> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>
>  Phillip Wood said:
>  > \{m,n\} is valid in a posix BRE[1]. If we're already using it without
>  > anyone
>  > complaining I think it would be better to update CodingGuidlines to allow
>  > it.
>
>  Yes, I agree. However, I think our usage of \{m,n\} is limited.
>  Let's skip the lifting for now.

OK.

^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2022-09-21 18:07 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2022-09-21 13:02 [PATCH v2 0/4] allow "grep -E", remove {e,f}grep usage Đoàn Trần Công Danh
2022-09-21 13:02 ` [PATCH v2 1/4] CodingGuidelines: allow grep -E Đoàn Trần Công Danh
2022-09-21 13:02 ` [PATCH v2 2/4] t: remove \{m,n\} from BRE grep usage Đoàn Trần Công Danh
2022-09-21 18:06   ` Junio C Hamano
2022-09-21 13:02 ` [PATCH v2 3/4] t: convert egrep usage to "grep -E" Đoàn Trần Công Danh
2022-09-21 13:02 ` [PATCH v2 4/4] t: convert fgrep usage to "grep -F" Đoàn Trần Công Danh

Code repositories for project(s) associated with this public inbox

	https://80x24.org/mirrors/git.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).