diff mbox series

[v2,02/10] meson: add BUILDING_RTE_SDK

Message ID 20190613142344.9188-3-nhorman@tuxdriver.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series dpdk: introduce __rte_internal tag |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/Intel-compilation fail Compilation issues

Commit Message

Neil Horman June 13, 2019, 2:23 p.m. UTC 
  The __rte_internal macro is defined dependent on the value of the build
environment variable BUILDING_RTE_SDK.  This variable was set in the
Makefile environment but not the meson environment, so lets reconcile
the two by defining it for meson in the lib and drivers directories, but
not the examples/apps directories, which should be treated as they are
not part of the core DPDK library

Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
CC: Bruce Richardson <bruce.richardson@intel.com>
CC: Thomas Monjalon <thomas@monjalon.net>
---
 drivers/meson.build | 1 +
 lib/meson.build     | 1 +
 2 files changed, 2 insertions(+)

Comments

Bruce Richardson June 13, 2019, 2:44 p.m. UTC | #1 
On Thu, Jun 13, 2019 at 10:23:36AM -0400, Neil Horman wrote:
> The __rte_internal macro is defined dependent on the value of the build
> environment variable BUILDING_RTE_SDK.  This variable was set in the
> Makefile environment but not the meson environment, so lets reconcile
> the two by defining it for meson in the lib and drivers directories, but
> not the examples/apps directories, which should be treated as they are
> not part of the core DPDK library
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> CC: Bruce Richardson <bruce.richardson@intel.com>
> CC: Thomas Monjalon <thomas@monjalon.net>
> ---
>  drivers/meson.build | 1 +
>  lib/meson.build     | 1 +
>  2 files changed, 2 insertions(+)
> 
> diff --git a/drivers/meson.build b/drivers/meson.build
> index 4c444f495..a312277d1 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -23,6 +23,7 @@ endif
>  
>  # specify -D_GNU_SOURCE unconditionally
>  default_cflags += '-D_GNU_SOURCE'
> +default_cflags += '-DBUILDING_RTE_SDK'
>  
>  foreach class:dpdk_driver_classes
>  	drivers = []
> diff --git a/lib/meson.build b/lib/meson.build
> index e067ce5ea..0e398d534 100644
> --- a/lib/meson.build
> +++ b/lib/meson.build
> @@ -35,6 +35,7 @@ if is_windows
>  endif
>  
>  default_cflags = machine_args
> +default_cflags += '-DBUILDING_RTE_SDK'
>  if cc.has_argument('-Wno-format-truncation')
>  	default_cflags += '-Wno-format-truncation'
>  endif

While this will work, it's not something that individual components should
ever need to override so I think using "add_project_arguments()" function
is a better way to add this to the C builds.

add_project_arguments('-DBUILDING_RTE_SDK', language: 'c')

Ref: http://mesonbuild.com/Adding-arguments.html
Ref: http://mesonbuild.com/Reference-manual.html#add_project_arguments

/Bruce
Neil Horman June 19, 2019, 10:39 a.m. UTC | #2 
On Thu, Jun 13, 2019 at 03:44:02PM +0100, Bruce Richardson wrote:
> On Thu, Jun 13, 2019 at 10:23:36AM -0400, Neil Horman wrote:
> > The __rte_internal macro is defined dependent on the value of the build
> > environment variable BUILDING_RTE_SDK.  This variable was set in the
> > Makefile environment but not the meson environment, so lets reconcile
> > the two by defining it for meson in the lib and drivers directories, but
> > not the examples/apps directories, which should be treated as they are
> > not part of the core DPDK library
> > 
> > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > CC: Bruce Richardson <bruce.richardson@intel.com>
> > CC: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >  drivers/meson.build | 1 +
> >  lib/meson.build     | 1 +
> >  2 files changed, 2 insertions(+)
> > 
> > diff --git a/drivers/meson.build b/drivers/meson.build
> > index 4c444f495..a312277d1 100644
> > --- a/drivers/meson.build
> > +++ b/drivers/meson.build
> > @@ -23,6 +23,7 @@ endif
> >  
> >  # specify -D_GNU_SOURCE unconditionally
> >  default_cflags += '-D_GNU_SOURCE'
> > +default_cflags += '-DBUILDING_RTE_SDK'
> >  
> >  foreach class:dpdk_driver_classes
> >  	drivers = []
> > diff --git a/lib/meson.build b/lib/meson.build
> > index e067ce5ea..0e398d534 100644
> > --- a/lib/meson.build
> > +++ b/lib/meson.build
> > @@ -35,6 +35,7 @@ if is_windows
> >  endif
> >  
> >  default_cflags = machine_args
> > +default_cflags += '-DBUILDING_RTE_SDK'
> >  if cc.has_argument('-Wno-format-truncation')
> >  	default_cflags += '-Wno-format-truncation'
> >  endif
> 
> While this will work, it's not something that individual components should
> ever need to override so I think using "add_project_arguments()" function
> is a better way to add this to the C builds.
> 
That sounds like it makes sense to me, but reading the documentation for meson,
I'm not sure I see the behavioral difference.  Can you elaborate on how
add_project_arguments behaves differently here?

