configure: Check for SDL using SDL_Init, too

Message ID 1332956819-19761-1-git-send-email-martin@martin.st
State Superseded
Headers show

Commit Message

Martin Storsjö March 28, 2012, 5:46 p.m.
SDL 1.3 (which is the current version available e.g. in
macports) doesn't contain SDL_Linked_Version.

The current check for SDL_Linked_Version (available since SDL
1.2.13) was added 8f1b06c8, because including the headers for
SDL_Init redirects the main function, requiring linking the
SDLmain library for linking to work. When using the normal
function checks in configure, we don't link to any extra
libraries.
---
 configure |    3 ++-
 1 files changed, 2 insertions(+), 1 deletions(-)

Comments

Diego Biurrun March 28, 2012, 6 p.m. | #1
IMO slightly better:

  configure: fall back on checking for SDL_Init() if primary SDL check fails

On Wed, Mar 28, 2012 at 08:46:59PM +0300, Martin Storsjö wrote:
> SDL 1.3 (which is the current version available e.g. in
> macports) doesn't contain SDL_Linked_Version.
> 
> The current check for SDL_Linked_Version (available since SDL
> 1.2.13) was added 8f1b06c8, because including the headers for

was added in

> SDL_Init redirects the main function, requiring linking the

main()

> SDLmain library for linking to work. When using the normal
> function checks in configure, we don't link to any extra
> libraries.

nit: If you used lines about 80 characters instead of about 60
characters in length, this log message would be shorter.

> --- a/configure
> +++ b/configure
> @@ -2983,7 +2983,8 @@ if enabled libdc1394; then
>  
> -if check_pkg_config sdl SDL_version.h SDL_Linked_Version; then
> +if check_pkg_config sdl SDL_version.h SDL_Linked_Version ||
> +   check_pkg_config sdl SDL.h SDL_Init; then
>      check_cpp_condition SDL.h "(SDL_MAJOR_VERSION<<16 | SDL_MINOR_VERSION<<8 | SDL_PATCHLEVEL) >= 0x010201" $sdl_cflags &&
>      enable sdl &&
>      check_struct SDL.h SDL_VideoInfo current_w $sdl_cflags && enable sdl_video_size

Patch OK, preferably with my suggestions for the log message.

Diego
Mans Rullgard March 28, 2012, 6:26 p.m. | #2
Martin Storsjö <martin@martin.st> writes:

> SDL 1.3 (which is the current version available e.g. in
> macports) doesn't contain SDL_Linked_Version.
>
> The current check for SDL_Linked_Version (available since SDL
> 1.2.13) was added 8f1b06c8, because including the headers for
> SDL_Init redirects the main function, requiring linking the
> SDLmain library for linking to work. When using the normal
> function checks in configure, we don't link to any extra
> libraries.
> ---
>  configure |    3 ++-
>  1 files changed, 2 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 7418bca..3e24f00 100755
> --- a/configure
> +++ b/configure
> @@ -2983,7 +2983,8 @@ if enabled libdc1394; then
>      die "ERROR: No version of libdc1394 found "
>  fi
>
> -if check_pkg_config sdl SDL_version.h SDL_Linked_Version; then
> +if check_pkg_config sdl SDL_version.h SDL_Linked_Version ||
> +   check_pkg_config sdl SDL.h SDL_Init; then

If checking for SDL_Init works at all, then there should be no need to
check for SDL_Linked_Version.
Martin Storsjö March 28, 2012, 6:37 p.m. | #3
On Wed, 28 Mar 2012, Måns Rullgård wrote:

> Martin Storsjö <martin@martin.st> writes:
>
>> SDL 1.3 (which is the current version available e.g. in
>> macports) doesn't contain SDL_Linked_Version.
>>
>> The current check for SDL_Linked_Version (available since SDL
>> 1.2.13) was added 8f1b06c8, because including the headers for
>> SDL_Init redirects the main function, requiring linking the
>> SDLmain library for linking to work. When using the normal
>> function checks in configure, we don't link to any extra
>> libraries.
>> ---
>>  configure |    3 ++-
>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 7418bca..3e24f00 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2983,7 +2983,8 @@ if enabled libdc1394; then
>>      die "ERROR: No version of libdc1394 found "
>>  fi
>>
>> -if check_pkg_config sdl SDL_version.h SDL_Linked_Version; then
>> +if check_pkg_config sdl SDL_version.h SDL_Linked_Version ||
>> +   check_pkg_config sdl SDL.h SDL_Init; then
>
> If checking for SDL_Init works at all, then there should be no need to
> check for SDL_Linked_Version.

It doesn't, on the platforms where SDL redirects the main function. 
Thankfully, the new ones that lack SDL_Linked_Version don't do any such 
redirection afaik.

