Check for sdl-config before calling it

Submitted by Loïc Minier on Jan. 17, 2010, 12:45 p.m.

Details

Message ID 20100117124528.GA24106@bee.dooz.org
State New
Headers show

Commit Message

Loïc Minier Jan. 17, 2010, 12:45 p.m.
Hi

 On systems were sdl-config isn't installed, ./configure triggers this
 warning:
    ./configure: 957: sdl-config: not found

 The attached patch fixes the warning for me.

   Thanks,

Comments

Måns Rullgård Jan. 17, 2010, 1:17 p.m.
Loïc Minier <lool@dooz.org> writes:

>         Hi
>
>  On systems were sdl-config isn't installed, ./configure triggers this
>  warning:
>     ./configure: 957: sdl-config: not found
>
>  The attached patch fixes the warning for me.
>
>    Thanks,
> -- 
> Loïc Minier
>
> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
> Date: Sun, 17 Jan 2010 13:42:04 +0100
> Subject: [PATCH] Check for sdl-config before calling it
> MIME-Version: 1.0
> Content-Type: text/plain; charset=UTF-8
> Content-Transfer-Encoding: 8bit
>
> Check whether sdl-config is available before calling it, otherwise
> ./configure triggers a warning:
>     ./configure: 957: sdl-config: not found
>
> If neither the .pc file not sdl-config are present, disable SDL support.
>
> Signed-off-by: Loïc Minier <lool@dooz.org>
> ---
>  configure |    4 +++-
>  1 files changed, 3 insertions(+), 1 deletions(-)
>
> diff --git a/configure b/configure
> index 5631bbb..450e1a2 100755
> --- a/configure
> +++ b/configure
> @@ -993,9 +993,11 @@ fi
>  if $pkgconfig sdl --modversion >/dev/null 2>&1; then
>    sdlconfig="$pkgconfig sdl"
>    _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
> -else
> +elif which sdl-config >/dev/null 2>&1; then
>    sdlconfig='sdl-config'
>    _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
> +else
> +  sdl=no
>  fi

"which" isn't a standard command.  Would simply 2>/dev/null do the
trick just as well?  If you really need to test for the existence of
something, use "type".
Stefan Weil Jan. 17, 2010, 1:36 p.m.
Loïc Minier schrieb:
> Hi
>
> On systems were sdl-config isn't installed, ./configure triggers this
> warning:
> ./configure: 957: sdl-config: not found
>
> The attached patch fixes the warning for me.
>
> Thanks,

Hi,

which version did you test?
Git master has no sdl-config call at configure:957.

But I get warning messages, too, when pkg-config or
sdl-config are missing.

Your patch fixes the warning for sdl-config and sets
sdl=no. If configure was called with --enable-sdl,
this solution is wrong: configure should abort with
an error message (feature_not_found).

Regards,
Stefan
Stefan Weil Jan. 17, 2010, 1:43 p.m.
Måns Rullgård schrieb:
> Loïc Minier <lool@dooz.org> writes:
>
>   
>>         Hi
>>
>>  On systems were sdl-config isn't installed, ./configure triggers this
>>  warning:
>>     ./configure: 957: sdl-config: not found
>>
>>  The attached patch fixes the warning for me.
>>
>>    Thanks,
>> -- 
>> Loïc Minier
>>
>> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
>> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
>> Date: Sun, 17 Jan 2010 13:42:04 +0100
>> Subject: [PATCH] Check for sdl-config before calling it
>> MIME-Version: 1.0
>> Content-Type: text/plain; charset=UTF-8
>> Content-Transfer-Encoding: 8bit
>>
>> Check whether sdl-config is available before calling it, otherwise
>> ./configure triggers a warning:
>>     ./configure: 957: sdl-config: not found
>>
>> If neither the .pc file not sdl-config are present, disable SDL support.
>>
>> Signed-off-by: Loïc Minier <lool@dooz.org>
>> ---
>>  configure |    4 +++-
>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>
>> diff --git a/configure b/configure
>> index 5631bbb..450e1a2 100755
>> --- a/configure
>> +++ b/configure
>> @@ -993,9 +993,11 @@ fi
>>  if $pkgconfig sdl --modversion >/dev/null 2>&1; then
>>    sdlconfig="$pkgconfig sdl"
>>    _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
>> -else
>> +elif which sdl-config >/dev/null 2>&1; then
>>    sdlconfig='sdl-config'
>>    _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
>> +else
>> +  sdl=no
>>  fi
>>     
>
> "which" isn't a standard command.  Would simply 2>/dev/null do the
> trick just as well?  If you really need to test for the existence of
> something, use "type".
>   

