Gentoo Websites Logo
Go to: Gentoo Home Documentation Forums Lists Bugs Planet Store Wiki Get Gentoo!
Bug 512430 - [TRACKER] media-libs/libsdl multilib-wrapped headers breakage
Summary: [TRACKER] media-libs/libsdl multilib-wrapped headers breakage
Status: RESOLVED FIXED
Alias: None
Product: Gentoo Linux
Classification: Unclassified
Component: Eclasses (show other bugs)
Hardware: All Linux
: Normal normal (vote)
Assignee: Multilib team
URL:
Whiteboard:
Keywords: Tracker
Depends on: 510276 510474 512914 513974 514238 514240 514242 514302 514316 514358 514368 514426
Blocks:
  Show dependency tree
 
Reported: 2014-06-04 16:38 UTC by Julian Ospald
Modified: 2017-01-19 18:46 UTC (History)
4 users (show)

See Also:
Package list:
Runtime testing required: ---


Attachments
liquidwar6-0.4.3681-SDL.patch (liquidwar6-0.4.3681-SDL.patch,564 bytes, patch)
2014-06-05 02:41 UTC, Mr. Bones. (RETIRED)
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Julian Ospald 2014-06-04 16:38:54 UTC
after conversion of libsdl to multilib, liquidwar6 fails to emerge:

> checking for SDL.h... no
> configure: WARNING:
> *** Liquid War 6 needs SDL (http://www.libsdl.org/)
> 
> checking for SDL_Init in -lSDL... yes

The configure.ac code looks like this:

> AC_CHECK_HEADER(SDL/SDL.h, HAVE_SDL_H=1, AC_MSG_WARN([
> *** Liquid War 6 needs SDL (http://www.libsdl.org/)
> ]),[${SDL_EXTRA}])

config.log:
> configure:14410: x86_64-pc-linux-gnu-gcc -c -march=core-avx2 -O2 -pipe -Wall -g  conftest.c >&5
> In file included from /usr/include/SDL/SDL_config.h:10:0,
>                  from /usr/include/SDL/SDL_stdinc.h:30,
>                  from /usr/include/SDL/SDL_main.h:26,
>                  from /usr/include/SDL/SDL.h:30,
>                  from conftest.c:97:
> /usr/include/x86_64-pc-linux-gnu/SDL/SDL_config.h:30:26: fatal error: SDL_platform.h: No such file or directory
>  #include "SDL_platform.h"
                          ^
> compilation terminated.

This suggests that our approach of wrapping headers is bad.

I will definitely not convert more ebuilds that require wrapped headers. I'd rather consider to revert libsdl conversion or mask them, but that probably won't help.
Comment 1 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-06-04 22:12:41 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.
Comment 2 Julian Ospald 2014-06-04 22:19:33 UTC
(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
Comment 3 Samuli Suominen (RETIRED) gentoo-dev 2014-06-04 22:23:35 UTC
most definately yes, anything not using pkg-config is broken
Comment 4 Julian Ospald 2014-06-04 22:25:35 UTC
(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.
Comment 5 Mr. Bones. (RETIRED) gentoo-dev 2014-06-05 02:41:16 UTC
Created attachment 378300 [details, diff]
liquidwar6-0.4.3681-SDL.patch

patch works around it.
Comment 6 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-06-05 19:19:00 UTC
(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.
Comment 7 Julian Ospald 2014-06-05 19:37:25 UTC
(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.
Comment 8 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-06-05 19:51:03 UTC
(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.
Comment 9 Julian Ospald 2014-06-05 20:10:18 UTC
(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".
Comment 10 Christoph Junghans gentoo-dev 2014-06-05 20:37:31 UTC
(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.
Comment 11 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-06-06 19:46:24 UTC
(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).
Comment 12 Petr Zima 2014-06-10 22:17:42 UTC
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.
Comment 13 Julian Ospald 2014-06-10 22:28:03 UTC
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.
Comment 14 Andrew Church 2014-06-11 02:42:45 UTC
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?
Comment 15 Michał Górny archtester Gentoo Infrastructure gentoo-dev Security 2014-06-11 06:30:23 UTC
(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.
Comment 16 Ulrich Müller gentoo-dev 2014-06-11 06:40:01 UTC
[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.
Comment 17 Matthew Ogilvie 2014-06-11 07:26:26 UTC
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?
Comment 18 Ulrich Müller gentoo-dev 2014-06-11 07:35:47 UTC
(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
Comment 19 Julian Ospald 2014-06-11 10:26:04 UTC
(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
Comment 20 Ulrich Müller gentoo-dev 2014-06-11 10:36:05 UTC
(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.
Comment 21 Sergey Popov gentoo-dev 2014-06-11 11:09:28 UTC
(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.
Comment 22 Sergey Popov gentoo-dev 2014-06-11 11:10:17 UTC
(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.
Comment 23 Julian Ospald 2014-06-11 14:43:28 UTC
(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
Comment 24 Ulrich Müller gentoo-dev 2014-06-11 15:29:55 UTC
(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.
Comment 25 Julian Ospald 2014-06-11 15:45:03 UTC
(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
Comment 26 Sergey Popov gentoo-dev 2014-06-12 11:00:21 UTC
(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 :-)