Re: [PATCH v2 1/2] build: fix list_dir_globs failure in MSYS2

[Ric Li <ricmli@outlook.com>]

On 2023/10/11 23:34, Bruce Richardson wrote:
> On Wed, Oct 11, 2023 at 05:27:22PM +0200, Thomas Monjalon wrote:
>> 20/09/2023 16:18, Ric Li:
>>> When running 'meson setup' on Windows with MSYS2,
>>> "list-dir-globs.py * failed with status 1".
>>
>> We don't know why it is failing?
>> What about other usages of list_dir_globs in drivers and lib?

Looks lile MSYS2 shell expands this wildcard automatically
before passing it to the child process.

I print the args in list-dir-globs.py and found that args are
the expanded dir names, so the len(sys.argv) is larger than 2,
which makes this script fail. The '*/*' arg in drivers/meson.build
works well just as expected, and no '*' used in lib.

This is from MSYS2 documentation:
"Windows programs parse the command line themselves, it isn't parsed for them by
the calling process, as on Linux. This means that if wildcards (glob patterns) are
to be accepted by the program, it has to be able to expand them somehow. MinGW-w64
supplies the correct start-up code, so it happens automatically, in a manner
compatible with MSVC-compiled programs. If undesirable, the behavior can be
disabled at program build."

I think this fix is not needed if we can find a way to disable the auto-expanding
behaviour of the MSYS2 program. I've tried the runtime way by setting
"MSYS=noglob" envvar but not working here...

>>
>>> Avoid using globbing to get components for app build
>>> since they are already listed in the meson file.
>>
>> I don't understand the logic.
>>
>>> +disable_apps = ',' + get_option('disable_apps')
>>> +disable_apps = run_command(list_dir_globs, disable_apps, check: true).stdout().split()
>>
>> This could fail.>>
>>> +
>>> +enable_apps = ',' + get_option('enable_apps')
>>> +enable_apps = run_command(list_dir_globs, enable_apps, check: true).stdout().split()
>>> +if enable_apps.length() == 0
>>> +    enable_apps = apps
>>> +endif
>>
>> If nothing is enabled, we enable all?
>>
> Yes, if the enable_apps list is empty we should enable everything.
> However, on reviewing the v2, I missed the fact that this patch is
> removing the expansion of the disable_apps value.> 
> Given your comment, this check can probably also be improved by checking
> the get_option('enable_apps') length, rather than the expanded version.
> 
> /Bruce