diff mbox series

net/octeontx: meson build fix if octeontx drivers are disabled

Message ID 1581925669-15294-1-git-send-email-agupta3@marvell.com (mailing list archive)
State Superseded, archived
Delegated to: Jerin Jacob
Headers
Series net/octeontx: meson build fix if octeontx drivers are disabled |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/iol-testing success Testing PASS
ci/iol-mellanox-Performance success Performance Testing PASS
ci/travis-robot warning Travis build: failed
ci/Intel-compilation success Compilation OK

Commit Message

Amit Gupta Feb. 17, 2020, 7:47 a.m. UTC 
  From: Amit Gupta <agupta3@marvell.com>

Add a condition to check if octeontx drivers are disabled.
octeontx drivers are built only if dependent drivers i.e.
ethdev, mempool and common/octeontx are enabled.

BugZilla ID # BUG 387

Signed-off-by: Amit Gupta <agupta3@marvell.com>
---
 drivers/net/octeontx/base/meson.build | 32 ++++++++++++++++++++++++--------
 1 file changed, 24 insertions(+), 8 deletions(-)

Comments

David Marchand Feb. 17, 2020, 9:54 a.m. UTC | #1 
On Mon, Feb 17, 2020 at 8:48 AM <agupta3@marvell.com> wrote:
>
> From: Amit Gupta <agupta3@marvell.com>
>
> Add a condition to check if octeontx drivers are disabled.
> octeontx drivers are built only if dependent drivers i.e.
> ethdev, mempool and common/octeontx are enabled.
>
> BugZilla ID # BUG 387

Interesting format, but we prefer consistency, you can see this in git history.
Bugzilla ID: xxx

This can be caught by running the devtools/check-git-log.sh script.


