[v2] Support --disable-fixincludes.
Commit Message
On 5/20/22 14:42, Alexandre Oliva wrote:
> On May 11, 2022, Martin Liška <mliska@suse.cz> wrote:
>
>> Ready to be installed?
>
> Hmm... I don't like that --disable-fixincludes would still configure,
> build and even install fixincludes. This would be surprising, given
> that the semantics of disabling a component is to not even configure it.
>
> How about leaving the top-level alone, and changing gcc/configure.ac to
> clear STMP_FIXINC when --disable-fixincludes is given?
>
Sure, that's a good idea.
Allways install limits.h and syslimits.h header files
to include folder.
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.
Patch can bootstrap on x86_64-linux-gnu and survives regression tests.
Ready to be installed?
Thanks,
Martin
Comments
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!
On 5/25/22 07:37, Alexandre Oliva wrote:
> 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/
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?
Martin
>
> Thanks!
>
On 7/8/2022 5:14 AM, Martin Liška wrote:
> On 5/25/22 07:37, Alexandre Oliva wrote:
>> 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/
> 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?
Once Alex is OK with this patch, then it'll be good to go.
jeff
On Sat, 2022-07-09 at 10:11 -0600, Jeff Law via Gcc-patches wrote:
> Once Alex is OK with this patch, then it'll be good to go.
>
> jeff
Gentle ping as a distro maintainer :).
On Aug 31, 2022, Xi Ruoyao <xry111@xry111.site> wrote:
> On Sat, 2022-07-09 at 10:11 -0600, Jeff Law via Gcc-patches wrote:
>> Once Alex is OK with this patch, then it'll be good to go.
>>
>> jeff
> Gentle ping as a distro maintainer :).
Oops, thanks, sorry, I seem to have missed it the first time around.
The patch looks good to me, thanks Martin,
From ba9bed4512d73d34d4c9bf5830e758097d517bc3 Mon Sep 17 00:00:00 2001
From: Martin Liska <mliska@suse.cz>
Date: Tue, 24 May 2022 13:06:07 +0200
Subject: [PATCH] Support --disable-fixincludes.
Allways install limits.h and syslimits.h header files
to include folder.
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.
gcc/ChangeLog:
* Makefile.in: Always install limits.h and syslimits.h to
include folder.
* configure.ac: Assign STMP_FIXINC blank if
--disable-fixincludes is used.
* configure: Regenerate.
---
gcc/Makefile.in | 22 ++++++++--------------
gcc/configure | 10 ++++++++--
gcc/configure.ac | 6 ++++++
3 files changed, 22 insertions(+), 16 deletions(-)
@@ -3153,19 +3153,20 @@ stmp-int-hdrs: $(STMP_FIXINC) $(T_GLIMITS_H) $(T_STDINT_GCC_H) $(USER_H) fixinc_
set -e; for ml in `cat fixinc_list`; do \
sysroot_headers_suffix=`echo $${ml} | sed -e 's/;.*$$//'`; \
multi_dir=`echo $${ml} | sed -e 's/^[^;]*;//'`; \
- fix_dir=include-fixed$${multi_dir}; \
+ include_dir=include$${multi_dir}; \
if $(LIMITS_H_TEST) ; then \
cat $(srcdir)/limitx.h $(T_GLIMITS_H) $(srcdir)/limity.h > tmp-xlimits.h; \
else \
cat $(T_GLIMITS_H) > tmp-xlimits.h; \
fi; \
- $(mkinstalldirs) $${fix_dir}; \
- chmod a+rx $${fix_dir} || true; \
+ $(mkinstalldirs) $${include_dir}; \
+ chmod a+rx $${include_dir} || true; \
$(SHELL) $(srcdir)/../move-if-change \
tmp-xlimits.h tmp-limits.h; \
- rm -f $${fix_dir}/limits.h; \
- cp -p tmp-limits.h $${fix_dir}/limits.h; \
- chmod a+r $${fix_dir}/limits.h; \
+ rm -f $${include_dir}/limits.h; \
+ cp -p tmp-limits.h $${include_dir}/limits.h; \
+ chmod a+r $${include_dir}/limits.h; \
+ cp $(srcdir)/gsyslimits.h $${include_dir}/syslimits.h; \
done
# Install the README
rm -f include-fixed/README
@@ -3255,13 +3256,6 @@ stmp-fixinc: gsyslimits.h macro_list fixinc_list \
cd $(build_objdir)/fixincludes && \
$(SHELL) ./fixinc.sh "$${gcc_dir}/$${fix_dir}" \
$(BUILD_SYSTEM_HEADER_DIR) $(OTHER_FIXINCLUDES_DIRS) ); \
- rm -f $${fix_dir}/syslimits.h; \
- if [ -f $${fix_dir}/limits.h ]; then \
- mv $${fix_dir}/limits.h $${fix_dir}/syslimits.h; \
- else \
- cp $(srcdir)/gsyslimits.h $${fix_dir}/syslimits.h; \
- fi; \
- chmod a+r $${fix_dir}/syslimits.h; \
done; \
fi
$(STAMP) stmp-fixinc
@@ -3979,7 +3973,7 @@ install-mkheaders: stmp-int-hdrs install-itoolsdirs \
set -e; for ml in `cat fixinc_list`; do \
multi_dir=`echo $${ml} | sed -e 's/^[^;]*;//'`; \
$(mkinstalldirs) $(DESTDIR)$(itoolsdatadir)/include$${multi_dir}; \
- $(INSTALL_DATA) include-fixed$${multi_dir}/limits.h $(DESTDIR)$(itoolsdatadir)/include$${multi_dir}/limits.h; \
+ $(INSTALL_DATA) include$${multi_dir}/limits.h $(DESTDIR)$(itoolsdatadir)/include$${multi_dir}/limits.h; \
done
$(INSTALL_SCRIPT) $(srcdir)/../mkinstalldirs \
$(DESTDIR)$(itoolsdir)/mkinstalldirs ; \
@@ -13548,6 +13548,12 @@ then
BUILD_LDFLAGS='$(LDFLAGS_FOR_BUILD)'
fi
+
+if test x$enable_fixincludes = xno;
+then
+ STMP_FIXINC=''
+fi
+
# Expand extra_headers to include complete path.
# This substitutes for lots of t-* files.
extra_headers_list=
@@ -19674,7 +19680,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 19676 "configure"
+#line 19683 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -19780,7 +19786,7 @@ else
lt_dlunknown=0; lt_dlno_uscore=1; lt_dlneed_uscore=2
lt_status=$lt_dlunknown
cat > conftest.$ac_ext <<_LT_EOF
-#line 19782 "configure"
+#line 19789 "configure"
#include "confdefs.h"
#if HAVE_DLFCN_H
@@ -2501,6 +2501,12 @@ then
BUILD_LDFLAGS='$(LDFLAGS_FOR_BUILD)'
fi
+
+if test x$enable_fixincludes = xno;
+then
+ STMP_FIXINC=''
+fi
+
# Expand extra_headers to include complete path.
# This substitutes for lots of t-* files.
extra_headers_list=
--
2.36.1