DPDK patches and discussions
 help / color / mirror / Atom feed
* [dpdk-dev] [PATCH] build: don't parse build configs of explicitly disabled drivers
@ 2020-03-26  9:22 Darek Stojaczyk
  2020-03-26 11:03 ` Bruce Richardson
                   ` (2 more replies)
  0 siblings, 3 replies; 6+ messages in thread
From: Darek Stojaczyk @ 2020-03-26  9:22 UTC (permalink / raw)
  To: dev; +Cc: Bruce Richardson, Darek Stojaczyk

Even when a PMD was disabled with meson's disable_drivers option
its config file was still being parsed. Some of the PMD configs
attempt to find a library they depend on and parse its header files
with certain assumptions. If the library is found, but it's simply
too old to contain the necessary header files, the meson build
fails and it can only be fixed by either updating that library, or
expanding the meson script for the faulty PMD.

While the latter should be still done for the sake of DPDK quality,
an intermediate solution would be to skip building the faulty PMD
- there's a chance we don't need it. That's what this patch allows.

Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
---
 drivers/meson.build | 9 ++++++---
 1 file changed, 6 insertions(+), 3 deletions(-)

diff --git a/drivers/meson.build b/drivers/meson.build
index 5502bf9924..a13c62a3b0 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -60,9 +60,6 @@ foreach class:dpdk_driver_classes
 		ext_deps = []
 		pkgconfig_extra_libs = []
 
-		# pull in driver directory which should assign to each of the above
-		subdir(drv_path)
-
 		# skip disabled drivers. For meson 0.49 change this to use
 		# "in" keyword
 		foreach disable_path: disabled_drivers
@@ -71,6 +68,12 @@ foreach class:dpdk_driver_classes
 				reason = 'Explicitly disabled via build config'
 			endif
 		endforeach
+
+		if build
+			# pull in driver directory which should update all the local variables
+			subdir(drv_path)
+		endif
+
 		if build
 			# get dependency objs from strings
 			shared_deps = ext_deps
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] build: don't parse build configs of explicitly disabled drivers
  2020-03-26  9:22 [dpdk-dev] [PATCH] build: don't parse build configs of explicitly disabled drivers Darek Stojaczyk
@ 2020-03-26 11:03 ` Bruce Richardson
  2020-03-30 13:52 ` Bruce Richardson
  2020-05-07 10:13 ` [dpdk-dev] [PATCH v2] " Darek Stojaczyk
  2 siblings, 0 replies; 6+ messages in thread
From: Bruce Richardson @ 2020-03-26 11:03 UTC (permalink / raw)
  To: Darek Stojaczyk; +Cc: dev

On Thu, Mar 26, 2020 at 10:22:59AM +0100, Darek Stojaczyk wrote:
> Even when a PMD was disabled with meson's disable_drivers option
> its config file was still being parsed. Some of the PMD configs
> attempt to find a library they depend on and parse its header files
> with certain assumptions. If the library is found, but it's simply
> too old to contain the necessary header files, the meson build
> fails and it can only be fixed by either updating that library, or
> expanding the meson script for the faulty PMD.
> 
> While the latter should be still done for the sake of DPDK quality,
> an intermediate solution would be to skip building the faulty PMD
> - there's a chance we don't need it. That's what this patch allows.
> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---
>  drivers/meson.build | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/meson.build b/drivers/meson.build
> index 5502bf9924..a13c62a3b0 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -60,9 +60,6 @@ foreach class:dpdk_driver_classes
>  		ext_deps = []
>  		pkgconfig_extra_libs = []
>  
> -		# pull in driver directory which should assign to each of the above
> -		subdir(drv_path)
> -
>  		# skip disabled drivers. For meson 0.49 change this to use
>  		# "in" keyword
>  		foreach disable_path: disabled_drivers
> @@ -71,6 +68,12 @@ foreach class:dpdk_driver_classes
>  				reason = 'Explicitly disabled via build config'
>  			endif
>  		endforeach
> +
> +		if build
> +			# pull in driver directory which should update all the local variables
> +			subdir(drv_path)
> +		endif
> +
>  		if build
>  			# get dependency objs from strings
>  			shared_deps = ext_deps
> -- 

Good idea to fix this. The original idea was to parse the subdir files so
that the "reason" field could report out on any missing dependencies even
if the driver was explicitly disabled. However, that choice did cause other
problems so I think skipping the parsing as you do here is the better
choice.

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

Note: if for a future release we bump our minimum meson version to 0.49 or
greater we should be able to simplify all these loops and "if build" checks
as the "break" and "continue" keywords become available, as does the "in"
keyword for checking for a value in an array.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] build: don't parse build configs of explicitly disabled drivers
  2020-03-26  9:22 [dpdk-dev] [PATCH] build: don't parse build configs of explicitly disabled drivers Darek Stojaczyk
  2020-03-26 11:03 ` Bruce Richardson