Neil

> add_project_arguments('-DBUILDING_RTE_SDK', language: 'c')
> 
> Ref: http://mesonbuild.com/Adding-arguments.html
> Ref: http://mesonbuild.com/Reference-manual.html#add_project_arguments
> 
> /Bruce
>
Bruce Richardson June 19, 2019, 10:46 a.m. UTC | #3 
On Wed, Jun 19, 2019 at 06:39:00AM -0400, Neil Horman wrote:
> On Thu, Jun 13, 2019 at 03:44:02PM +0100, Bruce Richardson wrote:
> > On Thu, Jun 13, 2019 at 10:23:36AM -0400, Neil Horman wrote:
> > > The __rte_internal macro is defined dependent on the value of the build
> > > environment variable BUILDING_RTE_SDK.  This variable was set in the
> > > Makefile environment but not the meson environment, so lets reconcile
> > > the two by defining it for meson in the lib and drivers directories, but
> > > not the examples/apps directories, which should be treated as they are
> > > not part of the core DPDK library
> > > 
> > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > > CC: Bruce Richardson <bruce.richardson@intel.com>
> > > CC: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > >  drivers/meson.build | 1 +
> > >  lib/meson.build     | 1 +
> > >  2 files changed, 2 insertions(+)
> > > 
> > > diff --git a/drivers/meson.build b/drivers/meson.build
> > > index 4c444f495..a312277d1 100644
> > > --- a/drivers/meson.build
> > > +++ b/drivers/meson.build
> > > @@ -23,6 +23,7 @@ endif
> > >  
> > >  # specify -D_GNU_SOURCE unconditionally
> > >  default_cflags += '-D_GNU_SOURCE'
> > > +default_cflags += '-DBUILDING_RTE_SDK'
> > >  
> > >  foreach class:dpdk_driver_classes
> > >  	drivers = []
> > > diff --git a/lib/meson.build b/lib/meson.build
> > > index e067ce5ea..0e398d534 100644
> > > --- a/lib/meson.build
> > > +++ b/lib/meson.build
> > > @@ -35,6 +35,7 @@ if is_windows
> > >  endif
> > >  
> > >  default_cflags = machine_args
> > > +default_cflags += '-DBUILDING_RTE_SDK'
> > >  if cc.has_argument('-Wno-format-truncation')
> > >  	default_cflags += '-Wno-format-truncation'
> > >  endif
> > 
> > While this will work, it's not something that individual components should
> > ever need to override so I think using "add_project_arguments()" function
> > is a better way to add this to the C builds.
> > 
> That sounds like it makes sense to me, but reading the documentation for meson,
> I'm not sure I see the behavioral difference.  Can you elaborate on how
> add_project_arguments behaves differently here?
> 
The end result should be the same. The key differences are:
1. Only ever needs to be set in one place
2. Cannot be overridden by the individual objects in the build.

So it's just slightly cleaner. If you prefer your existing approach, I'm ok
with that.