"which" is already used several times in configure,
so this would not make things worse.

A macro for all these tests might be a better solution.
And that macro should use "type" or "type -t".

Regards,
Stefan
Måns Rullgård Jan. 17, 2010, 2:39 p.m.
Stefan Weil <weil@mail.berlios.de> writes:

> Måns Rullgård schrieb:
>> Loïc Minier <lool@dooz.org> writes:
>>
>>   
>>>         Hi
>>>
>>>  On systems were sdl-config isn't installed, ./configure triggers this
>>>  warning:
>>>     ./configure: 957: sdl-config: not found
>>>
>>>  The attached patch fixes the warning for me.
>>>
>>>    Thanks,
>>> -- 
>>> Loïc Minier
>>>
>>> From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
>>> From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
>>> Date: Sun, 17 Jan 2010 13:42:04 +0100
>>> Subject: [PATCH] Check for sdl-config before calling it
>>> MIME-Version: 1.0
>>> Content-Type: text/plain; charset=UTF-8
>>> Content-Transfer-Encoding: 8bit
>>>
>>> Check whether sdl-config is available before calling it, otherwise
>>> ./configure triggers a warning:
>>>     ./configure: 957: sdl-config: not found
>>>
>>> If neither the .pc file not sdl-config are present, disable SDL support.
>>>
>>> Signed-off-by: Loïc Minier <lool@dooz.org>
>>> ---
>>>  configure |    4 +++-
>>>  1 files changed, 3 insertions(+), 1 deletions(-)
>>>
>>> diff --git a/configure b/configure
>>> index 5631bbb..450e1a2 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -993,9 +993,11 @@ fi
>>>  if $pkgconfig sdl --modversion >/dev/null 2>&1; then
>>>    sdlconfig="$pkgconfig sdl"
>>>    _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
>>> -else
>>> +elif which sdl-config >/dev/null 2>&1; then
>>>    sdlconfig='sdl-config'
>>>    _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
>>> +else
>>> +  sdl=no
>>>  fi
>>>     
>>
>> "which" isn't a standard command.  Would simply 2>/dev/null do the
>> trick just as well?  If you really need to test for the existence of
>> something, use "type".
>>   
>
> "which" is already used several times in configure,

Then those should be fixed.

> so this would not make things worse.

One error does not justify another.

> A macro for all these tests might be a better solution.
> And that macro should use "type" or "type -t".

"type -t" is also not standard.  The standard "type" has no options.

In most cases the simpler solution is still probably to try the
command and let it fail.

Patch hide | download patch | download mbox

From 94876939db7f46cf8d920e289d0d4f929d3b7df1 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Lo=C3=AFc=20Minier?= <lool@dooz.org>
Date: Sun, 17 Jan 2010 13:42:04 +0100
Subject: [PATCH] Check for sdl-config before calling it
MIME-Version: 1.0
Content-Type: text/plain; charset=UTF-8
Content-Transfer-Encoding: 8bit

Check whether sdl-config is available before calling it, otherwise
./configure triggers a warning:
    ./configure: 957: sdl-config: not found

If neither the .pc file not sdl-config are present, disable SDL support.

Signed-off-by: Loïc Minier <lool@dooz.org>
---
 configure |    4 +++-
 1 files changed, 3 insertions(+), 1 deletions(-)

diff --git a/configure b/configure
index 5631bbb..450e1a2 100755
--- a/configure
+++ b/configure
@@ -993,9 +993,11 @@  fi
 if $pkgconfig sdl --modversion >/dev/null 2>&1; then
   sdlconfig="$pkgconfig sdl"
   _sdlversion=`$sdlconfig --modversion 2>/dev/null | sed 's/[^0-9]//g'`
-else
+elif which sdl-config >/dev/null 2>&1; then
   sdlconfig='sdl-config'
   _sdlversion=`$sdlconfig --version | sed 's/[^0-9]//g'`
+else
+  sdl=no
 fi
 
 sdl_too_old=no
-- 
1.6.5