diff mbox series

[1/2] app/test: add support for optional dependencies

Message ID 20230928110450.862698-1-bruce.richardson@intel.com (mailing list archive)
State Accepted, archived
Delegated to: David Marchand
Headers
Series [1/2] app/test: add support for optional dependencies |

Checks

Context Check Description
ci/checkpatch success coding style OK

Commit Message

Bruce Richardson Sept. 28, 2023, 11:04 a.m. UTC 
  Some tests make optionally use a component but don't require it for
building. If we include the dependency in the per-file lists, then tests
may be unnecessarily omitted, as the dependency is not required.
On the other hand, removing the optional dependency from the list can
cause build failures, as the test case may include the optional code,
but then fail to build as the build system won't have added the necessary
paths for header inclusion, or the necessary libraries for linking.

Resolve this by explicitly providing a list of optional dependencies.
Any items in this list will be added to the dependency list if
available, but otherwise won't be involved in enable/disable of specific
tests.

Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
---
 app/test/meson.build | 10 ++++++++++
 1 file changed, 10 insertions(+)

Comments

David Marchand Sept. 28, 2023, 12:50 p.m. UTC | #1 
On Thu, Sep 28, 2023 at 1:05 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> Some tests make optionally use a component but don't require it for

s/make //

> building. If we include the dependency in the per-file lists, then tests
> may be unnecessarily omitted, as the dependency is not required.
> On the other hand, removing the optional dependency from the list can
> cause build failures, as the test case may include the optional code,
> but then fail to build as the build system won't have added the necessary
> paths for header inclusion, or the necessary libraries for linking.
>
> Resolve this by explicitly providing a list of optional dependencies.
> Any items in this list will be added to the dependency list if
> available, but otherwise won't be involved in enable/disable of specific
> tests.
>
> Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>

For the series,
Reviewed-by: David Marchand <david.marchand@redhat.com>

Akhil can you confirm this series works for you?

Thanks!
Akhil Goyal Sept. 28, 2023, 1:06 p.m. UTC | #2 
> On Thu, Sep 28, 2023 at 1:05 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > Some tests make optionally use a component but don't require it for
> 
> s/make //
> 
> > building. If we include the dependency in the per-file lists, then tests
> > may be unnecessarily omitted, as the dependency is not required.
> > On the other hand, removing the optional dependency from the list can
> > cause build failures, as the test case may include the optional code,
> > but then fail to build as the build system won't have added the necessary
> > paths for header inclusion, or the necessary libraries for linking.
> >
> > Resolve this by explicitly providing a list of optional dependencies.
> > Any items in this list will be added to the dependency list if
> > available, but otherwise won't be involved in enable/disable of specific
> > tests.
> >
> > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> 
> For the series,
> Reviewed-by: David Marchand <david.marchand@redhat.com>
> 
> Akhil can you confirm this series works for you?
Tested-by: Akhil Goyal <gakhil@marvell.com>
David Marchand Sept. 28, 2023, 2:42 p.m. UTC | #3 
On Thu, Sep 28, 2023 at 3:06 PM Akhil Goyal <gakhil@marvell.com> wrote:
> > > Some tests make optionally use a component but don't require it for
> >
> > s/make //
> >
> > > building. If we include the dependency in the per-file lists, then tests
> > > may be unnecessarily omitted, as the dependency is not required.
> > > On the other hand, removing the optional dependency from the list can
> > > cause build failures, as the test case may include the optional code,
> > > but then fail to build as the build system won't have added the necessary
> > > paths for header inclusion, or the necessary libraries for linking.
> > >
> > > Resolve this by explicitly providing a list of optional dependencies.
> > > Any items in this list will be added to the dependency list if
> > > available, but otherwise won't be involved in enable/disable of specific
> > > tests.
> > >
> > > Signed-off-by: Bruce Richardson <bruce.richardson@intel.com>
> > Reviewed-by: David Marchand <david.marchand@redhat.com>
> Tested-by: Akhil Goyal <gakhil@marvell.com>

Series applied, thanks for the quick fix Bruce.
diff mbox series

Patch

diff --git a/app/test/meson.build b/app/test/meson.build
index 05bae9216d..80b60f68b2 100644
--- a/app/test/meson.build
+++ b/app/test/meson.build
@@ -5,6 +5,10 @@ 
 deps += ['cmdline', 'ring', 'mempool', 'mbuf']
 sources += files('commands.c', 'test.c')
 
+# optional dependencies: some files may use these - and so we should link them in -
+# but do not explicitly require them so they are not listed in the per-file lists below
+optional_deps = []
+
 # some other utility C files, providing functions used by various tests
 # so we need to include these deps in the dependency list for the files using those fns.
 packet_burst_generator_deps = ['net']
@@ -226,6 +230,12 @@  foreach f, f_deps : source_file_deps
         sources += files(f)
     endif
 endforeach
+# add the optional dependencies
+foreach d:optional_deps
+    if is_variable(def_lib + '_rte_' + d) and d not in deps
+        deps += d
+    endif
+endforeach
 
 if cc.has_argument('-Wno-format-truncation')
     cflags += '-Wno-format-truncation'