/Bruce
Neil Horman June 19, 2019, 6:34 p.m. UTC | #4 
On Wed, Jun 19, 2019 at 11:46:12AM +0100, Bruce Richardson wrote:
> On Wed, Jun 19, 2019 at 06:39:00AM -0400, Neil Horman wrote:
> > On Thu, Jun 13, 2019 at 03:44:02PM +0100, Bruce Richardson wrote:
> > > On Thu, Jun 13, 2019 at 10:23:36AM -0400, Neil Horman wrote:
> > > > The __rte_internal macro is defined dependent on the value of the build
> > > > environment variable BUILDING_RTE_SDK.  This variable was set in the
> > > > Makefile environment but not the meson environment, so lets reconcile
> > > > the two by defining it for meson in the lib and drivers directories, but
> > > > not the examples/apps directories, which should be treated as they are
> > > > not part of the core DPDK library
> > > > 
> > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > > > CC: Bruce Richardson <bruce.richardson@intel.com>
> > > > CC: Thomas Monjalon <thomas@monjalon.net>
> > > > ---
> > > >  drivers/meson.build | 1 +
> > > >  lib/meson.build     | 1 +
> > > >  2 files changed, 2 insertions(+)
> > > > 
> > > > diff --git a/drivers/meson.build b/drivers/meson.build
> > > > index 4c444f495..a312277d1 100644
> > > > --- a/drivers/meson.build
> > > > +++ b/drivers/meson.build
> > > > @@ -23,6 +23,7 @@ endif
> > > >  
> > > >  # specify -D_GNU_SOURCE unconditionally
> > > >  default_cflags += '-D_GNU_SOURCE'
> > > > +default_cflags += '-DBUILDING_RTE_SDK'
> > > >  
> > > >  foreach class:dpdk_driver_classes
> > > >  	drivers = []
> > > > diff --git a/lib/meson.build b/lib/meson.build
> > > > index e067ce5ea..0e398d534 100644
> > > > --- a/lib/meson.build
> > > > +++ b/lib/meson.build
> > > > @@ -35,6 +35,7 @@ if is_windows
> > > >  endif
> > > >  
> > > >  default_cflags = machine_args
> > > > +default_cflags += '-DBUILDING_RTE_SDK'
> > > >  if cc.has_argument('-Wno-format-truncation')
> > > >  	default_cflags += '-Wno-format-truncation'
> > > >  endif
> > > 
> > > While this will work, it's not something that individual components should
> > > ever need to override so I think using "add_project_arguments()" function
> > > is a better way to add this to the C builds.
> > > 
> > That sounds like it makes sense to me, but reading the documentation for meson,
> > I'm not sure I see the behavioral difference.  Can you elaborate on how
> > add_project_arguments behaves differently here?
> > 
> The end result should be the same. The key differences are:
> 1. Only ever needs to be set in one place
> 2. Cannot be overridden by the individual objects in the build.
> 
> So it's just slightly cleaner. If you prefer your existing approach, I'm ok
> with that.
> 

No, I like the aspect of item 2, though, just for clarification, do we really
only need to set it at the top level?  I ask because it seems that
BUILDING_RTE_SDK should be set for the lib and drivers subtrees, but not the app
subtree, as that tree, being the location of our example code, should be treated
as non-core dpdk libraries, and so should throw an error if any code there
attempts to use a dpdk internal only function.  Or is there some other magic
afoot in that subtree?

Neil

