Re: [PATCH v2] Support --disable-fixincludes.

[Alexandre Oliva <oliva@gnu.org>]

On May 24, 2022, Martin Liška <mliska@suse.cz> wrote:

> Allways install limits.h and syslimits.h header files
> to include folder.

typo: s/Allways/Always/

I'm a little worried about this bit, too.  limitx.h includes
"syslimits.h", mentioning it should be in the same directory.  Perhaps
it could be left in include-fixed?

The patch also changes syslimits.h from either the fixincluded file or
gsyslimits.h to use gsyslimits.h unconditionally, which seemed wrong at
first.

Now I see how these two hunks work together: syslimits.h will now always
#include_next <limits.h>, which will find it in include-fixed if it's
there, and system header files otherwise.  Nice!, but IMHO the commit
message could be a little more verbose on the extent of the change and
why that (is supposed to) work.


It also looks like install-mkheaders installs limits-related headers for
when fixincludes runs; we could probably skip the whole thing if
fixincludes is disabled, but I'm also worried about how the changes
above might impact post-install fixincludes: if that installs
gsyslimits.h et al in include-fixed while your changes moves it to
include, headers might end up in a confusing state.  I haven't worked
out whether that's safe, but there appears to be room for cleanups
there.

gcc/config/mips/t-sdemtk also places syslimits.h explicitly in include/
rather than include-fixed/, as part of disabling fixincludes, which is
good, but it could be cleaned up as well.

I don't see other config fragments that might require adjustments, so I
think the patch looks good; hopefully my worries are unjustified, and
the cleanups it enables could be


We still create the README file in there and install it, even with
fixincludes disabled and thus unavailable, don't we?  That README then
becomes misleading; we might be better off not installing it.


> When --disable-fixincludes is used, then no systen header files
> are fixed by the tools in fixincludes. Moreover, the fixincludes
> tools are not built any longer.

typo: s/systen/system/


Could you please check that a post-install mkheaders still has a
functional limits.h with these changes?  The patch is ok (with the typo
fixes) if so.  The cleanups it enables would be welcome as separate
patches ;-)

Thanks!

-- 
Alexandre Oliva, happy hacker                https://FSFLA.org/blogs/lxo/
   Free Software Activist                       GNU Toolchain Engineer
Disinformation flourishes because many people care deeply about injustice
but very few check the facts.  Ask me about <https://stallmansupport.org>