That is - the headers that define SDL_Init also redefine main, to redirect 
initialization via their main wrapper. If this header is included, one has 
to link with the SDLmain library, otherwise one ends up with undefined 
references to main.

We could either link to this library while checking for SDL_Init - that 
was one suggestion back when 8f1b06c8 was made - but we chose to test a 
different function from a different header, which didn't include the main 
redirection, instead of modifying check_pkg_config and the related 
functions.

// Martin
Mans Rullgard March 28, 2012, 7:54 p.m. | #4
Martin Storsjö <martin@martin.st> writes:

> On Wed, 28 Mar 2012, Måns Rullgård wrote:
>
>> Martin Storsjö <martin@martin.st> writes:
>>
>>> SDL 1.3 (which is the current version available e.g. in
>>> macports) doesn't contain SDL_Linked_Version.
>>>
>>> The current check for SDL_Linked_Version (available since SDL
>>> 1.2.13) was added 8f1b06c8, because including the headers for
>>> SDL_Init redirects the main function, requiring linking the
>>> SDLmain library for linking to work. When using the normal
>>> function checks in configure, we don't link to any extra
>>> libraries.
>>> ---
>>>  configure |    3 ++-
>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 7418bca..3e24f00 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2983,7 +2983,8 @@ if enabled libdc1394; then
>>>      die "ERROR: No version of libdc1394 found "
>>>  fi
>>>
>>> -if check_pkg_config sdl SDL_version.h SDL_Linked_Version; then
>>> +if check_pkg_config sdl SDL_version.h SDL_Linked_Version ||
>>> +   check_pkg_config sdl SDL.h SDL_Init; then
>>
>> If checking for SDL_Init works at all, then there should be no need to
>> check for SDL_Linked_Version.
>
> It doesn't, on the platforms where SDL redirects the main
> function. Thankfully, the new ones that lack SDL_Linked_Version don't
> do any such redirection afaik.
>
> That is - the headers that define SDL_Init also redefine main, to
> redirect initialization via their main wrapper. If this header is
> included, one has to link with the SDLmain library, otherwise one ends
> up with undefined references to main.

Why does so much as thinking about SDL make me angry?

> We could either link to this library while checking for SDL_Init -

Wouldn't that break on systems that don't have the wrapper?
Martin Storsjö March 29, 2012, 8:10 a.m. | #5
On Wed, 28 Mar 2012, Måns Rullgård wrote:

> Martin Storsjö <martin@martin.st> writes:
>
>> On Wed, 28 Mar 2012, Måns Rullgård wrote:
>>
>>> Martin Storsjö <martin@martin.st> writes:
>>>
>>>> SDL 1.3 (which is the current version available e.g. in
>>>> macports) doesn't contain SDL_Linked_Version.
>>>>
>>>> The current check for SDL_Linked_Version (available since SDL
>>>> 1.2.13) was added 8f1b06c8, because including the headers for
>>>> SDL_Init redirects the main function, requiring linking the
>>>> SDLmain library for linking to work. When using the normal
>>>> function checks in configure, we don't link to any extra
>>>> libraries.
>>>> ---
>>>>  configure |    3 ++-
>>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 7418bca..3e24f00 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -2983,7 +2983,8 @@ if enabled libdc1394; then
>>>>      die "ERROR: No version of libdc1394 found "
>>>>  fi
>>>>
>>>> -if check_pkg_config sdl SDL_version.h SDL_Linked_Version; then
>>>> +if check_pkg_config sdl SDL_version.h SDL_Linked_Version ||
>>>> +   check_pkg_config sdl SDL.h SDL_Init; then
>>>
>>> If checking for SDL_Init works at all, then there should be no need to
>>> check for SDL_Linked_Version.
>>
>> It doesn't, on the platforms where SDL redirects the main
>> function. Thankfully, the new ones that lack SDL_Linked_Version don't
>> do any such redirection afaik.
>>
>> That is - the headers that define SDL_Init also redefine main, to
>> redirect initialization via their main wrapper. If this header is
>> included, one has to link with the SDLmain library, otherwise one ends
>> up with undefined references to main.
>
> Why does so much as thinking about SDL make me angry?
>
>> We could either link to this library while checking for SDL_Init -
>
> Wouldn't that break on systems that don't have the wrapper?

I think all systems (with SDL 1.2) have the wrapper, but on the ones where 
it doesn't redirect main, it might be an empty library. But on SDL 1.3, it 
might of course not exist...

After rechecking closer, the issue isn't so much about linking to the 
wrapper, check_pkg_config actually does that correctly already, since the 
pc file incidates the library to add.

The problem is that the main redirection is defined like this:

#define main SDL_main
extern C_LINKAGE int SDL_main(int argc, char *argv[]);

And the function check tests compiling this:

     1   #include <SDL.h>
     2   long check_SDL_Init(void) { return (long) SDL_Init; }
     3   int main(void) { return 0; }

