diff mbox

Check for sdl-config before calling it

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

Commit Message

Loïc Minier Jan. 17, 2010, 12:45 p.m. UTC 
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. UTC | #1 
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. UTC | #2 
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. UTC | #3 
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. UTC | #4 
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.
diff mbox

Patch

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