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

[Thomas Monjalon <thomas@monjalon.net>]

Any update?
Would you like to provide a v3?


24/10/2023 18:08, Ric Li:
> 
> 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
>