[6/8] fortran: use grep -F instead of fgrep

Message ID 3fe5664607c4e530fbb91048c2e363ddee917250.camel@xry111.site
State New
Headers
Series Stop using obsoleted egrep/fgrep |

Commit Message

Xi Ruoyao June 24, 2022, 7:06 a.m. UTC
  fgrep has been deprecated in favor of grep -F for a long time, and the
next grep release (3.8 or 4.0) will print a warning of fgrep is used.
Stop using fgrep so we won't see the warning.

gcc/ChangeLog:

	* fortran/Make-lang.in: Use grep -F instead of fgrep.
---
 gcc/fortran/Make-lang.in | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)
  

Comments

Bernhard Reutner-Fischer June 24, 2022, 11:13 a.m. UTC | #1
On Fri, 24 Jun 2022 15:06:32 +0800
Xi Ruoyao via Gcc-patches <gcc-patches@gcc.gnu.org> wrote:

> fgrep has been deprecated in favor of grep -F for a long time, and the
> next grep release (3.8 or 4.0) will print a warning of fgrep is used.
> Stop using fgrep so we won't see the warning.
> 
> gcc/ChangeLog:
> 
> 	* fortran/Make-lang.in: Use grep -F instead of fgrep.
> ---
>  gcc/fortran/Make-lang.in | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/gcc/fortran/Make-lang.in b/gcc/fortran/Make-lang.in
> index 1cb47cb1a52..51279b03aad 100644
> --- a/gcc/fortran/Make-lang.in
> +++ b/gcc/fortran/Make-lang.in
> @@ -278,7 +278,7 @@ $(DESTDIR)$(man1dir)/$(GFORTRAN_INSTALL_NAME)$(man1ext): doc/gfortran.1 \
>  	-chmod a-x $@
>  
>  fortran.uninstall:
> -	if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v -i debian' >/dev/null 2>&1; then \
> +	if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v -i debian' >/dev/null 2>&1; then \
>  	  echo " install-info --delete --info-dir=$(DESTDIR)$(infodir) $(DESTDIR)$(infodir)/gfortran.info"; \
>  	  install-info --delete --info-dir=$(DESTDIR)$(infodir) $(DESTDIR)$(infodir)/gfortran.info || : ; \
>  	else : ; fi; \

I'd replace -s >/dev/null 2>&1 with -q while at it.

But why is -F used here in the first place?
I do not see much in debian that can be interpreted as a regex?

thanks,
  
Xi Ruoyao June 24, 2022, 12:22 p.m. UTC | #2
On Fri, 2022-06-24 at 13:13 +0200, Bernhard Reutner-Fischer wrote:

> > -       if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v -i debian' >/dev/null 2>&1; then \
> > +       if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v -i debian' >/dev/null 2>&1; then \
> >           echo " install-info --delete --info-dir=$(DESTDIR)$(infodir) $(DESTDIR)$(infodir)/gfortran.info"; \
> >           install-info --delete --info-dir=$(DESTDIR)$(infodir) $(DESTDIR)$(infodir)/gfortran.info || : ; \
> >         else : ; fi; \
> 
> I'd replace -s >/dev/null 2>&1 with -q while at it.
> 
> But why is -F used here in the first place?
> I do not see much in debian that can be interpreted as a regex?

I'm not sure.  It was there since 2004.  Perhaps the author thinks fgrep
may save several CPU cycles :).  I'll just use a plain grep in PATCH v2.

Rainer: do you have some idea about the availability of "-q" on
different hosts?  If you agree I'll use it instead of -s > /dev/null
too.
  
Rainer Orth June 24, 2022, 12:35 p.m. UTC | #3
Hi Xi,

> On Fri, 2022-06-24 at 13:13 +0200, Bernhard Reutner-Fischer wrote:
>
>> > -       if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v
>> > -i debian' >/dev/null 2>&1; then \
>> > +       if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v
>> > -i debian' >/dev/null 2>&1; then \
>> >           echo " install-info --delete --info-dir=$(DESTDIR)$(infodir)
>> > $(DESTDIR)$(infodir)/gfortran.info"; \
>> >           install-info --delete --info-dir=$(DESTDIR)$(infodir)
>> > $(DESTDIR)$(infodir)/gfortran.info || : ; \
>> >         else : ; fi; \
>> 
>> I'd replace -s >/dev/null 2>&1 with -q while at it.
>> 
>> But why is -F used here in the first place?
>> I do not see much in debian that can be interpreted as a regex?
>
> I'm not sure.  It was there since 2004.  Perhaps the author thinks fgrep
> may save several CPU cycles :).  I'll just use a plain grep in PATCH v2.
>
> Rainer: do you have some idea about the availability of "-q" on
> different hosts?  If you agree I'll use it instead of -s > /dev/null
> too.

again, the autoconf manual warns agains it, even more so against -s.
That's the first reference for portability issues and shouldn't be
ignored without good reason.

