diff mbox series

[v3,1/3] common/mlx5: get Windows dependency from standard variables

Message ID 20230302132150.3330288-2-thomas@monjalon.net (mailing list archive)
State Accepted, archived
Delegated to: Raslan Darawsheh
Headers
Series mlx5: fix Windows build with Linux MinGW |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Thomas Monjalon March 2, 2023, 1:21 p.m. UTC 
  The DevX library path had to be provided through the variables
DEVX_INC_PATH and DEVX_LIB_PATH.
It was non-standard and triggers some issues with recent Meson.

Using CFLAGS/LDFLAGS is standard and simpler.
It is also possible to use the Meson options -Dc_args and -Dc_link_args.
There are 2 options to provide:
	-I<include_directory>
	-L<library_directory>

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 doc/guides/platform/mlx5.rst            | 11 +++++-----
 drivers/common/mlx5/windows/meson.build | 28 ++++++++++++-------------
 2 files changed, 20 insertions(+), 19 deletions(-)

Comments

Tyler Retzlaff March 2, 2023, 5:17 p.m. UTC | #1 
On Thu, Mar 02, 2023 at 02:21:48PM +0100, Thomas Monjalon wrote:
> The DevX library path had to be provided through the variables
> DEVX_INC_PATH and DEVX_LIB_PATH.
> It was non-standard and triggers some issues with recent Meson.

is it possible for meson to search the default install location that the
devx sdk installation is normally located on windows? then only if you
installed it to some silly non-default location you have to provide
CFLAGS/LDFLAGS?

> 
> Using CFLAGS/LDFLAGS is standard and simpler.
> It is also possible to use the Meson options -Dc_args and -Dc_link_args.
> There are 2 options to provide:
> 	-I<include_directory>
> 	-L<library_directory>
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---

Acked-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
Thomas Monjalon March 3, 2023, 2:12 p.m. UTC | #2 
02/03/2023 18:17, Tyler Retzlaff:
> On Thu, Mar 02, 2023 at 02:21:48PM +0100, Thomas Monjalon wrote:
> > The DevX library path had to be provided through the variables
> > DEVX_INC_PATH and DEVX_LIB_PATH.
> > It was non-standard and triggers some issues with recent Meson.
> 
> is it possible for meson to search the default install location that the
> devx sdk installation is normally located on windows? then only if you
> installed it to some silly non-default location you have to provide
> CFLAGS/LDFLAGS?

Meson will look into standard system directories I guess,
but DevX is never installed in a system directory.
Which path do you have in mind?
Tyler Retzlaff March 3, 2023, 9:09 p.m. UTC | #3 
On Fri, Mar 03, 2023 at 03:12:19PM +0100, Thomas Monjalon wrote:
> 02/03/2023 18:17, Tyler Retzlaff:
> > On Thu, Mar 02, 2023 at 02:21:48PM +0100, Thomas Monjalon wrote:
> > > The DevX library path had to be provided through the variables
> > > DEVX_INC_PATH and DEVX_LIB_PATH.
> > > It was non-standard and triggers some issues with recent Meson.
> > 
> > is it possible for meson to search the default install location that the
> > devx sdk installation is normally located on windows? then only if you
> > installed it to some silly non-default location you have to provide
> > CFLAGS/LDFLAGS?
> 
> Meson will look into standard system directories I guess,
> but DevX is never installed in a system directory.
> Which path do you have in mind?

i think when i install the sdk it goes here by default if i just mash
the next button. is it wrong to take a look at this path by default?

C:\Program Files\Mellanox\MLNX_WinOF2_DevX_SDK\{inc,lib}


> 
>
Thomas Monjalon March 6, 2023, 8:30 a.m. UTC | #4 
03/03/2023 22:09, Tyler Retzlaff:
> On Fri, Mar 03, 2023 at 03:12:19PM +0100, Thomas Monjalon wrote:
> > 02/03/2023 18:17, Tyler Retzlaff:
> > > On Thu, Mar 02, 2023 at 02:21:48PM +0100, Thomas Monjalon wrote:
> > > > The DevX library path had to be provided through the variables
> > > > DEVX_INC_PATH and DEVX_LIB_PATH.
> > > > It was non-standard and triggers some issues with recent Meson.
> > > 
> > > is it possible for meson to search the default install location that the
> > > devx sdk installation is normally located on windows? then only if you
> > > installed it to some silly non-default location you have to provide
> > > CFLAGS/LDFLAGS?
> > 
> > Meson will look into standard system directories I guess,
> > but DevX is never installed in a system directory.
> > Which path do you have in mind?
> 
> i think when i install the sdk it goes here by default if i just mash
> the next button. is it wrong to take a look at this path by default?
> 
> C:\Program Files\Mellanox\MLNX_WinOF2_DevX_SDK\{inc,lib}

