Re: [PATCH] buildtools: allow a different minimum meson version for Windows

[Bruce Richardson <bruce.richardson@intel.com>]

On Mon, Jun 16, 2025 at 08:36:31AM +0100, Bruce Richardson wrote:
> On Fri, Jun 13, 2025 at 09:14:38AM -0700, Andre Muezerie wrote:
> > There is a minimum meson version specified in the DPDK meson project
> > section, which has been documented. This string is parsed by
> > buildtools\get-min-meson-version.py and this information is used by
> > lab automation to install the corresponding meson package on the
> > build machine.
> > 
> > Turns out that the meson version specified on the DPDK project
> > (0.57.x) is buggy on Windows: it has issues related to path
> > manipulation, and these issues are causing failures. Therefore,
> > a newer meson is required on Windows.
> > 
> > To avoid bringing the minimum requirements up for all operating
> > systems before a more appropriate release, this patch implements
> > a mechanism allowing a different version to be specified for Windows.
> > 
> > Signed-off-by: Andre Muezerie <andremue@linux.microsoft.com>
> 
> I am fine with the principle, few comments inline below just on the
> specific implementation.
> 
> /Bruce
> 
> > ---
> >  buildtools/get-min-meson-version.py | 12 ++++++++++--
> >  meson.build                         |  9 +++++++++
> >  2 files changed, 19 insertions(+), 2 deletions(-)
> > 
> > diff --git a/buildtools/get-min-meson-version.py b/buildtools/get-min-meson-version.py
> > index f0de8fdf2b..10a2596a5a 100755
> > --- a/buildtools/get-min-meson-version.py
> > +++ b/buildtools/get-min-meson-version.py
> > @@ -11,12 +11,19 @@
> >  basedir = dirname(buildtools_path)
> >  
> >  with open(join(basedir, "meson.build")) as f:
> > +    if os.name == "nt":
> > +        keyword = "windows_meson_version"
> > +        pattern = r"windows_meson_version:\s*'>=\s*([0-9\.]+)'"
> > +    else:
> > +        keyword = "meson_version"
> > +        pattern = r"meson_version:\s*'>=\s*([0-9\.]+)'"
> > +
> 
> Minor nit, but I'd suggest shortening this a little to:
> 
>     keyword = "meson_version"
>     if os.name == "nt":
>         keyword = "windows_" + keyword
>     pattern = fr"{keyword}:\s*'>=\s*([0-9\.]+)'"
> 
> or even further shortening gives:
> 
>     keyword = "windows_meson_version" if os.name == "nt" else "meson_version"
>     pattern = fr"{keyword}:\s*'>=\s*([0-9\.]+)'"
> 
> >      for ln in f.readlines():
> > -        if "meson_version" not in ln:
> > +        if keyword not in ln:
> >              continue
> >  
> >          ln = ln.strip()
> > -        ver_match = re.search(r"meson_version:\s*'>=\s*([0-9\.]+)'", ln)
> > +        ver_match = re.search(pattern, ln)
> >          if not ver_match:
> >              print(
> >                  f"Meson version specifier not in recognised format: '{ln}'",
> > @@ -24,3 +31,4 @@
> >              )
> >              sys.exit(1)
> >          print(ver_match.group(1), end="")
> > +        break
> > diff --git a/meson.build b/meson.build
> > index 5ff68cb3af..1ae51996be 100644
> > --- a/meson.build
> > +++ b/meson.build
> > @@ -13,6 +13,15 @@ project('DPDK', 'c',
> >          meson_version: '>= 0.57.2'
> >  )
> >  
> > +# Meson does not support dynamic/conditional values for meson_version in
> > +# the project section.
> > +# The line below contains the minimum meson version required on Windows,
> > +# which gets parsed by get-min-meson-version.py.
> > +# It is important to keep this line below the "project" section, so that
> > +# the script can make the correct determination according to the operating
> > +# system.
> > +# windows_meson_version: '>= 1.5.2'
> > +
> 
> Why not pick a string that is not a superset of the general meson_version
> one? That would avoid the need to position it below the project one, and
> save you having to document that in a long comment. (also avoid the need to
> add the "break" into the python script, not that that is a big deal!)
> For example, how about using:
> 
> * windows_min_meson
> * windows_meson_ver
> * windows meson_min_ver
> 
Or even: "meson_version_windows", since the rest of the pattern we look for
occurs after this string.