DPDK patches and discussions
 help / color / mirror / Atom feed
From: Bruce Richardson <bruce.richardson@intel.com>
To: Tyler Retzlaff <roretzla@linux.microsoft.com>
Cc: <dev@dpdk.org>, Harman Kalra <hkalra@marvell.com>
Subject: Re: [PATCH v3 1/2] build: build only one library type on Windows
Date: Mon, 15 Apr 2024 10:55:37 +0100	[thread overview]
Message-ID: <Zhz5mc_a3VPm59eJ@bricha3-mobl1.ger.corp.intel.com> (raw)
In-Reply-To: <1712962333-14355-2-git-send-email-roretzla@linux.microsoft.com>

On Fri, Apr 12, 2024 at 03:52:12PM -0700, Tyler Retzlaff wrote:
> MSVC is the only compiler that can produce usable shared libraries for
> DPDK on Windows because of the use of exported TLS variables.
> 
> Disable building of shared libraries with LLVM and MinGW so that
> remaining __declspec macros needed for the functional libraries built by
> MSVC can be used without triggering errors in LLVM and MinGW builds.
> 
> For Windows only install the default_library type to avoid confusion.
> Windows builds cannot build both shared and static in a single pass so
> install only the functional variant.
> 
> Signed-off-by: Tyler Retzlaff <roretzla@linux.microsoft.com>
> ---
>  app/meson.build                       |  6 +++
>  config/meson.build                    | 20 ++++++++++
>  drivers/meson.build                   | 64 +++++++++++++++----------------
>  drivers/net/octeontx/base/meson.build |  2 +-
>  examples/meson.build                  |  6 +++
>  lib/meson.build                       | 72 +++++++++++++++--------------------
>  6 files changed, 94 insertions(+), 76 deletions(-)
> 
<snip>
> -
> -        # create a dependency object and add it to the global dictionary so
> -        # testpmd or other built-in apps can find it if necessary
> -        shared_dep = declare_dependency(link_with: shared_lib,
> -                include_directories: includes,
> -                dependencies: shared_deps)
> +        if is_shared_enabled
> +            shared_lib = shared_library(lib_name, sources,
> +                    objects: objs,
> +                    include_directories: includes,
> +                    dependencies: shared_deps,
> +                    c_args: cflags,
> +                    link_args: lk_args,
> +                    link_depends: lk_deps,
> +                    version: abi_version,
> +                    soversion: so_version,
> +                    install: install_shared,
> +                    install_dir: driver_install_path)
> +
> +            # create a dependency object and add it to the global dictionary so
> +            # testpmd or other built-in apps can find it if necessary
> +            shared_dep = declare_dependency(link_with: shared_lib,
> +                    include_directories: includes,
> +                    dependencies: shared_deps)
> +
> +        else
> +            shared_dep = {}
> +        endif

Very minor nit, but it's generally easier to follow code where you put the
short leg of the condition first, and then the longer leg. Having the short
leg (in this case the one-line) first means the reader has no difficulty
remembering what the original condition was, once they get to the "else".
The longer leg requires more attention and having it last means the reader
doesn't need to worry about maintaining the context of the "if" in memory.
[To a certain extent this is just user-preference, so feel free to ignore
this comment if it's too much work! :-)]

>          static_dep = declare_dependency(
>                  include_directories: includes,
>                  dependencies: static_deps)
> diff --git a/drivers/net/octeontx/base/meson.build b/drivers/net/octeontx/base/meson.build
> index 8e5e8c1..a793c19 100644
> --- a/drivers/net/octeontx/base/meson.build
> +++ b/drivers/net/octeontx/base/meson.build
> @@ -10,7 +10,7 @@ sources = [
>  depends = ['ethdev', 'mempool_octeontx']
>  static_objs = []
>  foreach d: depends
> -    if not is_variable('shared_rte_' + d)
> +    if not is_variable('static_rte_' + d)

Since we have empty objects defined for the shared vars in the no-shared
case, you can drop this change.

>          subdir_done()
>      endif
>      static_objs += get_variable('static_rte_' + d)
<snip>

  reply	other threads:[~2024-04-15  9:55 UTC|newest]

Thread overview: 25+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2024-03-14 19:44 [PATCH] build and install " Tyler Retzlaff
2024-03-14 19:44 ` [PATCH] build: build " Tyler Retzlaff
2024-03-15  6:30 ` [PATCH v2 0/2] build and install " Tyler Retzlaff
2024-03-15  6:30   ` [PATCH v2 1/2] build: build " Tyler Retzlaff
2024-04-12 13:00     ` Bruce Richardson
2024-04-12 23:04       ` Tyler Retzlaff
2024-04-15  9:42         ` Bruce Richardson
2024-03-15  6:30   ` [PATCH v2 2/2] buildtools: when building static library use static deps Tyler Retzlaff
2024-03-15  8:28     ` Bruce Richardson
2024-04-04 18:47       ` Tyler Retzlaff
2024-04-05  9:11         ` Bruce Richardson
2024-04-12 14:09     ` Bruce Richardson
2024-04-12 22:52       ` Tyler Retzlaff
2024-04-12 22:52 ` [PATCH v3 0/2] build and install only one library type on Windows Tyler Retzlaff
2024-04-12 22:52   ` [PATCH v3 1/2] build: build " Tyler Retzlaff
2024-04-15  9:55     ` Bruce Richardson [this message]
2024-04-12 22:52   ` [PATCH v3 2/2] buildtools: when building static library use static deps Tyler Retzlaff
2024-04-15  9:56     ` Bruce Richardson
2024-04-15 16:45 ` [PATCH v4 0/2] build and install only one library type on Windows Tyler Retzlaff
2024-04-15 16:45   ` [PATCH v4 1/2] build: build " Tyler Retzlaff
2024-04-15 16:45   ` [PATCH v4 2/2] buildtools: when building static library use static deps Tyler Retzlaff
2024-04-15 17:12 ` [PATCH v5 0/2] build and install only one library type on Windows Tyler Retzlaff
2024-04-15 17:12   ` [PATCH v5 1/2] build: build " Tyler Retzlaff
2024-04-15 17:24     ` Bruce Richardson
2024-04-15 17:12   ` [PATCH v5 2/2] buildtools: when building static library use static deps Tyler Retzlaff

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=Zhz5mc_a3VPm59eJ@bricha3-mobl1.ger.corp.intel.com \
    --to=bruce.richardson@intel.com \
    --cc=dev@dpdk.org \
    --cc=hkalra@marvell.com \
    --cc=roretzla@linux.microsoft.com \
    /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).