Re: [PATCH] acpi/tests/bios-tables-test: add an environment variable for iasl location

[Michael S. Tsirkin]

On Wed, May 17, 2023 at 04:58:06PM +0100, Alex Bennée wrote:
> 
> "Michael S. Tsirkin" <mst@redhat.com> writes:
> 
> > On Wed, May 17, 2023 at 04:16:47PM +0100, Alex Bennée wrote:
> >> 
> >> Ani Sinha <anisinha@redhat.com> writes:
> >> 
> >> >> On 17-May-2023, at 8:06 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >> 
> >> >> On Wed, May 17, 2023 at 07:57:53PM +0530, Ani Sinha wrote:
> >> >>> 
> >> >>> 
> >> >>>> On 17-May-2023, at 7:47 PM, Michael S. Tsirkin <mst@redhat.com> wrote:
> >> >>>> 
> >> >>>> On Wed, May 17, 2023 at 05:37:51PM +0530, Ani Sinha wrote:
> >> >>>>> Currently the meson based QEMU build process locates the iasl binary 
> >> >>>>> from the
> >> >>>>> current PATH and other locations [1] and uses that to set 
> >> >>>>> CONFIG_IASL which is
> >> >>>>> then used by the test.
> >> >>>>> 
> >> >>>>> This has two disadvantages:
> >> >>>>> - If iasl was not previously installed in the PATH, one has to 
> >> >>>>> install iasl
> >> >>>>>  and rebuild QEMU in order to pick up the iasl location. One cannot 
> >> >>>>> simply
> >> >>>>>  use the existing bios-tables-test binary because CONFIG_IASL is 
> >> >>>>> only set
> >> >>>>>  during the QEMU build time by meson and then bios-tables-test has 
> >> >>>>> to be
> >> >>>>>  rebuilt with CONFIG_IASL set in order to use iasl.
> >> 
> >> Usually we work the other way by checking at configure time and skipping
> >> the feature if the prerequisites are not in place. We do this with gdb:
> >> 
> >>   ../../configure 
> >> --gdb=/home/alex/src/tools/binutils-gdb.git/builds/all/install/bin/gdb
> >> 
> >> which checks gdb is at least new enough to support the features we need:
> >> 
> >>   if test -n "$gdb_bin"; then
> >>       gdb_version=$($gdb_bin --version | head -n 1)
> >>       if version_ge ${gdb_version##* } 9.1; then
> >>           echo "HAVE_GDB_BIN=$gdb_bin" >> $config_host_mak
> >>           gdb_arches=$("$source_path/scripts/probe-gdb-support.py" 
> >> $gdb_bin)
> >>       else
> >>           gdb_bin=""
> >>       fi
> >>   fi
> >> 
> >> >>>>> - Sometimes, the stock iasl that comes with distributions is simply 
> >> >>>>> not good
> >> >>>>>  enough because it does not support the latest ACPI changes - newly
> >> >>>>>  introduced tables or new table attributes etc. In order to test 
> >> >>>>> ACPI code
> >> >>>>>  in QEMU, one has to clone the latest acpica upstream repository and
> >> >>>>>  rebuild iasl in order to get support for it. In those cases, one 
> >> >>>>> may want
> >> >>>>>  the test to use the iasl binary from a non-standard location.
> >> 
> >> I think configure should be checking if iasl is new enough and reporting
> >> to the user at configure time they need to do something different. We
> >> don't want to attempt to run tests that will fail unless the user has
> >> added the right magic to their environment.
> >
> > iasl is a disassembler we trigger for user convenience in case tests
> > fail. It will never cause tests to fail.
> 
> Fair enough. But I still think the place to report it is in configure.
> Maybe something like:
> 
>     iasl                         : /usr/bin/iasl (version 20200925, might not 
> handle all ACPI)           
> 
> in the Host Binaries section. Re-configuring shouldn't cause too much of
> the build to be regenerated although we could certainly do better in
> this regard.

won't all of it be regenerated? a header everyone includes changes.

-- 
MST