diff mbox

configure: make it possible to disable IASL

Message ID 20131124095239.GA30786@redhat.com
State New
Headers show

Commit Message

Michael S. Tsirkin Nov. 24, 2013, 9:52 a.m. UTC
Useful for platforms with a broken IASL.

Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

Comments

Marcel Apfelbaum Nov. 24, 2013, 1:04 p.m. UTC | #1
On Sun, 2013-11-24 at 11:52 +0200, Michael S. Tsirkin wrote:
> Useful for platforms with a broken IASL.
> 
> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> ---
>  configure | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
> 
> diff --git a/configure b/configure
> index 0592ba7..d0a0abe 100755
> --- a/configure
> +++ b/configure
> @@ -1080,6 +1080,7 @@ echo "  --source-path=PATH       path of source code [$source_path]"
>  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
>  echo "  --cc=CC                  use C compiler CC [$cc]"
>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
> +echo "                           iasl='' (empty string) disables ACPI compiler"
>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
>  echo "                           build time"
>  echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
> @@ -4269,7 +4270,7 @@ else
>  fi
>  echo "PYTHON=$python" >> $config_host_mak
>  echo "CC=$cc" >> $config_host_mak
> -if $iasl -h > /dev/null 2>&1; then
> +if test "$iasl" && $iasl -h > /dev/null 2>&1; then
>    echo "IASL=$iasl" >> $config_host_mak
>  fi
>  echo "CC_I386=$cc_i386" >> $config_host_mak

I tried to disable iasl without this patch and it worked for me...
I used:
    - [Qemu-devel] [PATCH] configure: make --iasl option actually work
    - ./configure --iasl="" (or --iasl=)
and it worked (no sign of IASL in config-host.mak)

I think this is because the prev test "$iasl -h > /dev/null 2>&1"
fails of course.

I don't get it how <test "$iasl"> helps if iasl is broken.
Thanks,
Marcel
Michael S. Tsirkin Nov. 24, 2013, 1:42 p.m. UTC | #2
On Sun, Nov 24, 2013 at 03:04:04PM +0200, Marcel Apfelbaum wrote:
> On Sun, 2013-11-24 at 11:52 +0200, Michael S. Tsirkin wrote:
> > Useful for platforms with a broken IASL.
> > 
> > Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > ---
> >  configure | 3 ++-
> >  1 file changed, 2 insertions(+), 1 deletion(-)
> > 
> > diff --git a/configure b/configure
> > index 0592ba7..d0a0abe 100755
> > --- a/configure
> > +++ b/configure
> > @@ -1080,6 +1080,7 @@ echo "  --source-path=PATH       path of source code [$source_path]"
> >  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
> >  echo "  --cc=CC                  use C compiler CC [$cc]"
> >  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
> > +echo "                           iasl='' (empty string) disables ACPI compiler"
> >  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
> >  echo "                           build time"
> >  echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
> > @@ -4269,7 +4270,7 @@ else
> >  fi
> >  echo "PYTHON=$python" >> $config_host_mak
> >  echo "CC=$cc" >> $config_host_mak
> > -if $iasl -h > /dev/null 2>&1; then
> > +if test "$iasl" && $iasl -h > /dev/null 2>&1; then
> >    echo "IASL=$iasl" >> $config_host_mak
> >  fi
> >  echo "CC_I386=$cc_i386" >> $config_host_mak
> 
> I tried to disable iasl without this patch and it worked for me...
> I used:
>     - [Qemu-devel] [PATCH] configure: make --iasl option actually work
>     - ./configure --iasl="" (or --iasl=)
> and it worked (no sign of IASL in config-host.mak)
> 
> I think this is because the prev test "$iasl -h > /dev/null 2>&1"
> fails of course.

By luck, yes.

So this is just a documentation patch: cleaner to document the
interface explicitly.
I'll make this clearer in the commit log.

> I don't get it how <test "$iasl"> helps if iasl is broken.
> Thanks,
> Marcel
> 
> 
> 

