Re: [PATCH] Makefile, git-sh-setup.sh, t/: do not use `egrep` or `fgrep`

["Ævar Arnfjörð Bjarmason" <avarab@gmail.com>]

On Sat, Nov 13 2021, Ville Skyttä wrote:

> `egrep` and `fgrep` have been deprecated in GNU grep since 2007, and in
> current post 3.7 Git they have been made to emit obsolescence warnings.
>
> `grep -E` and `grep -F` on the other hand have been in POSIX and its
> predecessors for decades; use them instead, and use basic regular
> expressions instead of extended ones where applicable.
>
> Signed-off-by: Ville Skyttä <ville.skytta@iki.fi>
> ---
>  Makefile                             | 2 +-
>  git-sh-setup.sh                      | 2 +-
>  t/perf/run                           | 4 ++--
>  t/t1304-default-acl.sh               | 4 ++--
>  t/t3700-add.sh                       | 2 +-
>  t/t3702-add-edit.sh                  | 2 +-
>  t/t4014-format-patch.sh              | 8 ++++----
>  t/t5320-delta-islands.sh             | 2 +-
>  t/t7003-filter-branch.sh             | 4 ++--
>  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 +-
>  18 files changed, 38 insertions(+), 38 deletions(-)

Sounds sensible, but as far as sane_egrep goes this branch would be
better built on top of my ab/sh-retire-helper-functions, i.e. the
sane_egrep you're changing here will be gone entirely once that merges
down (post-upcoming release, presumably).

On the other hand that conflict is rather minor.

> [...]
>  # 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..ce75d4debe 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 ^-r-- &&
> +		ls -l text+x | grep ^-r-x
>  	)
>  '
>  
> diff 

This looks completely fine since this use is trivial, i.e. let's just
use BRE here.

But just a note that on some implementations BRE & ERE aren't just a
syntax difference, but they dispatch to entirely different regex
engines. I've seen very different performance characteristics with BRE
v.s. ERE, and even cases on some GNU software (can't recall the
specifics now, sorry, I think on glibc) where some things that are
pathological and have runaway memory use on BRE would be just fine on
ERE.

So again, it doesn't matter here, but just since you're poking in this
area a note that -E isn't just "I'm using ERE features". I think it's
probably a good idea to always use it, unles there's a good reason not
to.