configure: add --disable-fix-includes

Hello.

It seems to me that fixincludes is hardy unused feature for nowadays header
files and so I'm suggesting a developer option that can skip the fixing.

How is the feature used on other targets?

Right now, fixinclude takes about 11 seconds on my machine, where
it reads (and applies regexes) 130MB of header files.

The number of fixed headers is negligible without any significant
change. I'm attaching diff that I can see on my developer machine.

Patch can bootstrap on x86_64-linux-gnu and survives regression tests.

Ready to be installed?
Thanks,
Martin

fixincludes/ChangeLog:

	* fixinc.in: Add early exit.

gcc/ChangeLog:

	* Makefile.in: Support disable_fix_includes.
	* configure.ac: Add --disable-fix-includes.
	* configure: Regenerate.
---
  fixincludes/fixinc.in |  6 ++++++
  gcc/Makefile.in       |  6 ++++--
  gcc/configure         | 21 +++++++++++++++++++--
  gcc/configure.ac      |  6 ++++++
  4 files changed, 35 insertions(+), 4 deletions(-)

Hi Martin,

> It seems to me that fixincludes is hardy unused feature for nowadays header
> files and so I'm suggesting a developer option that can skip the fixing.

please remember that there's a world beyond current-day Linux.

> How is the feature used on other targets?

There are still quite a number of fixes on e.g. Solaris or macOS.  And
people are still building gcc on older OS versions for one reason or
another...

