[PATCH 0/4] allow grep -E, and remove egrep

[PATCH 0/4] allow grep -E, and remove egrep

[Đoàn Trần Công Danh]

Despite our CodingGuidelines dictates that grep -E and or grep \{m,n\}
are forbidden. Our code bases uses them all over place.

In addiion, GNU grep 3.8 started to spit warning 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}"

Đ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                    |  6 ++++--
 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, 55 insertions(+), 55 deletions(-)

-- 
2.38.0.rc0

[PATCH 1/4] CodingGuidelines: allow grep -E

[Đoàn Trần Công Danh]

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

[PATCH 2/4] t: remove \{m,n\} from BRE grep usage

[Đoàn Trần Công Danh]

\{m,n\} is a GNU extension to BRE, and it's forbidden by our
CodingGuidelines.

Change to fixed strings or ERE.

Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
---
 t/t3200-branch.sh             | 6 ++++--
 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, 10 insertions(+), 8 deletions(-)

diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
index 9723c2827c..f05ac1fe0b 100755
--- a/t/t3200-branch.sh
+++ b/t/t3200-branch.sh
@@ -201,8 +201,10 @@ 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
+	zero="00000000" &&
+	zero="$zero$zero$zero$zero$zero" &&
+	grep " $zero.*$msg$" .git/logs/HEAD &&
+	grep "^$zero.*$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..aa3bb2e308 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][0-9a-f]/){$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

[PATCH 3/4] t: convert egrep usage to "grep -E"

[Đoàn Trần Công Danh]

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

[PATCH 4/4] t: convert fgrep usage to "grep -F"

[Đoàn Trần Công Danh]

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..a00cdba049 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 -F 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 -F 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..21b5d285d5 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 -F "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 -F "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 -F "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..b3c9425d55 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 -F "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 -F "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..e53bfc02f6 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 -F "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 -F "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 -F "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 -F "SVN-side change in and out of ignored www"
 	)
 '
 
-- 
2.38.0.rc0

Re: [PATCH 2/4] t: remove \{m,n\} from BRE grep usage

[SZEDER Gábor]

On Tue, Sep 20, 2022 at 10:49:14PM +0700, Đoàn Trần Công Danh wrote:
> \{m,n\} is a GNU extension to BRE, and it's forbidden by our
> CodingGuidelines.
> 
> Change to fixed strings or ERE.
> 
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>  t/t3200-branch.sh             | 6 ++++--
>  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, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 9723c2827c..f05ac1fe0b 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -201,8 +201,10 @@ 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
> +	zero="00000000" &&
> +	zero="$zero$zero$zero$zero$zero" &&
> +	grep " $zero.*$msg$" .git/logs/HEAD &&
> +	grep "^$zero.*$msg$" .git/logs/HEAD

I think these could use $ZERO_OID instead.

Re: [PATCH 2/4] t: remove \{m,n\} from BRE grep usage

[Phillip Wood]

Hi Đoàn

On 20/09/2022 16:49, Đoàn Trần Công Danh wrote:
> \{m,n\} is a GNU extension to BRE, and it's forbidden by our
> CodingGuidelines.

\{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.

Best Wishes

Phillip

[1] 
https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html

Section 9.3.6 says
...
5. When a BRE matching a single character, a subexpression, or a 
back-reference is followed by an interval expression of the format 
"\{m\}", "\{m,\}", or "\{m,n\}", together with that interval expression 
it shall match what repeated consecutive occurrences of the BRE would 
match. The values of m and n are decimal integers in the range 0 <= m<= 
n<= {RE_DUP_MAX}, where m specifies the exact or minimum number of 
occurrences and n specifies the maximum number of occurrences. The 
expression "\{m\}" shall match exactly m occurrences of the preceding 
BRE, "\{m,\}" shall match at least m occurrences, and "\{m,n\}" shall 
match any number of occurrences between m and n, inclusive.

For example, in the string "abababccccccd" the BRE "c\{3\}" is matched 
by characters seven to nine, the BRE "\(ab\)\{4,\}" is not matched at 
all, and the BRE "c\{1,3\}d" is matched by characters ten to thirteen.

> Change to fixed strings or ERE.
> 
> Signed-off-by: Đoàn Trần Công Danh <congdanhqx@gmail.com>
> ---
>   t/t3200-branch.sh             | 6 ++++--
>   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, 10 insertions(+), 8 deletions(-)
> 
> diff --git a/t/t3200-branch.sh b/t/t3200-branch.sh
> index 9723c2827c..f05ac1fe0b 100755
> --- a/t/t3200-branch.sh
> +++ b/t/t3200-branch.sh
> @@ -201,8 +201,10 @@ 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
> +	zero="00000000" &&
> +	zero="$zero$zero$zero$zero$zero" &&
> +	grep " $zero.*$msg$" .git/logs/HEAD &&
> +	grep "^$zero.*$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..aa3bb2e308 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][0-9a-f]/){$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

Re: [PATCH 0/4] allow grep -E, and remove egrep

[Junio C Hamano]

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

> Despite our CodingGuidelines dictates that grep -E and or grep \{m,n\}
> are forbidden. Our code bases uses them all over place.

That makes it sounds like rules dictating and coders resisting, but
that is not what is happening.

Portability guidelines need updating as tools on different platforms
change over time.  An effort like this series is very much
appreciated from time to time.

Re: [PATCH 2/4] t: remove \{m,n\} from BRE grep usage

[Junio C Hamano]

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

> \{m,n\} is a GNU extension to BRE, and it's forbidden by our
> CodingGuidelines.

Is it?

https://pubs.opengroup.org/onlinepubs/9699919799/basedefs/V1_chap09.html#tag_09_03_06

says otherwise.  There may be some other GNU extensions to BRE that
allows you to write ERE elements with different syntax, but I doubt
this is one of them.  Perhaps you are thinking about "A\|B"
alternation?  In ERE "A|B" is alternation, and GNU BRE allows "A\|B"
but that is outside POSIX, IIUC.  "A\+" (1 or more of A) and "A\?"
(0 or 1 of A) are the same way.

We do say we don't use "\{m,n\}" in the guidelines, which was
written more than 10 years ago that codifies the habit acquired
while having to deal with regexp implementations of various UNIX
variants like early SystemV and BSD4 from more than 20 years ago.

If we are using the syntax in many of our tests that everybody runs,
that can be taken as a sign that those platforms who had problems
with the syntax have died out, or at least to them Git does not
matter.

So my prefererence is to

 - Allow \{m,n\} when it makes sense and codify it in the guidelines

 - Rewriting tests is fine if it makes the result easier to read,
   but it shouldn't be done for the sole purpose of getting rid of
   the \{m,n\} syntax.

 - As there are folks without GNU, until these GNU extensions for |,
   +, and ? are adopted widely, keep forbidding their use in BRE.

>  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
> +	zero="00000000" &&
> +	zero="$zero$zero$zero$zero$zero" &&
> +	grep " $zero.*$msg$" .git/logs/HEAD &&
> +	grep "^$zero.*$msg$" .git/logs/HEAD
>  '

This is not good

>  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..aa3bb2e308 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][0-9a-f]/){$fanout}[0-9a-f]{$after_last_slash}$"

The use of -E makes it more readable and is good.  The innermost "a
pair of hexdigits" that would repeat $fanout times may be easier to
read if you keep the {2}, though.