| Summary: | [TRACKER] media-libs/libsdl multilib-wrapped headers breakage | ||
|---|---|---|---|
| Product: | Gentoo Linux | Reporter: | Julian Ospald <hasufell> |
| Component: | Eclasses | Assignee: | Multilib team <multilib+disabled> |
| Status: | RESOLVED FIXED | ||
| Severity: | normal | CC: | games, qa, ssuominen, zima |
| Priority: | Normal | Keywords: | Tracker |
| Version: | unspecified | ||
| Hardware: | All | ||
| OS: | Linux | ||
| Whiteboard: | |||
| Package list: | Runtime testing required: | --- | |
| Bug Depends on: | 510276, 510474, 512914, 513974, 514238, 514240, 514242, 514302, 514316, 514358, 514368, 514426 | ||
| Bug Blocks: | |||
| Attachments: | liquidwar6-0.4.3681-SDL.patch | ||
|
Description
Julian Ospald
2014-06-04 16:38:54 UTC
I thought we already agreed that packages are expected to use pkg-config to grab flags for SDL, and anything that doesn't do that is broken. (In reply to Michał Górny from comment #1) > I thought we already agreed that packages are expected to use pkg-config to > grab flags for SDL, and anything that doesn't do that is broken. definitely not most definately yes, anything not using pkg-config is broken (In reply to Samuli Suominen from comment #3) > most definately yes, anything not using pkg-config is broken This is totally unrelated to pkgconfig. Check configure.ac. Pkgconfig can in no way check for specific headers. Created attachment 378300 [details, diff]
liquidwar6-0.4.3681-SDL.patch
patch works around it.
(In reply to Julian Ospald (hasufell) from comment #4) > (In reply to Samuli Suominen from comment #3) > > most definately yes, anything not using pkg-config is broken > > This is totally unrelated to pkgconfig. Check configure.ac. SDL upstream requires you to use pkg-config. If you try to compile any single file without respecting CFLAGS & LIBS from pkg-config -- no matter if it's actual source code or configure test -- your package is broken. > Pkgconfig can in no way check for specific headers. But pkg-config can provide you with proper CFLAGS & LIBS that can be used to perform those checks reliably. (In reply to Michał Górny from comment #6) > (In reply to Julian Ospald (hasufell) from comment #4) > > (In reply to Samuli Suominen from comment #3) > > > most definately yes, anything not using pkg-config is broken > > > > This is totally unrelated to pkgconfig. Check configure.ac. > > SDL upstream requires you to use pkg-config. If you try to compile any > single file without respecting CFLAGS & LIBS from pkg-config -- no matter if > it's actual source code or configure test -- your package is broken. > > > Pkgconfig can in no way check for specific headers. > > But pkg-config can provide you with proper CFLAGS & LIBS that can be used to > perform those checks reliably. Pkgconfig is an upstream interface (not the only one though... sdl-config is perfectly supported as well, which the package in question makes use of). However, there are tons of build systems that do not use it. You call them all broken, just because our multilib design depends on it? We messed up. Don't make poor excuses. This will bite us again in other ways. We shouldn't try to add more hackery to gentoo, but remove some of it. Sure, I can fix it on package level. This bug was not about "how do I fix my package". It's still proof that WE did something FRAGILE. (In reply to Julian Ospald (hasufell) from comment #7) > Pkgconfig is an upstream interface (not the only one though... sdl-config is > perfectly supported as well, which the package in question makes use of). What's the point you're trying to make? sdl-config provides correct CFLAGS as well. > However, there are tons of build systems that do not use it. You call them > all broken, just because our multilib design depends on it? SDL upstream calls them broken. I've given SDL maintainers the options, they decided it's not our issue to fix if people are using SDL in an unsupported way. If you have problem with that, bug the games team to change their decision. > This will bite us again in other ways. We shouldn't try to add more hackery > to gentoo, but remove some of it. Sure. Because wrapping headers that: 1. use relative include paths rather than top-of-includedir, 2. and include non-wrapped headers is very common. It's just great luck that only SDL does that of all our packages. > Sure, I can fix it on package level. This bug was not about "how do I fix my > package". It's still proof that WE did something FRAGILE. And we know that for some time already. So far there's been no good solution suggested and you don't seem to have one. (In reply to Michał Górny from comment #8) > And we know that for some time already. So far there's been no good solution > suggested and you don't seem to have one. It should not have happened in the first place. We are doing too much of these feature-additions that assume that upstreams do it "right" lately. It almost sounds like we expect upstream developers to code against random gentoo eclasses. Anyway, can we agree then that this bug is valid and we need to improve the wrapped header situation? Yes, I currently don't have a solution either, but let's not just resign. Your initial response sounded more like "I don't care" and not like "I care, but don't know a solution yet". (In reply to Julian Ospald (hasufell) from comment #9) > (In reply to Michał Górny from comment #8) > > And we know that for some time already. So far there's been no good solution > > suggested and you don't seem to have one. > > It should not have happened in the first place. We are doing too much of > these feature-additions that assume that upstreams do it "right" lately. It > almost sounds like we expect upstream developers to code against random > gentoo eclasses. > > Anyway, can we agree then that this bug is valid and we need to improve the > wrapped header situation? Yes, I currently don't have a solution either, but > let's not just resign. Your initial response sounded more like "I don't > care" and not like "I care, but don't know a solution yet". As a last resort, couldn't we wrap the header in place, meaning putting the actual original content inside the #ifdef's and create a X-times bigger monster header. (In reply to Julian Ospald (hasufell) from comment #9) > (In reply to Michał Górny from comment #8) > > And we know that for some time already. So far there's been no good solution > > suggested and you don't seem to have one. > > It should not have happened in the first place. We are doing too much of > these feature-additions that assume that upstreams do it "right" lately. It > almost sounds like we expect upstream developers to code against random > gentoo eclasses. Well, as you may remember I was pessimistic about header wrapping from day one, and strong supporters of 'well-tested solutions used in multilib portage' failed to mention the few known issues > Anyway, can we agree then that this bug is valid and we need to improve the > wrapped header situation? Yes, I currently don't have a solution either, but > let's not just resign. Your initial response sounded more like "I don't > care" and not like "I care, but don't know a solution yet". We may improve the situation if there is a good solution for it. And a good solution that won't risk larger breakage. But it's rare issue that is only triggered by very bad design of SDL, and people going against that design (like doing #include <SDL/SDL.h> instead of #include <SDL.h>). (In reply to Christoph Junghans from comment #10) > As a last resort, couldn't we wrap the header in place, meaning putting the > actual original content inside the #ifdef's and create a X-times bigger > monster header. This is going to make fixing the swig issues a lot harder, and I believe they are much more common and more important (look at number of people CC-ed on the m2crypto bug). Hello guys, I have just reported related bug #512914. And I indeed think that the header wrapping is not a good solution. I would suggest the other way around: install by _default_ to /usr/include/<arch>/ and globally accomodate the include search path (is it in gcc specs?). And maybe symlink some headers back to /usr/include for builds which hardcode the path. Please take it just as a lame idea from an experienced C/C++ programmer ;) I have no idea what other problems with multilib you are solving and do not want to bother anyone. I also guess that my fix in bug #512914 is not an ideal one, but it works for me at the moment. I do not have enough time to dive into the frozen-bubble build system and chcek if it uses pkg-config. I am really inclined to revert the multilib migration of sdl. What's the fallout of such a step? I'd say: * restore old emul-linux package * revbump sdl related ebuilds and rm multilib support * fix reverse deps if any Since portage is severly broken, I am not sure what will happen. Our depstrings are already confusing for the human reader. I agree the current state of having a subset of headers wrapped is broken, but IMHO the correct solution is multilib without header wrapping (i.e. with all headers installed to /usr/include/<abi>, as suggested in comment #12), and packages which break because they try to use SDL in a non-sanctioned way should be fixed either upstream or with a local patch -- just like Gentoo does for any other buggy package. The official recommendation is: - Use sdl-config or sdl2-config to get the proper flags (https://wiki.libsdl.org/FAQLinux#How_do_I_add_SDL_to_my_project.3F) - Use [#include "SDL.h"] (https://wiki.libsdl.org/FAQDevelopment#Do_I_.23include_.3CSDL.h.3E_or_.3CSDL.2FSDL.h.3E.3F) Here, configure.ac does call sdl-config but then uses the wrong path (SDL/SDL.h instead of just SDL.h) to try and find the header, so the compiler uses the /usr/include version of SDL.h instead of the path returned by sdl-config and things break. This is the fault of the package, not Gentoo. This does mean we need to wrap sdl*-config, but I don't think that's a great difficulty? (In reply to Julian Ospald (hasufell) from comment #13) > I am really inclined to revert the multilib migration of sdl. What's the > fallout of such a step? Because reverting is so much better than choosing a single working solution. The fallout would be a severe number of broken revdeps. Even if you fix them all, the upgrade will be PITA if possible at all. [QA] I agree with mgorny and ssuominen that if an upstream build system doesn't use pkg-config or sdl-config to obtain flags, then this is a bug, and it should be reported upstream. Also the bug was always there, header wrapping only exposes it now. (In reply to Julian Ospald (hasufell) from comment #13) > I'd say: > * restore old emul-linux package > * revbump sdl related ebuilds and rm multilib support > * fix reverse deps if any This isn't a feasible solution, but a dead end, for obvious reasons. Short term: My guess is that the quickest short term solution is to also "wrap" such recursively-included files which are included from files that differ by ABI, even though the recursively included files don't actually change between ABIs. (I expect this situation is really rare. There could be more such cases, but I wouldn't be surprised if SDL was the only one. It certainly seems like a strange way to organise things.) I suspect this is probably easier and more robust than trying to revert sdl and rejigger emul-linux packages. ---- Longer term, some possibly better solutions include: - Wrap headers inline, as in comment #10. - Instead of moving the included-from-wrapper headers to obscure directories, keep them in the same directory but give them unique names. (Something like "SDL.h.amd_x86_32.h") - Auto-detect future problems: Include some QA tools/scripts that error out (or at least warn loudly) if any of the included-from-wrapper headers try to #include files that aren't wrapped. - Rework "variable" headers to be defined in terms of standard mechanisms such as <stdint.h>, size_t, ptrdiff_t, and/or magic #ifdef's (usually against pre-defined CPU macros), so that they can do the correct thing without needing different headers for different ABIs. Or at the very least, organise it so that the variable header is an internal, very-short header that doesn't include other headers (instead, other headers include it). All else being equal, I think I prefer these kinds of solution strategies in most cases. But counterarguments include: (a) This could be more difficult to maintain than desired unless upstream can be convinced to accept such patches. And in the meantime, this is an arbitrary delay to multilib conversion. (b) This superficially violates the idea that "configure" should test for anything that varies and adjust code appropriately, rather than hard-coding a limited set of supported platforms (that might eventually be fixed or otherwise change in future versions of the platform) in #ifdef's. - I would suggest that neither hiding all SDL headers in platform directories, nor requiring that numerous other previously-working ebuilds patch the relevant programs to use things like sdl-config and #include's "correctly" are really good solutions. (Although it is reasonably to at least report such problems for such packages upstream when we notice them.) @Julian Ospald: You are rather vocally against wrapped headers. Do you have other alternative suggestions to get multilib working, or does my list above cover it? (In reply to Christoph Junghans from comment #10) > As a last resort, couldn't we wrap the header in place, meaning putting the > actual original content inside the #ifdef's and create a X-times bigger > monster header. Why X-times bigger? Only the pieces that are actually different need to be duplicated. GNU diff even has an option for it: -D, --ifdef=NAME output merged file with `#ifdef NAME' diffs (In reply to Ulrich Müller from comment #16) > [QA] I agree with mgorny and ssuominen that if an upstream build system > doesn't use pkg-config or sdl-config to obtain flags, then this is a bug, > and it should be reported upstream. Also the bug was always there, header > wrapping only exposes it now. > a) I disagree with that assumption that build systems are broken if they don't use pkg-config. that's just nonsense and I will NOT follow that suggestion no matter what you or QA says. b) liquidwar6 DOES make use of sdl-config as I already explained c) if there will be more of this breakage, then I will carry out the reversion myself (In reply to Julian Ospald (hasufell) from comment #19) > a) [...] I will NOT follow that suggestion no matter what you or QA says. > > c) if there will be more of this breakage, then I will carry out the > reversion myself CCing Comrel. (In reply to Julian Ospald (hasufell) from comment #19) > a) I disagree with that assumption that build systems are broken if they > don't use pkg-config. that's just nonsense and I will NOT follow that > suggestion no matter what you or QA says. > > b) liquidwar6 DOES make use of sdl-config as I already explained > > c) if there will be more of this breakage, then I will carry out the > reversion myself You realize, that right here and right now, you are trying to violate GLEP 48, do not you? >In case of emergency, or if package maintainers refuse to cooperate, the QA team may take action themselves to fix the problem. The QA team does not want to override the maintainer's wishes by default, but only wish to do so when the team finds it is in the best interest of users and fellow developers to have the issue addressed as soon as possible. So, unless we(read as: multilib team, games team and QA team) all will find proper solution, please do not do anything with libsdl ebuilds. (In reply to Sergey Popov from comment #21) > So, unless we(read as: multilib team, games team and QA team) all will find > proper solution, please do not do anything with libsdl ebuilds. I am sorry, i was not clear. Read last sentence as: please do not do anything with libsdl ebuilds, UNLESS you can fix it in proper way, without beginning revert war. (In reply to Sergey Popov from comment #21) > (In reply to Julian Ospald (hasufell) from comment #19) > > a) I disagree with that assumption that build systems are broken if they > > don't use pkg-config. that's just nonsense and I will NOT follow that > > suggestion no matter what you or QA says. > > > > b) liquidwar6 DOES make use of sdl-config as I already explained > > > > c) if there will be more of this breakage, then I will carry out the > > reversion myself > > You realize, that right here and right now, you are trying to violate GLEP > 48, do not you? > Before doing such assumptions, make some research first. I did the multilib conversions and so ANY breakage caused by that is on ME (right, I messed up). If this problems grows bigger then it is my responsibility to fix this. The only thing QA did in this thread is saying things that are wrong. If you expect all packages to use pkg-config then you have to fix 80%+ of the tree, including python packages, cmake, autotools... literally anything. This is so nonsensical that I will NOT do this. Instead QA almost encourages people to hack randomly on pkgconfig files downstream which is even more WRONG. So because our eclasses are partially misdesigned we dump the work on everyone, expect upstreams to follow OUR standards and just hack up stuff that does not get accepted upstream? So let's sum it up: * pkg-config works well for our multilib hackery... so just expect everyone to use it * if someone does not use it, just hack it up downstream * if any build system uses different approaches that work in 99% of the cases, but not our case of multilib hackery, then call them "broken" * tell people they just violated GLEP 48 by speaking their mind (In reply to Julian Ospald (hasufell) from comment #23) Please read again what I have written above. I have neither suggested that all packages in the tree should use pkg-config, nor have I said that we should ship our own pkgconfig files for packages that don't provide them. In the concrete case, libsdl _does_ support pkg-config and in addition comes with its own sdl-config, so packages depending on libsdl should use (one of) these tools. Indeed, liquidwar6's build system calls sdl-config --cflags. However, it fails to set CFLAGS/CPPFLAGS (using the sdl-config result) before calling AC_CHECK_HEADER{,S}. I argue that this qualifies as a bug in the build system (why use pkg-config or sdl-config when you ignore its output then?) and should be reported upstream. Also, I haven't claimed that the current method used for wrapping headers is perfect. If it will turn out that there are more bugs exposed by it, then some of the ideas outlined in comment #10 and comment #17 should be considered. (In reply to Ulrich Müller from comment #24) > (In reply to Julian Ospald (hasufell) from comment #23) > > Please read again what I have written above. I have neither suggested that > all packages in the tree should use pkg-config, nor have I said that we > should ship our own pkgconfig files for packages that don't provide them. > Sorry about that. I am pissed and got caught in the heat. > In the concrete case, libsdl _does_ support pkg-config and in addition comes > with its own sdl-config, so packages depending on libsdl should use (one of) > these tools. Indeed, liquidwar6's build system calls sdl-config --cflags. > However, it fails to set CFLAGS/CPPFLAGS (using the sdl-config result) > before calling AC_CHECK_HEADER{,S}. I argue that this qualifies as a bug in > the build system (why use pkg-config or sdl-config when you ignore its > output then?) and should be reported upstream. > Right, and I have already patched it. Besides... it wasn't trivial at all. Because autotools does some very weird things with the CFLAGS in all those calls and just setting CFLAGS properly before AC_CHECK_HEADERS did not work. So I have already wasted some time on researching when CFLAGS get _actually_ set for those headers checks etc. > Also, I haven't claimed that the current method used for wrapping headers is > perfect. If it will turn out that there are more bugs exposed by it, then > some of the ideas outlined in comment #10 and comment #17 should be > considered. Right, I am trying to work WITH people. That's why I opened this bug. What I will not do is: * introduce pkg-config to random packages * fix 200+ packages because of wrapped headers * add ANY other ebuild to the tree that does wrap headers (In reply to Julian Ospald (hasufell) from comment #25) > Right, I am trying to work WITH people. That's why I opened this bug. > > What I will not do is: > * introduce pkg-config to random packages > * fix 200+ packages because of wrapped headers > * add ANY other ebuild to the tree that does wrap headers Nobody asks you to do that. And we appreciate when you cooperate with others - that's what we all need here: clean, non-hacky solution to the whole problem. It's easy to revert all things and say: 'oops, screw that'. But it's not easy to fix things properly. > Right, and I have already patched it. Besides... it wasn't trivial at all. > Because autotools does some very weird things with the CFLAGS in all those > calls and just setting CFLAGS properly before AC_CHECK_HEADERS did not work. > So I have already wasted some time on researching when CFLAGS get _actually_ > set for those headers checks etc. You tell me. I have already spent many hours messing with autotools stuff, so i understand why you became upset :-) |
