[v2,1/2] build: fix list_dir_globs failure in MSYS2
Checks
Commit Message
When running 'meson setup' on Windows with MSYS2,
"list-dir-globs.py * failed with status 1".
Avoid using globbing to get components for app build
since they are already listed in the meson file.
Signed-off-by: Ric Li <ricmli@outlook.com>
---
app/meson.build | 18 +++++++++---------
1 file changed, 9 insertions(+), 9 deletions(-)
Comments
On Wed, Sep 20, 2023 at 10:18:45PM +0800, Ric Li wrote:
> When running 'meson setup' on Windows with MSYS2,
> "list-dir-globs.py * failed with status 1".
>
> Avoid using globbing to get components for app build
> since they are already listed in the meson file.
>
> Signed-off-by: Ric Li <ricmli@outlook.com>
> ---
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
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?
> 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?
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?
>
> > 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
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
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
>
@@ -5,15 +5,6 @@ if is_ms_compiler
subdir_done()
endif
-disable_apps = ',' + get_option('disable_apps')
-disable_apps = run_command(list_dir_globs, disable_apps, check: true).stdout().split()
-
-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 = run_command(list_dir_globs, '*', check: true).stdout().split()
-endif
-
apps = [
'dumpcap',
'pdump',
@@ -41,6 +32,15 @@ if get_option('tests')
apps += 'test'
endif
+disable_apps = ',' + get_option('disable_apps')
+disable_apps = run_command(list_dir_globs, disable_apps, check: true).stdout().split()
+
+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
+
default_cflags = machine_args + ['-DALLOW_EXPERIMENTAL_API']
default_ldflags = []
if get_option('default_library') == 'static' and not is_windows