In the GCC and Solaris context, /bin/grep supports both -q and -s in
Solaris 11.3 and 11.4.  It doesn't support -q on Solaris 10, though
(again, no longer relevant for GCC).

	Rainer
  
Thomas Koenig June 24, 2022, 1:58 p.m. UTC | #4
Hi,

> On Fri, 2022-06-24 at 13:13 +0200, Bernhard Reutner-Fischer wrote:
> 
>>> -       if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v -i debian' >/dev/null 2>&1; then \
>>> +       if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v -i debian' >/dev/null 2>&1; then \
>>>            echo " install-info --delete --info-dir=$(DESTDIR)$(infodir) $(DESTDIR)$(infodir)/gfortran.info"; \
>>>            install-info --delete --info-dir=$(DESTDIR)$(infodir) $(DESTDIR)$(infodir)/gfortran.info || : ; \
>>>          else : ; fi; \
>>
>> I'd replace -s >/dev/null 2>&1 with -q while at it.
>>
>> But why is -F used here in the first place?
>> I do not see much in debian that can be interpreted as a regex?
> 
> I'm not sure.  It was there since 2004.  Perhaps the author thinks fgrep
> may save several CPU cycles :).  I'll just use a plain grep in PATCH v2.
> 
> Rainer: do you have some idea about the availability of "-q" on
> different hosts?  If you agree I'll use it instead of -s > /dev/null
> too.

Is there any particular reason why we should not use the result of
AC_PROG_FGREP ?  AC_PROG_GREP should also work.
  
Bernhard Reutner-Fischer June 24, 2022, 6:30 p.m. UTC | #5
On 24 June 2022 14:35:20 CEST, Rainer Orth <ro@CeBiTec.Uni-Bielefeld.DE> wrote:
>Hi Xi,
>
>> On Fri, 2022-06-24 at 13:13 +0200, Bernhard Reutner-Fischer wrote:
>>
>>> > -       if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v
>>> > -i debian' >/dev/null 2>&1; then \
>>> > +       if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v
>>> > -i debian' >/dev/null 2>&1; then \
>>> >           echo " install-info --delete --info-dir=$(DESTDIR)$(infodir)
>>> > $(DESTDIR)$(infodir)/gfortran.info"; \
>>> >           install-info --delete --info-dir=$(DESTDIR)$(infodir)
>>> > $(DESTDIR)$(infodir)/gfortran.info || : ; \
>>> >         else : ; fi; \
>>> 
>>> I'd replace -s >/dev/null 2>&1 with -q while at it.
>>> 
>>> But why is -F used here in the first place?
>>> I do not see much in debian that can be interpreted as a regex?
>>
>> I'm not sure.  It was there since 2004.  Perhaps the author thinks fgrep
>> may save several CPU cycles :).  I'll just use a plain grep in PATCH v2.
>>
>> Rainer: do you have some idea about the availability of "-q" on
>> different hosts?  If you agree I'll use it instead of -s > /dev/null
>> too.
>
>again, the autoconf manual warns agains it, even more so against -s.
>That's the first reference for portability issues and shouldn't be
>ignored without good reason.
>
>In the GCC and Solaris context, /bin/grep supports both -q and -s in
>Solaris 11.3 and 11.4.  It doesn't support -q on Solaris 10, though
>(again, no longer relevant for GCC).

The good reason I would bring forward is that the systems mentioned in the autoconf docs are all not supported anymore (IRIX, ancient or old Solaris etc).
Furthermore, grep(1) is required by POSIX to support -q since at least 1997; see https://pubs.opengroup.org/onlinepubs/007908799/xcu/grep.html

That's about 25 years now, so everybody had plenty of time to implement this specific part, which is even trivial to implement for this particular case of grep -q.

All in all i think that we should not be held hostage by such systems any longer, at least for such cases that are trivial to fix to conformance. Of course that may be just /me.

Iff fixing egrep or fgrep occurrences though,  we should use plain grep where applicable, like in this case, IMO.

thanks,
  

Patch

diff --git a/gcc/fortran/Make-lang.in b/gcc/fortran/Make-lang.in
index 1cb47cb1a52..51279b03aad 100644
--- a/gcc/fortran/Make-lang.in
+++ b/gcc/fortran/Make-lang.in
@@ -278,7 +278,7 @@  $(DESTDIR)$(man1dir)/$(GFORTRAN_INSTALL_NAME)$(man1ext): doc/gfortran.1 \
 	-chmod a-x $@
 
 fortran.uninstall:
-	if $(SHELL) -c 'install-info --version | sed 1q | fgrep -s -v -i debian' >/dev/null 2>&1; then \
+	if $(SHELL) -c 'install-info --version | sed 1q | grep -F -s -v -i debian' >/dev/null 2>&1; then \
 	  echo " install-info --delete --info-dir=$(DESTDIR)$(infodir) $(DESTDIR)$(infodir)/gfortran.info"; \
 	  install-info --delete --info-dir=$(DESTDIR)$(infodir) $(DESTDIR)$(infodir)/gfortran.info || : ; \
 	else : ; fi; \