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

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

[Bernhard Reutner-Fischer]

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,

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

[Xi Ruoyao]

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.
-- 
Xi Ruoyao <xry111@xry111.site>
School of Aerospace Science and Technology, Xidian University

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

[Rainer Orth]

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

-- 
-----------------------------------------------------------------------------
Rainer Orth, Center for Biotechnology, Bielefeld University

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

[Thomas Koenig]

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.

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

[Bernhard Reutner-Fischer]

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,