@ 2020-03-30 13:52 ` Bruce Richardson
  2020-05-07 10:16   ` Stojaczyk, Dariusz
  2020-05-07 10:13 ` [dpdk-dev] [PATCH v2] " Darek Stojaczyk
  2 siblings, 1 reply; 6+ messages in thread
From: Bruce Richardson @ 2020-03-30 13:52 UTC (permalink / raw)
  To: Darek Stojaczyk; +Cc: dev

On Thu, Mar 26, 2020 at 10:22:59AM +0100, Darek Stojaczyk wrote:
> Even when a PMD was disabled with meson's disable_drivers option
> its config file was still being parsed. Some of the PMD configs
> attempt to find a library they depend on and parse its header files
> with certain assumptions. If the library is found, but it's simply
> too old to contain the necessary header files, the meson build
> fails and it can only be fixed by either updating that library, or
> expanding the meson script for the faulty PMD.
> 
> While the latter should be still done for the sake of DPDK quality,
> an intermediate solution would be to skip building the faulty PMD
> - there's a chance we don't need it. That's what this patch allows.
> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> ---
>  drivers/meson.build | 9 ++++++---
>  1 file changed, 6 insertions(+), 3 deletions(-)
> 
> diff --git a/drivers/meson.build b/drivers/meson.build
> index 5502bf9924..a13c62a3b0 100644
> --- a/drivers/meson.build
> +++ b/drivers/meson.build
> @@ -60,9 +60,6 @@ foreach class:dpdk_driver_classes
>  		ext_deps = []
>  		pkgconfig_extra_libs = []
>  
> -		# pull in driver directory which should assign to each of the above
> -		subdir(drv_path)
> -
>  		# skip disabled drivers. For meson 0.49 change this to use
>  		# "in" keyword
>  		foreach disable_path: disabled_drivers
> @@ -71,6 +68,12 @@ foreach class:dpdk_driver_classes
>  				reason = 'Explicitly disabled via build config'
>  			endif
>  		endforeach
> +
> +		if build
> +			# pull in driver directory which should update all the local variables
> +			subdir(drv_path)
> +		endif
> +

Looking at this code and the meson docs again, I think this block and
previous can be simplified by using the array "contains()" method to avoid
the loop.

	if disabled_drivers.contains(drv_path)
		build = false
		reson = '....'
	else
		# pull in driver ...
		subdir(drv_path)
	endif

Regards,
/Bruce

^ permalink raw reply	[flat|nested] 6+ messages in thread

* [dpdk-dev] [PATCH v2] build: don't parse build configs of explicitly disabled drivers
  2020-03-26  9:22 [dpdk-dev] [PATCH] build: don't parse build configs of explicitly disabled drivers Darek Stojaczyk
  2020-03-26 11:03 ` Bruce Richardson
  2020-03-30 13:52 ` Bruce Richardson
@ 2020-05-07 10:13 ` Darek Stojaczyk
  2020-05-19 14:50   ` Thomas Monjalon
  2 siblings, 1 reply; 6+ messages in thread
From: Darek Stojaczyk @ 2020-05-07 10:13 UTC (permalink / raw)
  To: dev, Bruce Richardson; +Cc: Darek Stojaczyk

Even when a PMD was disabled with meson's disable_drivers option
its config file was still being parsed. Some of the PMD configs
attempt to find a library they depend on and parse its header files
with certain assumptions. If the library is found, but it's simply
too old to contain the necessary header files, the meson build
fails and it can only be fixed by either updating that library, or
expanding the meson script for the faulty PMD.

While the latter should be still done for the sake of DPDK quality,
an intermediate solution would be to skip building the faulty PMD
- there's a chance we don't need it. That's what this patch allows.

Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
Acked-by: Bruce Richardson <bruce.richardson@intel.com>
---
v2:
 - simplified the code with if X.contains(), as Bruce suggested
 - removed the related comment from the code - it should be
   self-documented now

 drivers/meson.build | 19 ++++++++-----------
 1 file changed, 8 insertions(+), 11 deletions(-)

diff --git a/drivers/meson.build b/drivers/meson.build
index 72eec46088..483226a789 100644
--- a/drivers/meson.build
+++ b/drivers/meson.build
@@ -57,17 +57,14 @@ foreach class:dpdk_driver_classes
 		ext_deps = []
 		pkgconfig_extra_libs = []
 
-		# pull in driver directory which should assign to each of the above
-		subdir(drv_path)
-
-		# skip disabled drivers. For meson 0.49 change this to use
-		# "in" keyword
-		foreach disable_path: disabled_drivers
-			if drv_path == disable_path
-				build = false
-				reason = 'Explicitly disabled via build config'
-			endif
-		endforeach
+		if disabled_drivers.contains(drv_path)
+			build = false
+			reason = 'Explicitly disabled via build config'
+		else
+			# pull in driver directory which should update all the local variables
+			subdir(drv_path)
+		endif
+
 		if build
 			# get dependency objs from strings
 			shared_deps = ext_deps
-- 
2.17.1


^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH] build: don't parse build configs of explicitly disabled drivers
  2020-03-30 13:52 ` Bruce Richardson
