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

["Alex Bennée" <alex.bennee@linaro.org>]

Ani Sinha <anisinha@redhat.com> writes:

>> On 17-May-2023, at 8:46 PM, Alex Bennée <alex.bennee@linaro.org> 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.
>
> I think this is too hard a requirement for something that is only used
> for debugging aid. If iasl is absent, no big deal. We won’t get nice
> asl diffs but the test will fail and let the users know which blobs
> are in disagreement. For ACPI contributors and maintainers, in the
> previous incarnation, one could quickly install iasl and rerun the
> same bios-tables-test and get the asl diffs. Now we need an additional
> step of recompiling QEMU which is completely useless, confusing and
> burdensome.

That sounds like a bug in the build system - for example:

  ../../configure --disable-docs --target-list=aarch64-softmmu,arm-softmmu,aarch64-linux-user,arm-linux-user,aarch64_be-linux-user --gdb=$HOME/src/tools/binutils-gdb.git/builds/all/install/bin/gdb
  make -j9
  ... does complete build ...
  ../../configure --disable-docs --target-list=aarch64-softmmu,arm-softmmu,aarch64-linux-user,arm-linux-user,aarch64_be-linux-user
  make -j9
  ... regenerated 23 files ...

It certainly doesn't recompile all of QEMU and I'm sure some of the
header re generations the build system could be smarter about.

It's certainly not without precedent to use getenv() in tests, there are
a number of well defined cases. But if you are going to add it at least
document it docs/devel/testing.rst otherwise you will be the only person
who knows of this particular magic variable.

-- 
Alex Bennée
Virtualisation Tech Lead @ Linaro