Re: [PATCH] meson: Fix MESONINTROSPECT parsing

[Akihiko Odaki <akihiko.odaki@daynix.com>]

On 2023/08/12 17:01, Michael Tokarev wrote:
> 12.08.2023 09:15, Akihiko Odaki wrote:
>> The arguments in MESONINTROSPECT are quoted with shlex.quote() so it
>> must be parsed with shlex.split().
>>
>> Fixes: cf60ccc330 ("cutils: Introduce bundle mechanism")
>> Reported-by: Michael Tokarev <mjt@tls.msk.ru>
>> Signed-off-by: Akihiko Odaki <akihiko.odaki@daynix.com>
>> ---
>>   scripts/symlink-install-tree.py | 3 ++-
>>   1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/scripts/symlink-install-tree.py 
>> b/scripts/symlink-install-tree.py
>> index 8ed97e3c94..b72563895c 100644
>> --- a/scripts/symlink-install-tree.py
>> +++ b/scripts/symlink-install-tree.py
>> @@ -4,6 +4,7 @@
>>   import errno
>>   import json
>>   import os
>> +import shlex
>>   import subprocess
>>   import sys
>> @@ -14,7 +15,7 @@ def destdir_join(d1: str, d2: str) -> str:
>>       return str(PurePath(d1, *PurePath(d2).parts[1:]))
>>   introspect = os.environ.get('MESONINTROSPECT')
>> -out = subprocess.run([*introspect.split(' '), '--installed'],
>> +out = subprocess.run([*shlex.split(introspect), '--installed'],
>>                        stdout=subprocess.PIPE, check=True).stdout
>>   for source, dest in json.loads(out).items():
>>       bundle_dest = destdir_join('qemu-bundle', dest)
> 
> This fixes one of the two issues, - the script is being run
> now without failures.
> 
> Reviewed-by: Michael Tokarev <mjt@tls.msk.ru>
> Tested-by: Michael Tokarev <mjt@tls.msk.ru>
> 
> There's one more possible problem which is worth to fix, I'd say:
> it is the fact that script failure is not detected in any way.
> Shouldn't subprocess.run raise an exception in case of failure?
> I think it needs check=True (since python 3.5 iirc).

I missed that you noted this failure is not detected by configure. It is 
certainly better to fix.

It does have check=True but it's rather a obfuscated way to say that 
when you can just use subprocess.check_output(). I sent another patch to 
use subprocess.check_output().

The reason why configure does not detect the failure is that Meson 
ignores postconf script failures. I opened a pull request upstream:
https://github.com/mesonbuild/meson/pull/12115

Regards,
Akihiko Odaki