If we have 2 versions in 2 directories,
and we look at the default location, then we can miss the right one.
I'm afraid such facility induce more complications at the end.
Tyler Retzlaff March 6, 2023, 8:55 p.m. UTC | #5 
On Mon, Mar 06, 2023 at 09:30:00AM +0100, Thomas Monjalon wrote:
> 03/03/2023 22:09, Tyler Retzlaff:
> > On Fri, Mar 03, 2023 at 03:12:19PM +0100, Thomas Monjalon wrote:
> > > 02/03/2023 18:17, Tyler Retzlaff:
> > > > On Thu, Mar 02, 2023 at 02:21:48PM +0100, Thomas Monjalon wrote:
> > > > > The DevX library path had to be provided through the variables
> > > > > DEVX_INC_PATH and DEVX_LIB_PATH.
> > > > > It was non-standard and triggers some issues with recent Meson.
> > > > 
> > > > is it possible for meson to search the default install location that the
> > > > devx sdk installation is normally located on windows? then only if you
> > > > installed it to some silly non-default location you have to provide
> > > > CFLAGS/LDFLAGS?
> > > 
> > > Meson will look into standard system directories I guess,
> > > but DevX is never installed in a system directory.
> > > Which path do you have in mind?
> > 
> > i think when i install the sdk it goes here by default if i just mash
> > the next button. is it wrong to take a look at this path by default?
> > 
> > C:\Program Files\Mellanox\MLNX_WinOF2_DevX_SDK\{inc,lib}
> 
> If we have 2 versions in 2 directories,
> and we look at the default location, then we can miss the right one.
> I'm afraid such facility induce more complications at the end.