It doesn't but it creates an interface to disable iasl
for such configurations.
Stefan Weil Nov. 24, 2013, 1:54 p.m. UTC | #3
Am 24.11.2013 14:42, schrieb Michael S. Tsirkin:
> On Sun, Nov 24, 2013 at 03:04:04PM +0200, Marcel Apfelbaum wrote:
>> On Sun, 2013-11-24 at 11:52 +0200, Michael S. Tsirkin wrote:
>>> Useful for platforms with a broken IASL.
>>>
>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>> ---
>>>  configure | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index 0592ba7..d0a0abe 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -1080,6 +1080,7 @@ echo "  --source-path=PATH       path of source code [$source_path]"
>>>  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
>>>  echo "  --cc=CC                  use C compiler CC [$cc]"
>>>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
>>> +echo "                           iasl='' (empty string) disables ACPI compiler"
>>>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
>>>  echo "                           build time"
>>>  echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
>>> @@ -4269,7 +4270,7 @@ else
>>>  fi
>>>  echo "PYTHON=$python" >> $config_host_mak
>>>  echo "CC=$cc" >> $config_host_mak
>>> -if $iasl -h > /dev/null 2>&1; then
>>> +if test "$iasl" && $iasl -h > /dev/null 2>&1; then
>>>    echo "IASL=$iasl" >> $config_host_mak
>>>  fi
>>>  echo "CC_I386=$cc_i386" >> $config_host_mak
>> I tried to disable iasl without this patch and it worked for me...
>> I used:
>>     - [Qemu-devel] [PATCH] configure: make --iasl option actually work
>>     - ./configure --iasl="" (or --iasl=)
>> and it worked (no sign of IASL in config-host.mak)
>>
>> I think this is because the prev test "$iasl -h > /dev/null 2>&1"
>> fails of course.
> By luck, yes.
>
> So this is just a documentation patch: cleaner to document the
> interface explicitly.
> I'll make this clearer in the commit log.
>