> /Bruce
>
Bruce Richardson June 20, 2019, 10:20 a.m. UTC | #5 
On Wed, Jun 19, 2019 at 02:34:43PM -0400, Neil Horman wrote:
> On Wed, Jun 19, 2019 at 11:46:12AM +0100, Bruce Richardson wrote:
> > On Wed, Jun 19, 2019 at 06:39:00AM -0400, Neil Horman wrote:
> > > On Thu, Jun 13, 2019 at 03:44:02PM +0100, Bruce Richardson wrote:
> > > > On Thu, Jun 13, 2019 at 10:23:36AM -0400, Neil Horman wrote:
> > > > > The __rte_internal macro is defined dependent on the value of the build
> > > > > environment variable BUILDING_RTE_SDK.  This variable was set in the
> > > > > Makefile environment but not the meson environment, so lets reconcile
> > > > > the two by defining it for meson in the lib and drivers directories, but
> > > > > not the examples/apps directories, which should be treated as they are
> > > > > not part of the core DPDK library
> > > > > 
> > > > > Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> > > > > CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> > > > > CC: Bruce Richardson <bruce.richardson@intel.com>
> > > > > CC: Thomas Monjalon <thomas@monjalon.net>
> > > > > ---
> > > > >  drivers/meson.build | 1 +
> > > > >  lib/meson.build     | 1 +
> > > > >  2 files changed, 2 insertions(+)
> > > > > 
> > > > > diff --git a/drivers/meson.build b/drivers/meson.build
> > > > > index 4c444f495..a312277d1 100644
> > > > > --- a/drivers/meson.build
> > > > > +++ b/drivers/meson.build
> > > > > @@ -23,6 +23,7 @@ endif
> > > > >  
> > > > >  # specify -D_GNU_SOURCE unconditionally
> > > > >  default_cflags += '-D_GNU_SOURCE'
> > > > > +default_cflags += '-DBUILDING_RTE_SDK'
> > > > >  
> > > > >  foreach class:dpdk_driver_classes
> > > > >  	drivers = []
> > > > > diff --git a/lib/meson.build b/lib/meson.build
> > > > > index e067ce5ea..0e398d534 100644
> > > > > --- a/lib/meson.build
> > > > > +++ b/lib/meson.build
> > > > > @@ -35,6 +35,7 @@ if is_windows
> > > > >  endif
> > > > >  
> > > > >  default_cflags = machine_args
> > > > > +default_cflags += '-DBUILDING_RTE_SDK'
> > > > >  if cc.has_argument('-Wno-format-truncation')
> > > > >  	default_cflags += '-Wno-format-truncation'
> > > > >  endif
> > > > 
> > > > While this will work, it's not something that individual components should
> > > > ever need to override so I think using "add_project_arguments()" function
> > > > is a better way to add this to the C builds.
> > > > 
> > > That sounds like it makes sense to me, but reading the documentation for meson,
> > > I'm not sure I see the behavioral difference.  Can you elaborate on how
> > > add_project_arguments behaves differently here?
> > > 
> > The end result should be the same. The key differences are:
> > 1. Only ever needs to be set in one place
> > 2. Cannot be overridden by the individual objects in the build.
> > 
> > So it's just slightly cleaner. If you prefer your existing approach, I'm ok
> > with that.
> > 
> 
> No, I like the aspect of item 2, though, just for clarification, do we really
> only need to set it at the top level?  I ask because it seems that
> BUILDING_RTE_SDK should be set for the lib and drivers subtrees, but not the app
> subtree, as that tree, being the location of our example code, should be treated
> as non-core dpdk libraries, and so should throw an error if any code there
> attempts to use a dpdk internal only function.  Or is there some other magic
> afoot in that subtree?
> 
Good point. Keep your original patch as-is so.

/Bruce
Bruce Richardson June 20, 2019, 10:21 a.m. UTC | #6 
On Thu, Jun 13, 2019 at 10:23:36AM -0400, Neil Horman wrote:
> The __rte_internal macro is defined dependent on the value of the build
> environment variable BUILDING_RTE_SDK.  This variable was set in the
> Makefile environment but not the meson environment, so lets reconcile
> the two by defining it for meson in the lib and drivers directories, but
> not the examples/apps directories, which should be treated as they are
> not part of the core DPDK library
> 
> Signed-off-by: Neil Horman <nhorman@tuxdriver.com>
> CC: Jerin Jacob Kollanukkaran <jerinj@marvell.com>
> CC: Bruce Richardson <bruce.richardson@intel.com>
> CC: Thomas Monjalon <thomas@monjalon.net>

Acked-by: Bruce Richardson <bruce.richardson@intel.com>
diff mbox series

Patch

diff --git a/drivers/meson.build b/drivers/meson.build
index 4c444f495..a312277d1 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -23,6 +23,7 @@  endif
 
 # specify -D_GNU_SOURCE unconditionally
 default_cflags += '-D_GNU_SOURCE'
+default_cflags += '-DBUILDING_RTE_SDK'
 
 foreach class:dpdk_driver_classes
 	drivers = []
diff --git a/lib/meson.build b/lib/meson.build
index e067ce5ea..0e398d534 100644
--- a/lib/meson.build
+++ b/lib/meson.build
@@ -35,6 +35,7 @@  if is_windows
 endif
 
 default_cflags = machine_args
+default_cflags += '-DBUILDING_RTE_SDK'
 if cc.has_argument('-Wno-format-truncation')
 	default_cflags += '-Wno-format-truncation'
 endif