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

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

[Ric Li]

When running 'meson build' in MSYS2,
"list-dir-globs.py * failed with status 1".

Signed-off-by: Ric Li <ricmli@outlook.com>
---
 app/meson.build | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/app/meson.build b/app/meson.build
index e4bf5c531c..73e5138301 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -11,7 +11,7 @@ disable_apps = run_command(list_dir_globs, disable_apps, check: true).stdout().s
 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()
+    enable_apps = run_command(list_dir_globs, '*/', check: true).stdout().split()
 endif
 
 apps = [
-- 
2.42.0

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

[Bruce Richardson]

On Sat, Sep 16, 2023 at 09:15:19PM +0800, Ric Li wrote:
> When running 'meson build' in MSYS2,
> "list-dir-globs.py * failed with status 1".
> 
> Signed-off-by: Ric Li <ricmli@outlook.com>
> ---
>  app/meson.build | 2 +-
>  1 file changed, 1 insertion(+), 1 deletion(-)
> 
> diff --git a/app/meson.build b/app/meson.build
> index e4bf5c531c..73e5138301 100644
> --- a/app/meson.build
> +++ b/app/meson.build
> @@ -11,7 +11,7 @@ disable_apps = run_command(list_dir_globs, disable_apps, check: true).stdout().s
>  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()
> +    enable_apps = run_command(list_dir_globs, '*/', check: true).stdout().split()
>  endif
>  

Do we know more about why this particular failure is happening with MSYS2?
Can you try running the script manually to see what the specific python
error is, and if we can make the script more robust generally?

In terms of the fix, I actually think we should not be using a glob here at
all. Since we already have the list of apps present in the file, I think
that we should move the app list to the top of the file and then change the
code to be:

if enable_apps.length() == 0
    enable_apps = apps
endif

This sidesteps any issues with globbing, and also makes the code a bit
faster as we don't have to shell-out to a python script.

/Bruce

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

[Ric Li]

On 2023/9/19 16:12, Bruce Richardson wrote:
> On Sat, Sep 16, 2023 at 09:15:19PM +0800, Ric Li wrote:
>> When running 'meson build' in MSYS2,
>> "list-dir-globs.py * failed with status 1".
>>
>> Signed-off-by: Ric Li <ricmli@outlook.com>
>> ---
>>  app/meson.build | 2 +-
>>  1 file changed, 1 insertion(+), 1 deletion(-)
>>
>> diff --git a/app/meson.build b/app/meson.build
>> index e4bf5c531c..73e5138301 100644
>> --- a/app/meson.build
>> +++ b/app/meson.build
>> @@ -11,7 +11,7 @@ disable_apps = run_command(list_dir_globs, disable_apps, check: true).stdout().s
>>  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()
>> +    enable_apps = run_command(list_dir_globs, '*/', check: true).stdout().split()
>>  endif
>>  
> 
> Do we know more about why this particular failure is happening with MSYS2?
> Can you try running the script manually to see what the specific python
> error is, and if we can make the script more robust generally?
> 

Running the script manually showed nothing but the Usage log.
The arguments here are not accepted by this python script.

MSYS2 does mention some command line parsing issues, see:
https://www.msys2.org/wiki/Porting/
"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."

> In terms of the fix, I actually think we should not be using a glob here at
> all. Since we already have the list of apps present in the file, I think
> that we should move the app list to the top of the file and then change the
> code to be:
> 
> if enable_apps.length() == 0
>     enable_apps = apps
> endif
> 
> This sidesteps any issues with globbing, and also makes the code a bit
> faster as we don't have to shell-out to a python script.
> 
> /Bruce

That sounds reasonable. I'll test it and provide an update to the patch.

Thanks,
Ric

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

[Ric Li]

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(-)

diff --git a/app/meson.build b/app/meson.build
index e4bf5c531c..75ac37bef9 100644
--- a/app/meson.build
+++ b/app/meson.build
@@ -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
-- 
2.42.0

[PATCH v2 2/2] doc: add MSYS2 building guide

[Ric Li]

Introduce guide for using MSYS2 to build DPDK on Windows.
MSYS2 provides a Unix-like environment on Windows and
its package manager simplifies tool and dependency
installation, aiding Linux program migration.

Signed-off-by: Ric Li <ricmli@outlook.com>
---
 doc/guides/windows_gsg/build_dpdk.rst | 36 +++++++++++++++++++++++++++
 1 file changed, 36 insertions(+)

diff --git a/doc/guides/windows_gsg/build_dpdk.rst b/doc/guides/windows_gsg/build_dpdk.rst
index 29f2b38feb..27dff76efe 100644
--- a/doc/guides/windows_gsg/build_dpdk.rst
+++ b/doc/guides/windows_gsg/build_dpdk.rst
@@ -12,6 +12,7 @@ environments:
 
 * The Clang-LLVM C compiler and Microsoft MSVC linker.
 * The MinGW-w64 toolchain (either native or cross).
+* MSYS2 with the MINGW64/UCRT64 environment.
 
 The Meson Build system is used to prepare the sources for compilation
 with the Ninja backend.
@@ -55,6 +56,39 @@ Install to a folder without spaces in its name, like ``C:\MinGW``.
 This path is assumed for the rest of this guide.
 
 
+Option 3. MSYS2 with the MINGW64/UCRT64 Environment
+---------------------------------------------------
+
+Install MSYS2
+~~~~~~~~~~~~~
+
+Download and install MSYS2 from
+`MSYS2 website <https://www.msys2.org>`_.
+
+Follow the installation instructions provided on the MSYS2 website
+to set up the base system.
+Make sure to update the package database using the
+following commands in the MSYS2 terminal:
+
+.. code-block:: console
+
+    pacman -Syu
+
+Install Build Dependencies
+~~~~~~~~~~~~~~~~~~~~~~~~~~
+
+Open the MSYS2 MINGW64/UCRT64 terminal and use the package manager
+to install the required build tools and dependencies:
+
+.. code-block:: console
+
+    pacman -S git pactoys
+    pacboy -S meson:p gcc:p pkg-config:p
+
+Meson and Ninja are already installed via the package manager,
+so you can proceed with building the code.
+
+
 Install the Build System
 ------------------------
 
@@ -99,6 +133,8 @@ When using MinGW-w64, it is sufficient to have toolchain executables in PATH:
 
     set PATH=C:\MinGW\mingw64\bin;%PATH%
 
+When using MSYS2, perform in the MSYS2 MINGW64/UCRT64 terminal.
+
 To compile the examples, the flag ``-Dexamples`` is required.
 
 .. code-block:: console
-- 
2.42.0

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

[Bruce Richardson]

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>

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

[Thomas Monjalon]

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?

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

[Bruce Richardson]

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

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

[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

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

[Thomas Monjalon]

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
>