> diff --git a/gcc/configure.ac b/gcc/configure.ac
> index 1171c946e6e..6015e403aa9 100644
> --- a/gcc/configure.ac
> +++ b/gcc/configure.ac
> @@ -842,6 +842,12 @@ gather_stats=`if test $enable_gather_detailed_mem_stats != no; then echo 1; else
>   AC_DEFINE_UNQUOTED(GATHER_STATISTICS, $gather_stats,
>   [Define to enable detailed memory allocation stats gathering.])
>   
> +AC_ARG_ENABLE(disable-fix-includes,
> +[AS_HELP_STRING([--disable-fix-includes],

The beast is called fixincludes, no '-' or '_'.

> +		[skip fixing of includes])], [],

Better say 'running fixincludes' for example rather than being
vague/obtuse.

The new options requires documenting in install.texi.

That said, I'm not sure this is really worth yet adding another option.
And how are developers supposed to know if they can safely use it or
not.

Besides, have you actually run a regtest with that option?  I'm asking
because even on Ubuntu 20.04 fixincludes drops in it's own <limits.h>.
You need to check that dropping that is actually safe.

	Rainer

On Fri, Feb 4, 2022 at 2:08 PM Rainer Orth <ro@cebitec.uni-bielefeld.de> wrote:
>
> Hi Martin,
>
> > It seems to me that fixincludes is hardy unused feature for nowadays header
> > files and so I'm suggesting a developer option that can skip the fixing.
>
> please remember that there's a world beyond current-day Linux.
>
> > How is the feature used on other targets?
>
> There are still quite a number of fixes on e.g. Solaris or macOS.  And
> people are still building gcc on older OS versions for one reason or
> another...
>
> > diff --git a/gcc/configure.ac b/gcc/configure.ac
> > index 1171c946e6e..6015e403aa9 100644
> > --- a/gcc/configure.ac
> > +++ b/gcc/configure.ac
> > @@ -842,6 +842,12 @@ gather_stats=`if test $enable_gather_detailed_mem_stats != no; then echo 1; else
> >   AC_DEFINE_UNQUOTED(GATHER_STATISTICS, $gather_stats,
> >   [Define to enable detailed memory allocation stats gathering.])
> >
> > +AC_ARG_ENABLE(disable-fix-includes,
> > +[AS_HELP_STRING([--disable-fix-includes],
>
> The beast is called fixincludes, no '-' or '_'.
>
> > +             [skip fixing of includes])], [],
>
> Better say 'running fixincludes' for example rather than being
> vague/obtuse.
>
> The new options requires documenting in install.texi.
>
> That said, I'm not sure this is really worth yet adding another option.
> And how are developers supposed to know if they can safely use it or
> not.
>
> Besides, have you actually run a regtest with that option?  I'm asking
> because even on Ubuntu 20.04 fixincludes drops in it's own <limits.h>.
> You need to check that dropping that is actually safe.

limits.h and syslimits.h are the only include-fixed we "ship" in our
packages.  Not sure if they are really required though.

Richard.

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

On Fri, Feb 04, 2022 at 02:25:17PM +0100, Richard Biener via Gcc-patches wrote:
> > Besides, have you actually run a regtest with that option?  I'm asking
> > because even on Ubuntu 20.04 fixincludes drops in it's own <limits.h>.
> > You need to check that dropping that is actually safe.
> 
> limits.h and syslimits.h are the only include-fixed we "ship" in our
> packages.  Not sure if they are really required though.

We don't ship any include-fixed headers in Fedora/RHEL.
If there is an important need for fixing, it should be in Linux
distributions fixed by actually fixing the headers in their corresponding
packages, making modified copies of headers at gcc build time and overriding
through that headers from other packages that can change doesn't really work
properly when those headers ever change.  I know we could install the
fixincludes tool and trigger running it whenever any of such headers change
in other packages, but that is very expensive for very little gain.

On other targets I understand fixincludes is much more important.

	Jakub

On 2/4/22 14:30, Jakub Jelinek via Gcc-patches wrote:
> On Fri, Feb 04, 2022 at 02:25:17PM +0100, Richard Biener via Gcc-patches wrote:
>>> Besides, have you actually run a regtest with that option?  I'm asking
>>> because even on Ubuntu 20.04 fixincludes drops in it's own <limits.h>.
>>> You need to check that dropping that is actually safe.
>>
>> limits.h and syslimits.h are the only include-fixed we "ship" in our
>> packages.  Not sure if they are really required though.
> 
> We don't ship any include-fixed headers in Fedora/RHEL.
> If there is an important need for fixing, it should be in Linux
> distributions fixed by actually fixing the headers in their corresponding
> packages, making modified copies of headers at gcc build time and overriding
> through that headers from other packages that can change doesn't really work
> properly when those headers ever change.  I know we could install the
> fixincludes tool and trigger running it whenever any of such headers change
> in other packages, but that is very expensive for very little gain.

I fully agree with the suggested approach! I've also noticed the script makes
modifications in comments (that are unnecessary), plus it effectively breaks
header files for things like:

$ diff /usr/lib64/gcc/aarch64-suse-linux/11/include-fixed/bits/unistd_ext.h /usr/aarch64-suse-linux/sys-root/usr/include/bits/unistd_ext.h

46c37
< # if __has_include ("__linux__/close_range.h")
---
> # if __has_include ("linux/close_range.h")

I would suggest removing the fixed includes in openSUSE as well?

Martin

> 
> On other targets I understand fixincludes is much more important.
> 
> 	Jakub
>

On 2/4/22 14:07, Rainer Orth wrote:
> Hi Martin,
> 
>> It seems to me that fixincludes is hardy unused feature for nowadays header
>> files and so I'm suggesting a developer option that can skip the fixing.
> 
> please remember that there's a world beyond current-day Linux.

Sure! That's why I'm suggesting an option and not removing the machinery.

> 
>> How is the feature used on other targets?
> 
> There are still quite a number of fixes on e.g. Solaris or macOS.  And
> people are still building gcc on older OS versions for one reason or
> another...
> 
>> diff --git a/gcc/configure.ac b/gcc/configure.ac
>> index 1171c946e6e..6015e403aa9 100644
>> --- a/gcc/configure.ac
>> +++ b/gcc/configure.ac
>> @@ -842,6 +842,12 @@ gather_stats=`if test $enable_gather_detailed_mem_stats != no; then echo 1; else
>>    AC_DEFINE_UNQUOTED(GATHER_STATISTICS, $gather_stats,
>>    [Define to enable detailed memory allocation stats gathering.])
>>    
>> +AC_ARG_ENABLE(disable-fix-includes,
>> +[AS_HELP_STRING([--disable-fix-includes],
> 
> The beast is called fixincludes, no '-' or '_'.
> 
>> +		[skip fixing of includes])], [],
> 
> Better say 'running fixincludes' for example rather than being
> vague/obtuse.

Note using --disable-fixinclude effectively disables building of everything in the folder.
That's not what I want, it breaks GCC build.

> 
> The new options requires documenting in install.texi.
> 
> That said, I'm not sure this is really worth yet adding another option.
> And how are developers supposed to know if they can safely use it or
> not.

Sure.

> 
> Besides, have you actually run a regtest with that option?  I'm asking
> because even on Ubuntu 20.04 fixincludes drops in it's own <limits.h>.
> You need to check that dropping that is actually safe.

Let me try that.

Martin

> 
> 	Rainer
>

On 2/4/22 14:30, Jakub Jelinek via Gcc-patches wrote:
> We don't ship any include-fixed headers in Fedora/RHEL.

Removing include-fixed from an installed folder, I see:

make[2]: Entering directory '/home/marxin/Programming/postgres/src/common'
gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O3 -march=native -flto=auto -DFRONTEND -I. -I../../src/common -I../../src/include  -D_GNU_SOURCE  -DVAL_CC="\"gcc\"" -DVAL_CPPFLAGS="\"-D_GNU_SOURCE\"" -DVAL_CFLAGS="\"-Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O3 -march=native -flto=auto\"" -DVAL_CFLAGS_SL="\"-fPIC\"" -DVAL_LDFLAGS="\"-O3 -march=native -flto=auto -Wl,--as-needed -Wl,-rpath,'/usr/local/pgsql/lib64',--enable-new-dtags\"" -DVAL_LDFLAGS_EX="\"\"" -DVAL_LDFLAGS_SL="\"\"" -DVAL_LIBS="\"-lpgcommon -lpgport -lz -lreadline -lm \""  -c -o pg_lzcompress.o pg_lzcompress.c
In file included from pg_lzcompress.c:186:
/usr/include/limits.h:124:26: error: no include path in which to search for limits.h
   124 | # include_next <limits.h>
       |                          ^
pg_lzcompress.c:226:9: error: ‘INT_MAX’ undeclared here (not in a function)
   226 |         INT_MAX,                                        /* No upper limit on what we'll try to
       |         ^~~~~~~
pg_lzcompress.c:189:1: note: ‘INT_MAX’ is defined in header ‘<limits.h>’; did you forget to ‘#include <limits.h>’?
   188 | #include "common/pg_lzcompress.h"
   +++ |+#include <limits.h>

How do you solve this in Fedora/RHEL?

Thanks,
Martin

On 5/2/22 01:22, Martin Liška wrote:
> On 2/4/22 14:30, Jakub Jelinek via Gcc-patches wrote:
>> We don't ship any include-fixed headers in Fedora/RHEL.
> 
> Removing include-fixed from an installed folder, I see:
> 
> make[2]: Entering directory '/home/marxin/Programming/postgres/src/common'
> gcc -Wall -Wmissing-prototypes -Wpointer-arith 
> -Wdeclaration-after-statement -Werror=vla -Wendif-labels 
> -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type 
> -Wformat-security -fno-strict-aliasing -fwrapv 
> -fexcess-precision=standard -Wno-format-truncation 
> -Wno-stringop-truncation -O3 -march=native -flto=auto -DFRONTEND -I. 
> -I../../src/common -I../../src/include  -D_GNU_SOURCE  
> -DVAL_CC="\"gcc\"" -DVAL_CPPFLAGS="\"-D_GNU_SOURCE\"" 
> -DVAL_CFLAGS="\"-Wall -Wmissing-prototypes -Wpointer-arith 
> -Wdeclaration-after-statement -Werror=vla -Wendif-labels 
> -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type 
> -Wformat-security -fno-strict-aliasing -fwrapv 
> -fexcess-precision=standard -Wno-format-truncation 
> -Wno-stringop-truncation -O3 -march=native -flto=auto\"" 
> -DVAL_CFLAGS_SL="\"-fPIC\"" -DVAL_LDFLAGS="\"-O3 -march=native 
> -flto=auto -Wl,--as-needed 
> -Wl,-rpath,'/usr/local/pgsql/lib64',--enable-new-dtags\"" 
> -DVAL_LDFLAGS_EX="\"\"" -DVAL_LDFLAGS_SL="\"\"" -DVAL_LIBS="\"-lpgcommon 
> -lpgport -lz -lreadline -lm \""  -c -o pg_lzcompress.o pg_lzcompress.c
> In file included from pg_lzcompress.c:186:
> /usr/include/limits.h:124:26: error: no include path in which to search 
> for limits.h
>    124 | # include_next <limits.h>
>        |                          ^
> pg_lzcompress.c:226:9: error: ‘INT_MAX’ undeclared here (not in a function)
>    226 |         INT_MAX,                                        /* No 
> upper limit on what we'll try to
>        |         ^~~~~~~
> pg_lzcompress.c:189:1: note: ‘INT_MAX’ is defined in header 
> ‘<limits.h>’; did you forget to ‘#include <limits.h>’?
>    188 | #include "common/pg_lzcompress.h"
>    +++ |+#include <limits.h>
> 
> How do you solve this in Fedora/RHEL?

The Fedora gcc.spec file has this:

mv $FULLPATH/include-fixed/syslimits.h $FULLPATH/include/syslimits.h
mv $FULLPATH/include-fixed/limits.h $FULLPATH/include/limits.h

My understanding are these are not real fixinclude processed headers.

Allan

On 2/5/22 03:26, Allan McRae wrote:
> On 5/2/22 01:22, Martin Liška wrote:
>> On 2/4/22 14:30, Jakub Jelinek via Gcc-patches wrote:
>>> We don't ship any include-fixed headers in Fedora/RHEL.
>>
>> Removing include-fixed from an installed folder, I see:
>>
>> make[2]: Entering directory '/home/marxin/Programming/postgres/src/common'
>> gcc -Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O3 -march=native -flto=auto -DFRONTEND -I. -I../../src/common -I../../src/include  -D_GNU_SOURCE -DVAL_CC="\"gcc\"" -DVAL_CPPFLAGS="\"-D_GNU_SOURCE\"" -DVAL_CFLAGS="\"-Wall -Wmissing-prototypes -Wpointer-arith -Wdeclaration-after-statement -Werror=vla -Wendif-labels -Wmissing-format-attribute -Wimplicit-fallthrough=3 -Wcast-function-type -Wformat-security -fno-strict-aliasing -fwrapv -fexcess-precision=standard -Wno-format-truncation -Wno-stringop-truncation -O3 -march=native -flto=auto\"" -DVAL_CFLAGS_SL="\"-fPIC\"" -DVAL_LDFLAGS="\"-O3 -march=native -flto=auto -Wl,--as-needed -Wl,-rpath,'/usr/local/pgsql/lib64',--enable-new-dtags\"" -DVAL_LDFLAGS_EX="\"\"" 
>> -DVAL_LDFLAGS_SL="\"\"" -DVAL_LIBS="\"-lpgcommon -lpgport -lz -lreadline -lm \""  -c -o pg_lzcompress.o pg_lzcompress.c
>> In file included from pg_lzcompress.c:186:
>> /usr/include/limits.h:124:26: error: no include path in which to search for limits.h
>>    124 | # include_next <limits.h>
>>        |                          ^
>> pg_lzcompress.c:226:9: error: ‘INT_MAX’ undeclared here (not in a function)
>>    226 |         INT_MAX,                                        /* No upper limit on what we'll try to
>>        |         ^~~~~~~
>> pg_lzcompress.c:189:1: note: ‘INT_MAX’ is defined in header ‘<limits.h>’; did you forget to ‘#include <limits.h>’?
>>    188 | #include "common/pg_lzcompress.h"
>>    +++ |+#include <limits.h>
>>
>> How do you solve this in Fedora/RHEL?
> 
> The Fedora gcc.spec file has this:
> 
> mv $FULLPATH/include-fixed/syslimits.h $FULLPATH/include/syslimits.h
> mv $FULLPATH/include-fixed/limits.h $FULLPATH/include/limits.h

Yes, I noticed that as well.

> 
> My understanding are these are not real fixinclude processed headers.

You are correct. I've just prepared a patch that would exclude these 2 header files
from include-fixed. I'm planning the patch for next stage1.

Martin

> 
> Allan