diff mbox series

[1/2] meson: don't check dependencies for tests if not required

Message ID 20190529163958.30796-2-i.maximets@samsung.com (mailing list archive)
State Superseded, archived
Headers
Series Make meson configurable. |

Checks

Context Check Description
ci/checkpatch success coding style OK
ci/intel-Performance-Testing success Performance Testing PASS
ci/mellanox-Performance-Testing success Performance Testing PASS
ci/Intel-compilation fail Compilation issues

Commit Message

Ilya Maximets May 29, 2019, 4:39 p.m. UTC 
  Don't need to check dependencies if test apps will not be built anyway.

Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
---
 app/test/meson.build | 38 +++++++++++++++++++-------------------
 1 file changed, 19 insertions(+), 19 deletions(-)

Comments

Bruce Richardson May 30, 2019, 11:55 a.m. UTC | #1 
On Wed, May 29, 2019 at 07:39:57PM +0300, Ilya Maximets wrote:
> Don't need to check dependencies if test apps will not be built anyway.
> 
> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> ---
>  app/test/meson.build | 38 +++++++++++++++++++-------------------
>  1 file changed, 19 insertions(+), 19 deletions(-)
> 
Agree with the idea.

Would this work as a shorter alternative placed at the top of the file?

if not get_option('tests')
	subdir_done()
endif

/Bruce
Ilya Maximets May 30, 2019, 12:06 p.m. UTC | #2 
On 30.05.2019 14:55, Bruce Richardson wrote:
> On Wed, May 29, 2019 at 07:39:57PM +0300, Ilya Maximets wrote:
>> Don't need to check dependencies if test apps will not be built anyway.
>>
>> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
>> ---
>>  app/test/meson.build | 38 +++++++++++++++++++-------------------
>>  1 file changed, 19 insertions(+), 19 deletions(-)
>>
> Agree with the idea.
> 
> Would this work as a shorter alternative placed at the top of the file?
> 
> if not get_option('tests')
> 	subdir_done()
> endif

This looks good to me.
However, the resulted patch will be much larger because we'll have to
shift most of it to the left. If it's OK, I'll prepare v2 with this change.
What do you think?

Best regards, Ilya Maximets.
Bruce Richardson May 30, 2019, 12:20 p.m. UTC | #3 
On Thu, May 30, 2019 at 03:06:17PM +0300, Ilya Maximets wrote:
> On 30.05.2019 14:55, Bruce Richardson wrote:
> > On Wed, May 29, 2019 at 07:39:57PM +0300, Ilya Maximets wrote:
> >> Don't need to check dependencies if test apps will not be built anyway.
> >>
> >> Signed-off-by: Ilya Maximets <i.maximets@samsung.com>
> >> ---
> >>  app/test/meson.build | 38 +++++++++++++++++++-------------------
> >>  1 file changed, 19 insertions(+), 19 deletions(-)
> >>
> > Agree with the idea.
> > 
> > Would this work as a shorter alternative placed at the top of the file?
> > 
> > if not get_option('tests')
> > 	subdir_done()
> > endif
> 
> This looks good to me.
> However, the resulted patch will be much larger because we'll have to
> shift most of it to the left. If it's OK, I'll prepare v2 with this change.
> What do you think?
> 
Yes, there will be some left-shifting, but it should just be a single block
from lines 338-419, which is probably ok. The end result is better, I
think.
diff mbox series

Patch

diff --git a/app/test/meson.build b/app/test/meson.build
index 83391cef0..7a529b644 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -313,29 +313,29 @@  endif
 # specify -D_GNU_SOURCE unconditionally
 cflags += '-D_GNU_SOURCE'
 
-test_dep_objs = []
-if dpdk_conf.has('RTE_LIBRTE_COMPRESSDEV')
-	compress_test_dep = dependency('zlib', required: false)
-	if compress_test_dep.found()
-		test_dep_objs += compress_test_dep
-		test_sources += 'test_compressdev.c'
-		test_deps += 'compressdev'
-		fast_non_parallel_test_names += 'compressdev_autotest'
+if get_option('tests')
+	test_dep_objs = []
+	if dpdk_conf.has('RTE_LIBRTE_COMPRESSDEV')
+		compress_test_dep = dependency('zlib', required: false)
+		if compress_test_dep.found()
+			test_dep_objs += compress_test_dep
+			test_sources += 'test_compressdev.c'
+			test_deps += 'compressdev'
+			fast_non_parallel_test_names += 'compressdev_autotest'
+		endif
 	endif
-endif
 
-foreach d:test_deps
-	def_lib = get_option('default_library')
-	test_dep_objs += get_variable(def_lib + '_rte_' + d)
-endforeach
-test_dep_objs += cc.find_library('execinfo', required: false)
+	foreach d:test_deps
+		def_lib = get_option('default_library')
+		test_dep_objs += get_variable(def_lib + '_rte_' + d)
+	endforeach
+	test_dep_objs += cc.find_library('execinfo', required: false)
 
-link_libs = []
-if get_option('default_library') == 'static'
-	link_libs = dpdk_drivers
-endif
+	link_libs = []
+	if get_option('default_library') == 'static'
+		link_libs = dpdk_drivers
+	endif
 
-if get_option('tests')
 	dpdk_test = executable('dpdk-test',
 		test_sources,
 		link_whole: link_libs,