DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Andre Muezerie <andremue@linux.microsoft.com>
Cc: <dev@dpdk.org>
Subject: Re: [PATCH] buildtools: allow a different minimum meson version for Windows
Date: Mon, 16 Jun 2025 08:36:31 +0100	[thread overview]
Message-ID: <aE_Jf9p5HY17jc1M@bricha3-mobl1.ger.corp.intel.com> (raw)
In-Reply-To: <1749831278-8286-1-git-send-email-andremue@linux.microsoft.com>

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

/Bruce

  reply	other threads:[~2025-06-16  7:36 UTC|newest]

Thread overview: 7+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2025-06-13 16:14 Andre Muezerie
2025-06-16  7:36 ` Bruce Richardson [this message]
2025-06-16  8:27   ` Bruce Richardson
2025-06-16 15:09     ` Andre Muezerie
2025-06-16 15:16       ` Bruce Richardson
2025-06-16 15:31 ` [PATCH v2] " Andre Muezerie
2025-06-16 15:38   ` Bruce Richardson

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=aE_Jf9p5HY17jc1M@bricha3-mobl1.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=andremue@linux.microsoft.com \
    --cc=dev@dpdk.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for NNTP newsgroup(s).