@ 2020-05-07 10:16   ` Stojaczyk, Dariusz
  0 siblings, 0 replies; 6+ messages in thread
From: Stojaczyk, Dariusz @ 2020-05-07 10:16 UTC (permalink / raw)
  To: Richardson, Bruce; +Cc: dev


> -----Original Message-----
> From: Bruce Richardson <bruce.richardson@intel.com>
> Sent: Monday, March 30, 2020 3:53 PM
> To: Stojaczyk, Dariusz <dariusz.stojaczyk@intel.com>
> Cc: dev@dpdk.org
> Subject: Re: [PATCH] build: don't parse build configs of explicitly disabled
> drivers
> 
> [SNIP]
>
> Looking at this code and the meson docs again, I think this block and
> previous can be simplified by using the array "contains()" method to avoid
> the loop.
> 
> 	if disabled_drivers.contains(drv_path)
> 		build = false
> 		reson = '....'
> 	else
> 		# pull in driver ...
> 		subdir(drv_path)
> 	endif
> 
> Regards,
> /Bruce

Oops, I haven't seen that mail. Sorry! I just pushed v2.
D.

^ permalink raw reply	[flat|nested] 6+ messages in thread

* Re: [dpdk-dev] [PATCH v2] build: don't parse build configs of explicitly disabled drivers
  2020-05-07 10:13 ` [dpdk-dev] [PATCH v2] " Darek Stojaczyk
@ 2020-05-19 14:50   ` Thomas Monjalon
  0 siblings, 0 replies; 6+ messages in thread
From: Thomas Monjalon @ 2020-05-19 14:50 UTC (permalink / raw)
  To: Darek Stojaczyk; +Cc: dev, Bruce Richardson

07/05/2020 12:13, Darek Stojaczyk:
> Even when a PMD was disabled with meson's disable_drivers option
> its config file was still being parsed. Some of the PMD configs
> attempt to find a library they depend on and parse its header files
> with certain assumptions. If the library is found, but it's simply
> too old to contain the necessary header files, the meson build
> fails and it can only be fixed by either updating that library, or
> expanding the meson script for the faulty PMD.
> 
> While the latter should be still done for the sake of DPDK quality,
> an intermediate solution would be to skip building the faulty PMD
> - there's a chance we don't need it. That's what this patch allows.
> 
> Signed-off-by: Darek Stojaczyk <dariusz.stojaczyk@intel.com>
> Acked-by: Bruce Richardson <bruce.richardson@intel.com>
> ---
> v2:
>  - simplified the code with if X.contains(), as Bruce suggested
>  - removed the related comment from the code - it should be
>    self-documented now

Applied, thanks



^ permalink raw reply	[flat|nested] 6+ messages in thread

end of thread, other threads:[~2020-05-19 14:51 UTC | newest]

Thread overview: 6+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
2020-03-26  9:22 [dpdk-dev] [PATCH] build: don't parse build configs of explicitly disabled drivers Darek Stojaczyk
2020-03-26 11:03 ` Bruce Richardson
2020-03-30 13:52 ` Bruce Richardson
2020-05-07 10:16   ` Stojaczyk, Dariusz
2020-05-07 10:13 ` [dpdk-dev] [PATCH v2] " Darek Stojaczyk
2020-05-19 14:50   ` Thomas Monjalon

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).