--iasl=false works without further modifications (because test "false
-h" works and returns false). It also looks more natural than --iasl=.

Cheers, Stefan
Michael S. Tsirkin Nov. 24, 2013, 3:39 p.m. UTC | #4
On Sun, Nov 24, 2013 at 02:54:51PM +0100, Stefan Weil wrote:
> Am 24.11.2013 14:42, schrieb Michael S. Tsirkin:
> > On Sun, Nov 24, 2013 at 03:04:04PM +0200, Marcel Apfelbaum wrote:
> >> On Sun, 2013-11-24 at 11:52 +0200, Michael S. Tsirkin wrote:
> >>> Useful for platforms with a broken IASL.
> >>>
> >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>> ---
> >>>  configure | 3 ++-
> >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>
> >>> diff --git a/configure b/configure
> >>> index 0592ba7..d0a0abe 100755
> >>> --- a/configure
> >>> +++ b/configure
> >>> @@ -1080,6 +1080,7 @@ echo "  --source-path=PATH       path of source code [$source_path]"
> >>>  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
> >>>  echo "  --cc=CC                  use C compiler CC [$cc]"
> >>>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
> >>> +echo "                           iasl='' (empty string) disables ACPI compiler"
> >>>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
> >>>  echo "                           build time"
> >>>  echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
> >>> @@ -4269,7 +4270,7 @@ else
> >>>  fi
> >>>  echo "PYTHON=$python" >> $config_host_mak
> >>>  echo "CC=$cc" >> $config_host_mak
> >>> -if $iasl -h > /dev/null 2>&1; then
> >>> +if test "$iasl" && $iasl -h > /dev/null 2>&1; then
> >>>    echo "IASL=$iasl" >> $config_host_mak
> >>>  fi
> >>>  echo "CC_I386=$cc_i386" >> $config_host_mak
> >> I tried to disable iasl without this patch and it worked for me...
> >> I used:
> >>     - [Qemu-devel] [PATCH] configure: make --iasl option actually work
> >>     - ./configure --iasl="" (or --iasl=)
> >> and it worked (no sign of IASL in config-host.mak)
> >>
> >> I think this is because the prev test "$iasl -h > /dev/null 2>&1"
> >> fails of course.
> > By luck, yes.
> >
> > So this is just a documentation patch: cleaner to document the
> > interface explicitly.
> > I'll make this clearer in the commit log.
> >
> 
> --iasl=false works without further modifications (because test "false
> -h" works and returns false). It also looks more natural than --iasl=.
> 
> Cheers, Stefan

It seems that some people try --iasl= as the more natural way
to do this. It's not documented that a failing iasl will
cause a fall-back and I'd rather document an explicit option
than making it fail.
Michael S. Tsirkin Nov. 24, 2013, 4:48 p.m. UTC | #5
On Sun, Nov 24, 2013 at 05:39:58PM +0200, Michael S. Tsirkin wrote:
> On Sun, Nov 24, 2013 at 02:54:51PM +0100, Stefan Weil wrote:
> > Am 24.11.2013 14:42, schrieb Michael S. Tsirkin:
> > > On Sun, Nov 24, 2013 at 03:04:04PM +0200, Marcel Apfelbaum wrote:
> > >> On Sun, 2013-11-24 at 11:52 +0200, Michael S. Tsirkin wrote:
> > >>> Useful for platforms with a broken IASL.
> > >>>
> > >>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > >>> ---
> > >>>  configure | 3 ++-
> > >>>  1 file changed, 2 insertions(+), 1 deletion(-)
> > >>>
> > >>> diff --git a/configure b/configure
> > >>> index 0592ba7..d0a0abe 100755
> > >>> --- a/configure
> > >>> +++ b/configure
> > >>> @@ -1080,6 +1080,7 @@ echo "  --source-path=PATH       path of source code [$source_path]"
> > >>>  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
> > >>>  echo "  --cc=CC                  use C compiler CC [$cc]"
> > >>>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
> > >>> +echo "                           iasl='' (empty string) disables ACPI compiler"
> > >>>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
> > >>>  echo "                           build time"
> > >>>  echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
> > >>> @@ -4269,7 +4270,7 @@ else
> > >>>  fi
> > >>>  echo "PYTHON=$python" >> $config_host_mak
> > >>>  echo "CC=$cc" >> $config_host_mak
> > >>> -if $iasl -h > /dev/null 2>&1; then
> > >>> +if test "$iasl" && $iasl -h > /dev/null 2>&1; then
> > >>>    echo "IASL=$iasl" >> $config_host_mak
> > >>>  fi
> > >>>  echo "CC_I386=$cc_i386" >> $config_host_mak
> > >> I tried to disable iasl without this patch and it worked for me...
> > >> I used:
> > >>     - [Qemu-devel] [PATCH] configure: make --iasl option actually work
> > >>     - ./configure --iasl="" (or --iasl=)
> > >> and it worked (no sign of IASL in config-host.mak)
> > >>
> > >> I think this is because the prev test "$iasl -h > /dev/null 2>&1"
> > >> fails of course.
> > > By luck, yes.
> > >
> > > So this is just a documentation patch: cleaner to document the
> > > interface explicitly.
> > > I'll make this clearer in the commit log.
> > >
> > 
> > --iasl=false works without further modifications (because test "false
> > -h" works and returns false). It also looks more natural than --iasl=.
> > 
> > Cheers, Stefan
> 
> It seems that some people try --iasl= as the more natural way
> to do this. It's not documented that a failing iasl will
> cause a fall-back and I'd rather document an explicit option
> than making it fail.

I'm also curious why would you say =false is natural:
it's a string option not a boolean one.
Stefan Weil Nov. 24, 2013, 8:58 p.m. UTC | #6
Am 24.11.2013 17:48, schrieb Michael S. Tsirkin:
> On Sun, Nov 24, 2013 at 05:39:58PM +0200, Michael S. Tsirkin wrote:
>> On Sun, Nov 24, 2013 at 02:54:51PM +0100, Stefan Weil wrote:
>>> Am 24.11.2013 14:42, schrieb Michael S. Tsirkin:
>>>> On Sun, Nov 24, 2013 at 03:04:04PM +0200, Marcel Apfelbaum wrote:
>>>>> On Sun, 2013-11-24 at 11:52 +0200, Michael S. Tsirkin wrote:
>>>>>> Useful for platforms with a broken IASL.
>>>>>>
>>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
>>>>>> ---
>>>>>>  configure | 3 ++-
>>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>>>
>>>>>> diff --git a/configure b/configure
>>>>>> index 0592ba7..d0a0abe 100755
>>>>>> --- a/configure
>>>>>> +++ b/configure
>>>>>> @@ -1080,6 +1080,7 @@ echo "  --source-path=PATH       path of source code [$source_path]"
>>>>>>  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
>>>>>>  echo "  --cc=CC                  use C compiler CC [$cc]"
>>>>>>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
>>>>>> +echo "                           iasl='' (empty string) disables ACPI compiler"
>>>>>>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
>>>>>>  echo "                           build time"
>>>>>>  echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
>>>>>> @@ -4269,7 +4270,7 @@ else
>>>>>>  fi
>>>>>>  echo "PYTHON=$python" >> $config_host_mak
>>>>>>  echo "CC=$cc" >> $config_host_mak
>>>>>> -if $iasl -h > /dev/null 2>&1; then
>>>>>> +if test "$iasl" && $iasl -h > /dev/null 2>&1; then
>>>>>>    echo "IASL=$iasl" >> $config_host_mak
>>>>>>  fi
>>>>>>  echo "CC_I386=$cc_i386" >> $config_host_mak
>>>>> I tried to disable iasl without this patch and it worked for me...
>>>>> I used:
>>>>>     - [Qemu-devel] [PATCH] configure: make --iasl option actually work
>>>>>     - ./configure --iasl="" (or --iasl=)
>>>>> and it worked (no sign of IASL in config-host.mak)
>>>>>
>>>>> I think this is because the prev test "$iasl -h > /dev/null 2>&1"
>>>>> fails of course.
>>>> By luck, yes.
>>>>
>>>> So this is just a documentation patch: cleaner to document the
>>>> interface explicitly.
>>>> I'll make this clearer in the commit log.
>>>>
>>> --iasl=false works without further modifications (because test "false
>>> -h" works and returns false). It also looks more natural than --iasl=.
>>>
>>> Cheers, Stefan
>> It seems that some people try --iasl= as the more natural way
>> to do this. It's not documented that a failing iasl will
>> cause a fall-back and I'd rather document an explicit option
>> than making it fail.
> I'm also curious why would you say =false is natural:
> it's a string option not a boolean one.
>

'false' is a string here, namely the name of the executable which is
normally found at /bin/false.
It takes any number of parameters and always returns 1 (which is boolean
false
in shell conventions).

You could also write --iasl=/bin/false.

I suggest this kind of patch:

 echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
+echo "                           iasl=false disables the ACPI compiler"
 echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"


Stefan
Michael S. Tsirkin Nov. 25, 2013, 11:26 a.m. UTC | #7
On Sun, Nov 24, 2013 at 09:58:34PM +0100, Stefan Weil wrote:
> Am 24.11.2013 17:48, schrieb Michael S. Tsirkin:
> > On Sun, Nov 24, 2013 at 05:39:58PM +0200, Michael S. Tsirkin wrote:
> >> On Sun, Nov 24, 2013 at 02:54:51PM +0100, Stefan Weil wrote:
> >>> Am 24.11.2013 14:42, schrieb Michael S. Tsirkin:
> >>>> On Sun, Nov 24, 2013 at 03:04:04PM +0200, Marcel Apfelbaum wrote:
> >>>>> On Sun, 2013-11-24 at 11:52 +0200, Michael S. Tsirkin wrote:
> >>>>>> Useful for platforms with a broken IASL.
> >>>>>>
> >>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> >>>>>> ---
> >>>>>>  configure | 3 ++-
> >>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> >>>>>>
> >>>>>> diff --git a/configure b/configure
> >>>>>> index 0592ba7..d0a0abe 100755
> >>>>>> --- a/configure
> >>>>>> +++ b/configure
> >>>>>> @@ -1080,6 +1080,7 @@ echo "  --source-path=PATH       path of source code [$source_path]"
> >>>>>>  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
> >>>>>>  echo "  --cc=CC                  use C compiler CC [$cc]"
> >>>>>>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
> >>>>>> +echo "                           iasl='' (empty string) disables ACPI compiler"
> >>>>>>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
> >>>>>>  echo "                           build time"
> >>>>>>  echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
> >>>>>> @@ -4269,7 +4270,7 @@ else
> >>>>>>  fi
> >>>>>>  echo "PYTHON=$python" >> $config_host_mak
> >>>>>>  echo "CC=$cc" >> $config_host_mak
> >>>>>> -if $iasl -h > /dev/null 2>&1; then
> >>>>>> +if test "$iasl" && $iasl -h > /dev/null 2>&1; then
> >>>>>>    echo "IASL=$iasl" >> $config_host_mak
> >>>>>>  fi
> >>>>>>  echo "CC_I386=$cc_i386" >> $config_host_mak
> >>>>> I tried to disable iasl without this patch and it worked for me...
> >>>>> I used:
> >>>>>     - [Qemu-devel] [PATCH] configure: make --iasl option actually work
> >>>>>     - ./configure --iasl="" (or --iasl=)
> >>>>> and it worked (no sign of IASL in config-host.mak)
> >>>>>
> >>>>> I think this is because the prev test "$iasl -h > /dev/null 2>&1"
> >>>>> fails of course.
> >>>> By luck, yes.
> >>>>
> >>>> So this is just a documentation patch: cleaner to document the
> >>>> interface explicitly.
> >>>> I'll make this clearer in the commit log.
> >>>>
> >>> --iasl=false works without further modifications (because test "false
> >>> -h" works and returns false). It also looks more natural than --iasl=.
> >>>
> >>> Cheers, Stefan
> >> It seems that some people try --iasl= as the more natural way
> >> to do this. It's not documented that a failing iasl will
> >> cause a fall-back and I'd rather document an explicit option
> >> than making it fail.
> > I'm also curious why would you say =false is natural:
> > it's a string option not a boolean one.
> >
> 
> 'false' is a string here, namely the name of the executable which is
> normally found at /bin/false.
> It takes any number of parameters and always returns 1 (which is boolean
> false
> in shell conventions).
> 
> You could also write --iasl=/bin/false.
> 
> I suggest this kind of patch:
> 
>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
> +echo "                           iasl=false disables the ACPI compiler"

It's still not a natural interface.

It works by chance because we run iasl and test the return code,
but we don't have to.
For example, a reasonable implementation might produce
an error if the user-specified iasl fails, breaking this hack.

Interfaces should not follow implementation.
If you don't want iasl a reasonable syntax is '' or
--disable-iasl.

>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
> 
> 
> Stefan
Marcel Apfelbaum Nov. 25, 2013, 1:54 p.m. UTC | #8
On Mon, 2013-11-25 at 13:26 +0200, Michael S. Tsirkin wrote:
> On Sun, Nov 24, 2013 at 09:58:34PM +0100, Stefan Weil wrote:
> > Am 24.11.2013 17:48, schrieb Michael S. Tsirkin:
> > > On Sun, Nov 24, 2013 at 05:39:58PM +0200, Michael S. Tsirkin wrote:
> > >> On Sun, Nov 24, 2013 at 02:54:51PM +0100, Stefan Weil wrote:
> > >>> Am 24.11.2013 14:42, schrieb Michael S. Tsirkin:
> > >>>> On Sun, Nov 24, 2013 at 03:04:04PM +0200, Marcel Apfelbaum wrote:
> > >>>>> On Sun, 2013-11-24 at 11:52 +0200, Michael S. Tsirkin wrote:
> > >>>>>> Useful for platforms with a broken IASL.
> > >>>>>>
> > >>>>>> Signed-off-by: Michael S. Tsirkin <mst@redhat.com>
> > >>>>>> ---
> > >>>>>>  configure | 3 ++-
> > >>>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
> > >>>>>>
> > >>>>>> diff --git a/configure b/configure
> > >>>>>> index 0592ba7..d0a0abe 100755
> > >>>>>> --- a/configure
> > >>>>>> +++ b/configure
> > >>>>>> @@ -1080,6 +1080,7 @@ echo "  --source-path=PATH       path of source code [$source_path]"
> > >>>>>>  echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
> > >>>>>>  echo "  --cc=CC                  use C compiler CC [$cc]"
> > >>>>>>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
> > >>>>>> +echo "                           iasl='' (empty string) disables ACPI compiler"
> > >>>>>>  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
> > >>>>>>  echo "                           build time"
> > >>>>>>  echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
> > >>>>>> @@ -4269,7 +4270,7 @@ else
> > >>>>>>  fi
> > >>>>>>  echo "PYTHON=$python" >> $config_host_mak
> > >>>>>>  echo "CC=$cc" >> $config_host_mak
> > >>>>>> -if $iasl -h > /dev/null 2>&1; then
> > >>>>>> +if test "$iasl" && $iasl -h > /dev/null 2>&1; then
> > >>>>>>    echo "IASL=$iasl" >> $config_host_mak
> > >>>>>>  fi
> > >>>>>>  echo "CC_I386=$cc_i386" >> $config_host_mak
> > >>>>> I tried to disable iasl without this patch and it worked for me...
> > >>>>> I used:
> > >>>>>     - [Qemu-devel] [PATCH] configure: make --iasl option actually work
> > >>>>>     - ./configure --iasl="" (or --iasl=)
> > >>>>> and it worked (no sign of IASL in config-host.mak)
> > >>>>>
> > >>>>> I think this is because the prev test "$iasl -h > /dev/null 2>&1"
> > >>>>> fails of course.
> > >>>> By luck, yes.
> > >>>>
> > >>>> So this is just a documentation patch: cleaner to document the
> > >>>> interface explicitly.
> > >>>> I'll make this clearer in the commit log.
> > >>>>
> > >>> --iasl=false works without further modifications (because test "false
> > >>> -h" works and returns false). It also looks more natural than --iasl=.
> > >>>
> > >>> Cheers, Stefan
> > >> It seems that some people try --iasl= as the more natural way
> > >> to do this. It's not documented that a failing iasl will
> > >> cause a fall-back and I'd rather document an explicit option
> > >> than making it fail.
> > > I'm also curious why would you say =false is natural:
> > > it's a string option not a boolean one.
> > >
> > 
> > 'false' is a string here, namely the name of the executable which is
> > normally found at /bin/false.
> > It takes any number of parameters and always returns 1 (which is boolean
> > false
> > in shell conventions).
> > 
> > You could also write --iasl=/bin/false.
> > 
> > I suggest this kind of patch:
> > 
> >  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
> > +echo "                           iasl=false disables the ACPI compiler"
> 
> It's still not a natural interface.
> 
> It works by chance because we run iasl and test the return code,
> but we don't have to.
> For example, a reasonable implementation might produce
> an error if the user-specified iasl fails, breaking this hack.
> 
> Interfaces should not follow implementation.
> If you don't want iasl a reasonable syntax is '' or
> --disable-iasl.
+1 on --disable-iasl

Marcel

> >  echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
> > 
> > 
> > Stefan
Stefan Weil Nov. 25, 2013, 5:27 p.m. UTC | #9
Am 25.11.2013 14:54, schrieb Marcel Apfelbaum:
> On Mon, 2013-11-25 at 13:26 +0200, Michael S. Tsirkin wrote:
>> On Sun, Nov 24, 2013 at 09:58:34PM +0100, Stefan Weil wrote:
>>>>>> --iasl=false works without further modifications (because test "false
>>>>>> -h" works and returns false). It also looks more natural than --iasl=.
>>>>>>
>>>>>> Cheers, Stefan
>>>>> It seems that some people try --iasl= as the more natural way
>>>>> to do this. It's not documented that a failing iasl will
>>>>> cause a fall-back and I'd rather document an explicit option
>>>>> than making it fail.
>>>> I'm also curious why would you say =false is natural:
>>>> it's a string option not a boolean one.
>>>>
>>> 'false' is a string here, namely the name of the executable which is
>>> normally found at /bin/false.
>>> It takes any number of parameters and always returns 1 (which is boolean
>>> false
>>> in shell conventions).
>>>
>>> You could also write --iasl=/bin/false.
>>>
>>> I suggest this kind of patch:
>>>
>>>  echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
>>> +echo "                           iasl=false disables the ACPI compiler"
>> It's still not a natural interface.
>>
>> It works by chance because we run iasl and test the return code,
>> but we don't have to.
>> For example, a reasonable implementation might produce
>> an error if the user-specified iasl fails, breaking this hack.
>>
>> Interfaces should not follow implementation.
>> If you don't want iasl a reasonable syntax is '' or
>> --disable-iasl.
> +1 on --disable-iasl
>
> Marcel

I also prefer --disable-iasl because that's the usual QEMU way how to
disable features.

Cheers,
Stefan
diff mbox

Patch

diff --git a/configure b/configure
index 0592ba7..d0a0abe 100755
--- a/configure
+++ b/configure
@@ -1080,6 +1080,7 @@  echo "  --source-path=PATH       path of source code [$source_path]"
 echo "  --cross-prefix=PREFIX    use PREFIX for compile tools [$cross_prefix]"
 echo "  --cc=CC                  use C compiler CC [$cc]"
 echo "  --iasl=IASL              use ACPI compiler IASL [$iasl]"
+echo "                           iasl='' (empty string) disables ACPI compiler"
 echo "  --host-cc=CC             use C compiler CC [$host_cc] for code run at"
 echo "                           build time"
 echo "  --cxx=CXX                use C++ compiler CXX [$cxx]"
@@ -4269,7 +4270,7 @@  else
 fi
 echo "PYTHON=$python" >> $config_host_mak
 echo "CC=$cc" >> $config_host_mak
-if $iasl -h > /dev/null 2>&1; then
+if test "$iasl" && $iasl -h > /dev/null 2>&1; then
   echo "IASL=$iasl" >> $config_host_mak
 fi
 echo "CC_I386=$cc_i386" >> $config_host_mak