On May 24, 2022, Martin Liška <mli...@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>
