On 5/25/22 07:37, Alexandre Oliva wrote:
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/
Hello.
Fixed.
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?
Well, I would like to go w/o include-fixed if the option --disable-fixincludes
is used.
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.
Oh, to be honest I'm not fully familiar with how these 2 files work together.
Can you explain it to me so that I can adjust the changelog entry
correspondingly?
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.
I've check that 'make install-mkheaders' work fine w/ and w/o
--disable-fixincludes
after the patch.
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.
Can we do that as a follow-up patch?
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
Good.
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.
Sure, fixed in v2 of the patch.
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/
Fixed.
Could you please check that a post-install mkheaders still has a
functional limits.h with these changes?
How do I check that, please?
The patch is ok (with the typo
fixes) if so. The cleanups it enables would be welcome as separate
patches ;-)
Can I install the v2?