>
> Signed-off-by: Amit Gupta <agupta3@marvell.com>
> ---
>  drivers/net/octeontx/base/meson.build | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
>
> diff --git a/drivers/net/octeontx/base/meson.build b/drivers/net/octeontx/base/meson.build
> index a06a2c8..50e7972 100644
> --- a/drivers/net/octeontx/base/meson.build
> +++ b/drivers/net/octeontx/base/meson.build
> @@ -9,17 +9,33 @@ sources = [
>
>  depends = ['ethdev', 'mempool_octeontx']
>  static_objs = []
> -foreach d: depends
> -       static_objs += [get_variable('static_rte_' + d)]
> +
> +disabled_drivers = get_option('disable_drivers').split(',')
> +
> +build = true
> +foreach disable_path: disabled_drivers
> +        if (('net/octeontx' == disable_path) or
> +          ('event/octeontx' == disable_path) or
> +          ('common/octeontx' == disable_path) or
> +          ('mempool/octeontx' == disable_path))
> +                build = false
> +        endif
>  endforeach

I am no meson expert, but I'd say you can put a subdir_done() if your
check is false and then you don't need to check the build variable
later.
Besides, you can most likely do this check at the top of this file
alongside "depends".

Cc: Bruce who commented on this bz.

>
>  c_args = cflags
>  if allow_experimental_apis
> -       c_args += '-DALLOW_EXPERIMENTAL_API'
> +        c_args += '-DALLOW_EXPERIMENTAL_API'

Unrelated change + whitespace damage after.




>  endif
> -base_lib = static_library('octeontx_base', sources,
> -       c_args: c_args,
> -       dependencies: static_objs,
> -)
>
> -base_objs = base_lib.extract_all_objects()
> +if build
> +        foreach d: depends
> +                static_objs += [get_variable('static_rte_' + d)]
> +        endforeach
> +
> +        base_lib = static_library('octeontx_base', sources,
> +                c_args: c_args,
> +                dependencies: static_objs,
> +        )
> +
> +        base_objs = base_lib.extract_all_objects()
> +endif
> --
> 1.8.3.1
>


--
David Marchand
Bruce Richardson Feb. 17, 2020, 10:48 a.m. UTC | #2 
On Mon, Feb 17, 2020 at 01:17:49PM +0530, agupta3@marvell.com wrote:
> From: Amit Gupta <agupta3@marvell.com>
> 
> Add a condition to check if octeontx drivers are disabled.
> octeontx drivers are built only if dependent drivers i.e.
> ethdev, mempool and common/octeontx are enabled.
> 
> BugZilla ID # BUG 387
> 
> Signed-off-by: Amit Gupta <agupta3@marvell.com>
> ---
>  drivers/net/octeontx/base/meson.build | 32 ++++++++++++++++++++++++--------
>  1 file changed, 24 insertions(+), 8 deletions(-)
> 
> diff --git a/drivers/net/octeontx/base/meson.build b/drivers/net/octeontx/base/meson.build
> index a06a2c8..50e7972 100644
> --- a/drivers/net/octeontx/base/meson.build
> +++ b/drivers/net/octeontx/base/meson.build
> @@ -9,17 +9,33 @@ sources = [
>  
>  depends = ['ethdev', 'mempool_octeontx']
>  static_objs = []
> -foreach d: depends
> -	static_objs += [get_variable('static_rte_' + d)]
> +
> +disabled_drivers = get_option('disable_drivers').split(',')
> +
> +build = true
> +foreach disable_path: disabled_drivers
> +        if (('net/octeontx' == disable_path) or
> +	   ('event/octeontx' == disable_path) or
> +	   ('common/octeontx' == disable_path) or
> +	   ('mempool/octeontx' == disable_path))
> +                build = false
> +        endif
>  endforeach
>  
>  c_args = cflags
>  if allow_experimental_apis
> -	c_args += '-DALLOW_EXPERIMENTAL_API'
> +        c_args += '-DALLOW_EXPERIMENTAL_API'
>  endif
> -base_lib = static_library('octeontx_base', sources,
> -	c_args: c_args,
> -	dependencies: static_objs,
> -)
>  
> -base_objs = base_lib.extract_all_objects()
> +if build
> +        foreach d: depends
> +                static_objs += [get_variable('static_rte_' + d)]
> +        endforeach
> +
> +        base_lib = static_library('octeontx_base', sources,
> +                c_args: c_args,
> +                dependencies: static_objs,
> +        )
> +
> +        base_objs = base_lib.extract_all_objects()
> +endif

A better fix here might be possible using the fact that the get_variable()
call allows passing a fallback value in the case of failure. Therefore
something like the below might work cleaner:

foreach d: depends
	static_obj = [get_variable('static_rte_' + d, '']
	if static_obj == ''
		build = false
		reason = '....'
		subdir_done()
	endif
	static_objs += static_obj
...

Maybe a disabler object could be used also.

Regards,
/Bruce
Amit Gupta Feb. 19, 2020, 4:32 a.m. UTC | #3 
> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Monday, February 17, 2020 4:19 PM
> To: Amit Gupta <agupta3@marvell.com>
> Cc: Harman Kalra <hkalra@marvell.com>; dev@dpdk.org
> Subject: [EXT] Re: [dpdk-dev] [PATCH] net/octeontx: meson build fix if
> octeontx drivers are disabled
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Mon, Feb 17, 2020 at 01:17:49PM +0530, agupta3@marvell.com wrote:
> > From: Amit Gupta <agupta3@marvell.com>
> >
> > Add a condition to check if octeontx drivers are disabled.
> > octeontx drivers are built only if dependent drivers i.e.
> > ethdev, mempool and common/octeontx are enabled.
> >
> > BugZilla ID # BUG 387
> >
> > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > ---
> >  drivers/net/octeontx/base/meson.build | 32
> > ++++++++++++++++++++++++--------
> >  1 file changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/octeontx/base/meson.build
> > b/drivers/net/octeontx/base/meson.build
> > index a06a2c8..50e7972 100644
> > --- a/drivers/net/octeontx/base/meson.build
> > +++ b/drivers/net/octeontx/base/meson.build
> > @@ -9,17 +9,33 @@ sources = [
> >
> >  depends = ['ethdev', 'mempool_octeontx']  static_objs = [] -foreach
> > d: depends
> > -	static_objs += [get_variable('static_rte_' + d)]
> > +
> > +disabled_drivers = get_option('disable_drivers').split(',')
> > +
> > +build = true
> > +foreach disable_path: disabled_drivers
> > +        if (('net/octeontx' == disable_path) or
> > +	   ('event/octeontx' == disable_path) or
> > +	   ('common/octeontx' == disable_path) or
> > +	   ('mempool/octeontx' == disable_path))
> > +                build = false
> > +        endif
> >  endforeach
> >
> >  c_args = cflags
> >  if allow_experimental_apis
> > -	c_args += '-DALLOW_EXPERIMENTAL_API'
> > +        c_args += '-DALLOW_EXPERIMENTAL_API'
> >  endif
> > -base_lib = static_library('octeontx_base', sources,
> > -	c_args: c_args,
> > -	dependencies: static_objs,
> > -)
> >
> > -base_objs = base_lib.extract_all_objects()
> > +if build
> > +        foreach d: depends
> > +                static_objs += [get_variable('static_rte_' + d)]
> > +        endforeach
> > +
> > +        base_lib = static_library('octeontx_base', sources,
> > +                c_args: c_args,
> > +                dependencies: static_objs,
> > +        )
> > +
> > +        base_objs = base_lib.extract_all_objects() endif
> 
> A better fix here might be possible using the fact that the get_variable() call
> allows passing a fallback value in the case of failure. Therefore something like
> the below might work cleaner:
ACK, will update the same in V2
> 
> foreach d: depends
> 	static_obj = [get_variable('static_rte_' + d, '']
> 	if static_obj == ''
> 		build = false
> 		reason = '....'
> 		subdir_done()
> 	endif
> 	static_objs += static_obj
> ...
> 
> Maybe a disabler object could be used also.
> 
> Regards,
> /Bruce
Amit Gupta Feb. 19, 2020, 4:34 a.m. UTC | #4 
> -----Original Message-----
> From: David Marchand <david.marchand@redhat.com>
> Sent: Monday, February 17, 2020 3:24 PM
> To: Amit Gupta <agupta3@marvell.com>
> Cc: Harman Kalra <hkalra@marvell.com>; dev <dev@dpdk.org>; Bruce
> Richardson <bruce.richardson@intel.com>
> Subject: [EXT] Re: [dpdk-dev] [PATCH] net/octeontx: meson build fix if
> octeontx drivers are disabled
> 
> External Email
> 
> ----------------------------------------------------------------------
> On Mon, Feb 17, 2020 at 8:48 AM <agupta3@marvell.com> wrote:
> >
> > From: Amit Gupta <agupta3@marvell.com>
> >
> > Add a condition to check if octeontx drivers are disabled.
> > octeontx drivers are built only if dependent drivers i.e.
> > ethdev, mempool and common/octeontx are enabled.
> >
> > BugZilla ID # BUG 387
> 
> Interesting format, but we prefer consistency, you can see this in git history.
> Bugzilla ID: xxx
> 
> This can be caught by running the devtools/check-git-log.sh script.

ACK, will update the same in V2

> 
> 
> >
> > Signed-off-by: Amit Gupta <agupta3@marvell.com>
> > ---
> >  drivers/net/octeontx/base/meson.build | 32
> > ++++++++++++++++++++++++--------
> >  1 file changed, 24 insertions(+), 8 deletions(-)
> >
> > diff --git a/drivers/net/octeontx/base/meson.build
> > b/drivers/net/octeontx/base/meson.build
> > index a06a2c8..50e7972 100644
> > --- a/drivers/net/octeontx/base/meson.build
> > +++ b/drivers/net/octeontx/base/meson.build
> > @@ -9,17 +9,33 @@ sources = [
> >
> >  depends = ['ethdev', 'mempool_octeontx']  static_objs = [] -foreach
> > d: depends
> > -       static_objs += [get_variable('static_rte_' + d)]
> > +
> > +disabled_drivers = get_option('disable_drivers').split(',')
> > +
> > +build = true
> > +foreach disable_path: disabled_drivers
> > +        if (('net/octeontx' == disable_path) or
> > +          ('event/octeontx' == disable_path) or
> > +          ('common/octeontx' == disable_path) or
> > +          ('mempool/octeontx' == disable_path))
> > +                build = false
> > +        endif
> >  endforeach
> 
> I am no meson expert, but I'd say you can put a subdir_done() if your check
> is false and then you don't need to check the build variable later.
> Besides, you can most likely do this check at the top of this file alongside
> "depends".

ACK, will update the same in V2

> 
> Cc: Bruce who commented on this bz.
> 
> >
> >  c_args = cflags
> >  if allow_experimental_apis
> > -       c_args += '-DALLOW_EXPERIMENTAL_API'
> > +        c_args += '-DALLOW_EXPERIMENTAL_API'
> 
> Unrelated change + whitespace damage after.
> 
ACK, will update the same in V2
> 
> 
> 
> >  endif
> > -base_lib = static_library('octeontx_base', sources,
> > -       c_args: c_args,
> > -       dependencies: static_objs,
> > -)
> >
> > -base_objs = base_lib.extract_all_objects()
> > +if build
> > +        foreach d: depends
> > +                static_objs += [get_variable('static_rte_' + d)]
> > +        endforeach
> > +
> > +        base_lib = static_library('octeontx_base', sources,
> > +                c_args: c_args,
> > +                dependencies: static_objs,
> > +        )
> > +
> > +        base_objs = base_lib.extract_all_objects() endif
> > --
> > 1.8.3.1
> >
> 
> 
> --
> David Marchand
diff mbox series

Patch

diff --git a/drivers/net/octeontx/base/meson.build b/drivers/net/octeontx/base/meson.build
index a06a2c8..50e7972 100644
--- a/drivers/net/octeontx/base/meson.build
+++ b/drivers/net/octeontx/base/meson.build
@@ -9,17 +9,33 @@  sources = [
 
 depends = ['ethdev', 'mempool_octeontx']
 static_objs = []
-foreach d: depends
-	static_objs += [get_variable('static_rte_' + d)]
+
+disabled_drivers = get_option('disable_drivers').split(',')
+
+build = true
+foreach disable_path: disabled_drivers
+        if (('net/octeontx' == disable_path) or
+	   ('event/octeontx' == disable_path) or
+	   ('common/octeontx' == disable_path) or
+	   ('mempool/octeontx' == disable_path))
+                build = false
+        endif
 endforeach
 
 c_args = cflags
 if allow_experimental_apis
-	c_args += '-DALLOW_EXPERIMENTAL_API'
+        c_args += '-DALLOW_EXPERIMENTAL_API'
 endif
-base_lib = static_library('octeontx_base', sources,
-	c_args: c_args,
-	dependencies: static_objs,
-)
 
-base_objs = base_lib.extract_all_objects()
+if build
+        foreach d: depends
+                static_objs += [get_variable('static_rte_' + d)]
+        endforeach
+
+        base_lib = static_library('octeontx_base', sources,
+                c_args: c_args,
+                dependencies: static_objs,
+        )
+
+        base_objs = base_lib.extract_all_objects()
+endif