ending up with this error:

ffconf.UGdDwiYR.c:3: error: conflicting types for 'SDL_main'
SDL_main.h:57: error: previous declaration of 'SDL_main' was here

So sorry, I misremembered the details.

So the way I see it, either we avoid SDL.h (which includes SDL_main.h) in 
the function checks - which we've done so far (and maybe only use SDL.h as 
fallback check if SDL_Linked_Version failed) - or we add argc/argv to the 
main declaration in the function check (which of course could break again 
if there was another library doing similar nasty things with a different 
declaration).

// Martin
Mans Rullgard March 29, 2012, 11:35 a.m. | #6
Martin Storsjö <martin@martin.st> writes:

> On Wed, 28 Mar 2012, Måns Rullgård wrote:
>
>> Martin Storsjö <martin@martin.st> writes:
>>
>>> On Wed, 28 Mar 2012, Måns Rullgård wrote:
>>>
>>>> Martin Storsjö <martin@martin.st> writes:
>>>>
>>>>> SDL 1.3 (which is the current version available e.g. in
>>>>> macports) doesn't contain SDL_Linked_Version.
>>>>>
>>>>> The current check for SDL_Linked_Version (available since SDL
>>>>> 1.2.13) was added 8f1b06c8, because including the headers for
>>>>> SDL_Init redirects the main function, requiring linking the
>>>>> SDLmain library for linking to work. When using the normal
>>>>> function checks in configure, we don't link to any extra
>>>>> libraries.
>>>>> ---
>>>>>  configure |    3 ++-
>>>>>  1 files changed, 2 insertions(+), 1 deletions(-)
>>>>>
>>>>> diff --git a/configure b/configure
>>>>> index 7418bca..3e24f00 100755
>>>>> --- a/configure
>>>>> +++ b/configure
>>>>> @@ -2983,7 +2983,8 @@ if enabled libdc1394; then
>>>>>      die "ERROR: No version of libdc1394 found "
>>>>>  fi
>>>>>
>>>>> -if check_pkg_config sdl SDL_version.h SDL_Linked_Version; then
>>>>> +if check_pkg_config sdl SDL_version.h SDL_Linked_Version ||
>>>>> +   check_pkg_config sdl SDL.h SDL_Init; then
>>>>
>>>> If checking for SDL_Init works at all, then there should be no need to
>>>> check for SDL_Linked_Version.
>>>
>>> It doesn't, on the platforms where SDL redirects the main
>>> function. Thankfully, the new ones that lack SDL_Linked_Version don't
>>> do any such redirection afaik.
>>>
>>> That is - the headers that define SDL_Init also redefine main, to
>>> redirect initialization via their main wrapper. If this header is
>>> included, one has to link with the SDLmain library, otherwise one ends
>>> up with undefined references to main.
>>
>> Why does so much as thinking about SDL make me angry?
>>
>>> We could either link to this library while checking for SDL_Init -
>>
>> Wouldn't that break on systems that don't have the wrapper?
>
> I think all systems (with SDL 1.2) have the wrapper, but on the ones
> where it doesn't redirect main, it might be an empty library. But on
> SDL 1.3, it might of course not exist...
>
> After rechecking closer, the issue isn't so much about linking to the
> wrapper, check_pkg_config actually does that correctly already, since
> the pc file incidates the library to add.
>
> The problem is that the main redirection is defined like this:
>
> #define main SDL_main
> extern C_LINKAGE int SDL_main(int argc, char *argv[]);
>
> And the function check tests compiling this:
>
>     1   #include <SDL.h>
>     2   long check_SDL_Init(void) { return (long) SDL_Init; }
>     3   int main(void) { return 0; }
>
> ending up with this error:
>
> ffconf.UGdDwiYR.c:3: error: conflicting types for 'SDL_main'
> SDL_main.h:57: error: previous declaration of 'SDL_main' was here
>
> So sorry, I misremembered the details.

Is there some other function which doesn't pull in the main()
redirection and exists in all versions?

Patch

diff --git a/configure b/configure
index 7418bca..3e24f00 100755
--- a/configure
+++ b/configure
@@ -2983,7 +2983,8 @@  if enabled libdc1394; then
     die "ERROR: No version of libdc1394 found "
 fi
 
-if check_pkg_config sdl SDL_version.h SDL_Linked_Version; then
+if check_pkg_config sdl SDL_version.h SDL_Linked_Version ||
+   check_pkg_config sdl SDL.h SDL_Init; then
     check_cpp_condition SDL.h "(SDL_MAJOR_VERSION<<16 | SDL_MINOR_VERSION<<8 | SDL_PATCHLEVEL) >= 0x010201" $sdl_cflags &&
     enable sdl &&
     check_struct SDL.h SDL_VideoInfo current_w $sdl_cflags && enable sdl_video_size