Re: [PATCH] Makefile, git-sh-setup.sh, t/: do not use `egrep` or `fgrep`
- From
Ævar Arnfjörð Bjarmason <avarab@gmail.com>
- Date
- Nov 12, 2021, 23:02 UTC
- Message-ID
- <211113.8635o1ug4g.gmgdl@evledraar.gmail.com>
- In-Reply-To
- <20211112225334.1862016-1-ville.skytta@iki.fi>
On Sat, Nov 13 2021, Ville Skyttä wrote:
Show 28 quoted lines
> `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.
Show 18 quoted lines
> [...] > # 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.