oh, didn't realize you can have multiple installations. suppose you
could "pick the newest" but maybe that's not terribly sensible.
Tal Shnaiderman March 8, 2023, 10:24 a.m. UTC | #6 
> Subject: [PATCH v3 1/3] common/mlx5: get Windows dependency from
> standard variables
> 
> External email: Use caution opening links or attachments
> 
> 
> The DevX library path had to be provided through the variables
> DEVX_INC_PATH and DEVX_LIB_PATH.
> It was non-standard and triggers some issues with recent Meson.
> 
> Using CFLAGS/LDFLAGS is standard and simpler.
> It is also possible to use the Meson options -Dc_args and -Dc_link_args.
> There are 2 options to provide:
>         -I<include_directory>
>         -L<library_directory>
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  doc/guides/platform/mlx5.rst            | 11 +++++-----
>  drivers/common/mlx5/windows/meson.build | 28 ++++++++++++------------
> -
>  2 files changed, 20 insertions(+), 19 deletions(-)
> 
> diff --git a/doc/guides/platform/mlx5.rst b/doc/guides/platform/mlx5.rst
> index 2d6fbe7e44..5fc5d0cb8c 100644
> --- a/doc/guides/platform/mlx5.rst
> +++ b/doc/guides/platform/mlx5.rst
> @@ -259,13 +259,14 @@ configured by the ``ibverbs_link`` build option:
>  Compilation on Windows
>  ~~~~~~~~~~~~~~~~~~~~~~
> 
> -The DevX SDK location must be set through two environment variables:
> +The DevX SDK location must be set through CFLAGS/LDFLAGS,
> +either::
> 
> -``DEVX_LIB_PATH``
> -   path to the DevX lib file.
> +   meson.exe setup "-Dc_args=-I\"%DEVX_INC_PATH%\"" "-Dc_link_args=-
> L\"%DEVX_LIB_PATH%\"" ...
> 
> -``DEVX_INC_PATH``
> -   path to the DevX header files.
> +or::
> +
> +   set CFLAGS=-I"%DEVX_INC_PATH%" && set LDFLAGS=-
> L"%DEVX_LIB_PATH%" && meson.exe setup ...
> 
> 
>  .. _mlx5_common_env:
> diff --git a/drivers/common/mlx5/windows/meson.build
> b/drivers/common/mlx5/windows/meson.build
> index cc486014a8..f60daed840 100644
> --- a/drivers/common/mlx5/windows/meson.build
> +++ b/drivers/common/mlx5/windows/meson.build
> @@ -1,6 +1,20 @@
>  # SPDX-License-Identifier: BSD-3-Clause  # Copyright 2019 Mellanox
> Technologies, Ltd
> 
> +if not cc.has_header('mlx5devx.h')
> +    build = false
> +    reason = 'missing dependency, "mlx5devx.h"'
> +    subdir_done()
> +endif
> +
> +devxlib = cc.find_library('mlx5devx', required: true) if not
> +devxlib.found()
> +    build = false
> +    reason = 'missing dependency, "mlx5devx"'
> +    subdir_done()
> +endif
> +ext_deps += devxlib
> +
>  includes += include_directories('.')
> 
>  sources += files(
> @@ -8,20 +22,6 @@ sources += files(
>          'mlx5_common_os.c',
>  )
> 
> -res_lib = run_command(python3, '-c', 'import os;
> print(os.environ["DEVX_LIB_PATH"])', check: false) -res_inc =
> run_command(python3, '-c', 'import os;
> print(os.environ["DEVX_INC_PATH"])', check: false)
> -
> -if (res_lib.returncode() != 0 or res_inc.returncode() != 0)
> -    build = false
> -    reason = 'DevX environment variables are not set, DEVX_LIB_PATH and
> DEVX_INC_PATH vars must be exported'
> -    subdir_done()
> -endif
> -
> -devx_lib_dir = res_lib.stdout().strip() -devx_inc_dir =
> res_inc.stdout().strip()
> -
> -ext_deps += cc.find_library('mlx5devx', dirs: devx_lib_dir, required: true) -
> includes += include_directories(devx_inc_dir)  cflags_options = [
>          '-std=c11',
>          '-Wno-strict-prototypes',
> --
> 2.39.1

Acked-by: Tal Shnaiderman <talshn@nvidia.com>
diff mbox series

Patch

diff --git a/doc/guides/platform/mlx5.rst b/doc/guides/platform/mlx5.rst
index 2d6fbe7e44..5fc5d0cb8c 100644
--- a/doc/guides/platform/mlx5.rst
+++ b/doc/guides/platform/mlx5.rst
@@ -259,13 +259,14 @@  configured by the ``ibverbs_link`` build option:
 Compilation on Windows
 ~~~~~~~~~~~~~~~~~~~~~~
 
-The DevX SDK location must be set through two environment variables:
+The DevX SDK location must be set through CFLAGS/LDFLAGS,
+either::
 
-``DEVX_LIB_PATH``
-   path to the DevX lib file.
+   meson.exe setup "-Dc_args=-I\"%DEVX_INC_PATH%\"" "-Dc_link_args=-L\"%DEVX_LIB_PATH%\"" ...
 
-``DEVX_INC_PATH``
-   path to the DevX header files.
+or::
+
+   set CFLAGS=-I"%DEVX_INC_PATH%" && set LDFLAGS=-L"%DEVX_LIB_PATH%" && meson.exe setup ...
 
 
 .. _mlx5_common_env:
diff --git a/drivers/common/mlx5/windows/meson.build b/drivers/common/mlx5/windows/meson.build
index cc486014a8..f60daed840 100644
--- a/drivers/common/mlx5/windows/meson.build
+++ b/drivers/common/mlx5/windows/meson.build
@@ -1,6 +1,20 @@ 
 # SPDX-License-Identifier: BSD-3-Clause
 # Copyright 2019 Mellanox Technologies, Ltd
 
+if not cc.has_header('mlx5devx.h')
+    build = false
+    reason = 'missing dependency, "mlx5devx.h"'
+    subdir_done()
+endif
+
+devxlib = cc.find_library('mlx5devx', required: true)
+if not devxlib.found()
+    build = false
+    reason = 'missing dependency, "mlx5devx"'
+    subdir_done()
+endif
+ext_deps += devxlib
+
 includes += include_directories('.')
 
 sources += files(
@@ -8,20 +22,6 @@  sources += files(
         'mlx5_common_os.c',
 )
 
-res_lib = run_command(python3, '-c', 'import os; print(os.environ["DEVX_LIB_PATH"])', check: false)
-res_inc = run_command(python3, '-c', 'import os; print(os.environ["DEVX_INC_PATH"])', check: false)
-
-if (res_lib.returncode() != 0 or res_inc.returncode() != 0)
-    build = false
-    reason = 'DevX environment variables are not set, DEVX_LIB_PATH and DEVX_INC_PATH vars must be exported'
-    subdir_done()
-endif
-
-devx_lib_dir = res_lib.stdout().strip()
-devx_inc_dir = res_inc.stdout().strip()
-
-ext_deps += cc.find_library('mlx5devx', dirs: devx_lib_dir, required: true)
-includes += include_directories(devx_inc_dir)
 cflags_options = [
         '-std=c11',
         '-Wno-strict-prototypes',