Created attachment 394182 [details, diff] Patch to the eclass The fixincludes script from gcc is quite ugly in design. It wastes some time during build-time and confuses users like me who think 'this looks like a really bad idea'. Then I notice the gcc ebuild basically undoes the whole fixinclude thing by grepping for generated include files and removing them... So I'm attaching a patch that instead stubs out fixincludes. End result is the same but the script no longer wastes time and its output no longer confuses people. While at it, it's also safer than the ugly find/rm loop. Tested with 4.9.2. Running builds of older versions with the patch right now.
Created attachment 394192 [details, diff] Patch updated to handle gcc-3* The previous patch worked fine for all gcc-4.* slots but failed for gcc-3. Here's one that works fine with newest version in all in-tree 3.* and 4.* slots.
(In reply to Michał Górny from comment #1) > Created attachment 394192 [details, diff] [details, diff] > Patch updated to handle gcc-3* > > The previous patch worked fine for all gcc-4.* slots but failed for gcc-3. > Here's one that works fine with newest version in all in-tree 3.* and 4.* > slots. I compile tested 3.3 to 4.9 with no problems. For 4.9.2 I compared all .h's that would be put on the tree and they are identical. So this looks good to me. Post to gentoo-dev@ with the usual 2-3 day wait and then commit it.
Created attachment 394304 [details, diff] stub out fixed includes I've also tested and it seems you don't generate sym links in gcc/include* so we can get rid of that too.
Its in the tree, http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/toolchain.eclass?r1=1.647&r2=1.648
fixinclude is meant to sanitize headers for building GCC itself. You won't run into any problems on a standard glibc system, but I'd be surprised if this doesn't break BSD.
for example https://gcc.gnu.org/PR51705 / bug #444678
(In reply to Ryan Hill from comment #5) > fixinclude is meant to sanitize headers for building GCC itself. You won't > run into any problems on a standard glibc system, but I'd be surprised if > this doesn't break BSD. I wondered if you needed them for building gcc itself because you removed them after they were built, but i just couldn't convince myself looking at what was generated. I did consider BSD. Perhaps we should poke the bsd people to test and revert if it does.
The prefix guys would probably be affected as well, but I think they forked the eclass a long time ago. In any case I think this should be reverted. Most people probably don't need it but some might, and I don't like the idea that one bad header sneaking in could render GCC unbuildable for everybody. Potentially breaking systems outweighs saving a couple minutes of build time.
Like said in another place, we /need/ the fix-included headers, so please make it a switch we can easily enable.
(In reply to Ryan Hill from comment #8) > The prefix guys would probably be affected as well, but I think they forked > the eclass a long time ago. > > In any case I think this should be reverted. Most people probably don't > need it but some might, and I don't like the idea that one bad header > sneaking in could render GCC unbuildable for everybody. Potentially > breaking systems outweighs saving a couple minutes of build time. I agreee with not rendering gcc unbuildable. I'll email the list and revert. @Ryan, thanks for looking over my shoulders!
(In reply to Fabian Groffen from comment #9) > Like said in another place, we /need/ the fix-included headers, so please > make it a switch we can easily enable. Sorry dude. I will restore it. I don't want to exclude anyone. I don't have Ryan's memory why pieces were added to toolchain.eclass. I tested extensively but only in linux and non-prefix.
(In reply to Anthony Basile from comment #11) > Sorry dude. I will restore it. I don't want to exclude anyone. I don't > have Ryan's memory why pieces were added to toolchain.eclass. I tested > extensively but only in linux and non-prefix. No worries! Thanks!
(In reply to Fabian Groffen from comment #12) > (In reply to Anthony Basile from comment #11) > > Sorry dude. I will restore it. I don't want to exclude anyone. I don't > > have Ryan's memory why pieces were added to toolchain.eclass. I tested > > extensively but only in linux and non-prefix. > > No worries! Thanks! Reverted with a long comment on why we need to do things as we do to future proof against stupidity. Please take a look and let me know if there's still more to be done. http://sources.gentoo.org/cgi-bin/viewvc.cgi/gentoo-x86/eclass/toolchain.eclass?r1=1.648&r2=1.649 Not to detract too far from the original purpose of this bug, but maybe now we can merge the prefix toolchain.eclass into the tree. To be clear what's the official prefix overlay url?
Official prefix is here: http://rsync8.prefix.bitzolder.nl/hg/prefix-tree
For Linux, IMO the reasoning why we can remove the "fixed" headers is not that they break things (shouldn't that be fixed within gcc-fixincludes anyway?), but that we have full control over the original headers and prefer fixing these to get them work with gcc. For Prefix (and others), we don't have control over the original headers, so we need to use the "fixed" ones instead.
(In reply to Michael Haubenwallner from comment #15) > For Linux, IMO the reasoning why we can remove the "fixed" headers is not > that they break things (shouldn't that be fixed within gcc-fixincludes > anyway?), but that we have full control over the original headers and prefer > fixing these to get them work with gcc. Interesting perspective. The original point of fixincludes is that gcc requires ansi compliant headers to build and some unix vendors didn't provide them. Hence the hack. But I'm guessing that in linux a lot of those fixes got upstreamed. I was able to compile all versions of gcc in the tree (except 2.95) without the fixincludes on a system with 2000+ common packages. I even tested on my ubuntu box and didn't have any issues. > > For Prefix (and others), we don't have control over the original headers, so > we need to use the "fixed" ones instead. Right. I can see merit in having them if needed. I just didn't think of prefix.
(In reply to Anthony Basile from comment #16) > > > > For Prefix (and others), we don't have control over the original headers, so > > we need to use the "fixed" ones instead. > > Right. I can see merit in having them if needed. I just didn't think of > prefix. BTW, this is a good read by Felker, the author of musl, on why fixincludes are a bad idea: http://ewontfix.com/12/ It would be nice to get rid of the altogether,b ut as I said, if they're needed, they're needed.
(In reply to Guilherme Amadio from comment #14) > Official prefix is here: http://rsync8.prefix.bitzolder.nl/hg/prefix-tree Thanks I cloned and looked at it. I'm not sure what the ${D} -> ${ED} and ${EPREFIX} might break, like maybe cross compiling? We'd probably have to test in all cases where toolchains.eclass is consumed to make sure those updates are okay. I might have time to do that over the summer. I'm going to close this bug because the original issue was dealt with (not really fixed because it wasn't a problem in the first place).
Hi Anthony, (In reply to Anthony Basile from comment #18) > > Thanks I cloned and looked at it. I'm not sure what the ${D} -> ${ED} and > ${EPREFIX} might break, like maybe cross compiling? We'd probably have to > test in all cases where toolchains.eclass is consumed to make sure those > updates are okay. I might have time to do that over the summer. > > I'm going to close this bug because the original issue was dealt with (not > really fixed because it wasn't a problem in the first place). I would like to point you to bug 531610 for Prefix support, where I made a patch for your review. Benda
Upstream now have a proper --disable-fixincludes configure arg to disable it in GCC 13, see https://gcc.gnu.org/git/?p=gcc.git;a=commit;h=be9dd80f9334800300a80268dbb92cf3fafcfcf8. We're not using it yet because of: commit 6ce12231a50fd05970501f67adc916754a21fe60 Author: Sam James <sam@gentoo.org> Date: Tue Oct 4 02:23:32 2022 +0100 toolchain.eclass: allow fixincludes for >= GCC 13 Signed-off-by: Sam James <sam@gentoo.org> ... for https://gcc.gnu.org/bugzilla/show_bug.cgi?id=107128. But hopefully we can change that at some point.
