[v2,11/11] devtools: compile all buildable examples with pkg-config

Message ID 20201113122430.25354-12-david.marchand@redhat.com (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series Examples compilation fixes |

Checks

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

Commit Message

David Marchand Nov. 13, 2020, 12:24 p.m. UTC
  Rather than just installing all examples, we can use the build checks to
filter out any examples that are missing dependencies or are otherwise
unbuildable on the current system.
Introduce a new "buildable" special value for the -Dexamples= meson
option, this way existing behavior on installing all examples is
preserved.

Select only buildable examples and test their compilation for the
x86-default target.

Note for maintainers/users of the script: for existing environments,
the x86-default target might get broken by this patch since the script
now tries to build all "installed" examples and dependencies for some
might be unfulfilled.
To fix this temporary situation, you can either delete the whole
directory or reconfigure it:
$ meson configure $DPDK_BUILD_TEST_DIR/build-x86-default \
  -Dexamples=buildable

Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
Signed-off-by: David Marchand <david.marchand@redhat.com>
---
Changelog since v1:
- rebased on main,
- introduced a new "buildable" special value for the -Dexamples option,
- installation of the "multi-level" examples has been fixed, so
  corresponding exceptions have been removed,
- a fix for the vhost is waiting in next-virtio, I assume it will get
  pulled before this series,
- the only remaining exception is for vm_power_manager which is broken,

---
 devtools/test-meson-builds.sh | 11 +++++++++--
 examples/meson.build          | 24 ++++++++++++++++++++++--
 meson.build                   |  2 ++
 3 files changed, 33 insertions(+), 4 deletions(-)
  

Comments

David Marchand Nov. 13, 2020, 12:37 p.m. UTC | #1
Thomas,

On Fri, Nov 13, 2020 at 1:28 PM David Marchand
<david.marchand@redhat.com> wrote:
> - a fix for the vhost is waiting in next-virtio, I assume it will get
>   pulled before this series,

If next-virtio is not pulled yet, we can add a temp:
dpdk_examples_exclude += ['vhost']
  
Bruce Richardson Nov. 13, 2020, 2:06 p.m. UTC | #2
On Fri, Nov 13, 2020 at 01:24:30PM +0100, David Marchand wrote:
> Rather than just installing all examples, we can use the build checks to
> filter out any examples that are missing dependencies or are otherwise
> unbuildable on the current system.
> Introduce a new "buildable" special value for the -Dexamples= meson
> option, this way existing behavior on installing all examples is
> preserved.
> 
> Select only buildable examples and test their compilation for the
> x86-default target.
> 
> Note for maintainers/users of the script: for existing environments,
> the x86-default target might get broken by this patch since the script
> now tries to build all "installed" examples and dependencies for some
> might be unfulfilled.
> To fix this temporary situation, you can either delete the whole
> directory or reconfigure it:
> $ meson configure $DPDK_BUILD_TEST_DIR/build-x86-default \
>   -Dexamples=buildable
> 
> Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> Signed-off-by: David Marchand <david.marchand@redhat.com>
> ---
> Changelog since v1:
> - rebased on main,
> - introduced a new "buildable" special value for the -Dexamples option,
> - installation of the "multi-level" examples has been fixed, so
>   corresponding exceptions have been removed,
> - a fix for the vhost is waiting in next-virtio, I assume it will get
>   pulled before this series,
> - the only remaining exception is for vm_power_manager which is broken,
> 
> ---
I'm not sure my "suggested-by" should be on this, since the idea of adding
an extra buildable option is not mine here. I actually think I prefer the
previous approach based on your original suggestion of always skipping
unbuildable examples. Two reasons for that:
* I'm not fully sure of the value of installing examples that can't be
  built, which was the original issue you raised.
* I don't like mixing together two separate things - examples to build, and
  examples to install in a single option.

Therefore, I think we should choose one of two less confusing paths -
either lets just always install all applications, or let's always install
only those apps which can be built. I'm not massively concerned either way
which is chosen, but I don't think we should try and support both and
overload options to do so.

/Bruce
  
David Marchand Nov. 13, 2020, 2:14 p.m. UTC | #3
On Fri, Nov 13, 2020 at 3:06 PM Bruce Richardson
<bruce.richardson@intel.com> wrote:
>
> On Fri, Nov 13, 2020 at 01:24:30PM +0100, David Marchand wrote:
> > Rather than just installing all examples, we can use the build checks to
> > filter out any examples that are missing dependencies or are otherwise
> > unbuildable on the current system.
> > Introduce a new "buildable" special value for the -Dexamples= meson
> > option, this way existing behavior on installing all examples is
> > preserved.
> >
> > Select only buildable examples and test their compilation for the
> > x86-default target.
> >
> > Note for maintainers/users of the script: for existing environments,
> > the x86-default target might get broken by this patch since the script
> > now tries to build all "installed" examples and dependencies for some
> > might be unfulfilled.
> > To fix this temporary situation, you can either delete the whole
> > directory or reconfigure it:
> > $ meson configure $DPDK_BUILD_TEST_DIR/build-x86-default \
> >   -Dexamples=buildable
> >
> > Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > ---
> > Changelog since v1:
> > - rebased on main,
> > - introduced a new "buildable" special value for the -Dexamples option,
> > - installation of the "multi-level" examples has been fixed, so
> >   corresponding exceptions have been removed,
> > - a fix for the vhost is waiting in next-virtio, I assume it will get
> >   pulled before this series,
> > - the only remaining exception is for vm_power_manager which is broken,
> >
> > ---
> I'm not sure my "suggested-by" should be on this, since the idea of adding

I was not sure how to credit my copying of the meson bits :-).


> an extra buildable option is not mine here. I actually think I prefer the
> previous approach based on your original suggestion of always skipping
> unbuildable examples. Two reasons for that:
> * I'm not fully sure of the value of installing examples that can't be
>   built, which was the original issue you raised.

Thomas objected that all examples have always been provided regardless
of the configuration.
The examples serve as documentation and the API/guides documentations
are all compiled anyway.


> * I don't like mixing together two separate things - examples to build, and
>   examples to install in a single option.

Yes, I am not entirely happy either.
Time is ticking, I'll just drop this patch for 20.11.
  
Bruce Richardson Nov. 13, 2020, 2:24 p.m. UTC | #4
On Fri, Nov 13, 2020 at 03:14:14PM +0100, David Marchand wrote:
> On Fri, Nov 13, 2020 at 3:06 PM Bruce Richardson
> <bruce.richardson@intel.com> wrote:
> >
> > On Fri, Nov 13, 2020 at 01:24:30PM +0100, David Marchand wrote:
> > > Rather than just installing all examples, we can use the build checks to
> > > filter out any examples that are missing dependencies or are otherwise
> > > unbuildable on the current system.
> > > Introduce a new "buildable" special value for the -Dexamples= meson
> > > option, this way existing behavior on installing all examples is
> > > preserved.
> > >
> > > Select only buildable examples and test their compilation for the
> > > x86-default target.
> > >
> > > Note for maintainers/users of the script: for existing environments,
> > > the x86-default target might get broken by this patch since the script
> > > now tries to build all "installed" examples and dependencies for some
> > > might be unfulfilled.
> > > To fix this temporary situation, you can either delete the whole
> > > directory or reconfigure it:
> > > $ meson configure $DPDK_BUILD_TEST_DIR/build-x86-default \
> > >   -Dexamples=buildable
> > >
> > > Suggested-by: Bruce Richardson <bruce.richardson@intel.com>
> > > Signed-off-by: David Marchand <david.marchand@redhat.com>
> > > ---
> > > Changelog since v1:
> > > - rebased on main,
> > > - introduced a new "buildable" special value for the -Dexamples option,
> > > - installation of the "multi-level" examples has been fixed, so
> > >   corresponding exceptions have been removed,
> > > - a fix for the vhost is waiting in next-virtio, I assume it will get
> > >   pulled before this series,
> > > - the only remaining exception is for vm_power_manager which is broken,
> > >
> > > ---
> > I'm not sure my "suggested-by" should be on this, since the idea of adding
> 
> I was not sure how to credit my copying of the meson bits :-).
> 
> 
> > an extra buildable option is not mine here. I actually think I prefer the
> > previous approach based on your original suggestion of always skipping
> > unbuildable examples. Two reasons for that:
> > * I'm not fully sure of the value of installing examples that can't be
> >   built, which was the original issue you raised.
> 
> Thomas objected that all examples have always been provided regardless
> of the configuration.
> The examples serve as documentation and the API/guides documentations
> are all compiled anyway.
> 
Since we are looking to start adding in checks for features into the
example makefiles, I'd go with continuing the policy of just shipping them
all [1]. Over time we can update the makefiles to start adding proper build
error messages with explanations in cases where examples are missing
dependencies.

/Bruce

[1] http://patches.dpdk.org/patch/83911/
  

Patch

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 3ce49368cf..a04f3eb7ff 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -213,7 +213,8 @@  default_machine='nehalem'
 if ! check_cc_flags "-march=$default_machine" ; then
 	default_machine='corei7'
 fi
-build build-x86-default cc -Dlibdir=lib -Dmachine=$default_machine $use_shared
+build build-x86-default cc -Dlibdir=lib -Dmachine=$default_machine \
+	-Dexamples=buildable $use_shared
 
 # 32-bit with default compiler
 if check_cc_flags '-m32' ; then
@@ -266,10 +267,16 @@  pc_file=$(find $DESTDIR -name libdpdk.pc)
 export PKG_CONFIG_PATH=$(dirname $pc_file):$PKG_CONFIG_PATH
 libdir=$(dirname $(find $DESTDIR -name librte_eal.so))
 export LD_LIBRARY_PATH=$libdir:$LD_LIBRARY_PATH
-examples=${DPDK_BUILD_TEST_EXAMPLES:-"cmdline helloworld l2fwd l3fwd skeleton timer"}
 # if pkg-config defines the necessary flags, test building some examples
 if pkg-config --define-prefix libdpdk >/dev/null 2>&1; then
 	export PKGCONF="pkg-config --define-prefix"
+	examples=${DPDK_BUILD_TEST_EXAMPLES:-}
+	if [ -z "$examples" ]; then
+		for mk in $DESTDIR/usr/local/share/dpdk/examples/*/Makefile; do
+			name=$(basename $(dirname $mk))
+			examples="$examples $name"
+		done
+	fi
 	for example in $examples; do
 		echo "## Building $example"
 		$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example \
diff --git a/examples/meson.build b/examples/meson.build
index 46ec80919e..a23ab79e8c 100644
--- a/examples/meson.build
+++ b/examples/meson.build
@@ -55,9 +55,18 @@  endif
 if get_option('examples').to_lower() == 'all'
 	examples = all_examples
 	allow_skips = true # don't flag an error if we can't build an app
+	install_buildable = false
+elif get_option('examples').to_lower() == 'buildable'
+	examples = all_examples
+	allow_skips = true
+	install_buildable = true # only install examples that can be built
+	# FIXME: vm_power_manager relies on an internal header and can't build
+	# outside dpdk tree.
+	dpdk_examples_exclude += ['vm_power_manager']
 else
 	examples = get_option('examples').split(',')
 	allow_skips = false # error out if we can't build a requested app
+	install_buildable = false
 endif
 default_cflags = machine_args
 if cc.has_argument('-Wno-format-truncation')
@@ -81,10 +90,15 @@  foreach example: examples
 		foreach d:deps
 			var_name = get_option('default_library') + '_rte_' + d
 			if not is_variable(var_name)
-				error('Missing dependency "@0@" for example "@1@"'.format(d, name))
+				message('Missing dependency "@0@" for example "@1@"'.format(d, name))
+				build = false
+			else
+				dep_objs += [get_variable(var_name)]
 			endif
-			dep_objs += [get_variable(var_name)]
 		endforeach
+	endif
+
+	if build
 		if allow_experimental_apis
 			cflags += '-DALLOW_EXPERIMENTAL_API'
 		endif
@@ -98,5 +112,11 @@  foreach example: examples
 		error('Cannot build requested example "' + name + '"')
 	else
 		message('Skipping example "' + name + '"')
+		if install_buildable
+			# exclude based on top-level directory only
+			dir = example.split('/')[0]
+			dpdk_examples_exclude += dir
+			message('Excluding example directory "@0@" from install'.format(dir))
+		endif
 	endif
 endforeach
diff --git a/meson.build b/meson.build
index 45d974cd2c..559a9d2f1b 100644
--- a/meson.build
+++ b/meson.build
@@ -58,9 +58,11 @@  subdir('doc')
 
 # build any examples explicitly requested - useful for developers - and
 # install any example code into the appropriate install path
+dpdk_examples_exclude = []
 subdir('examples')
 install_subdir('examples',
 	install_dir: get_option('datadir') + '/dpdk',
+	exclude_directories: dpdk_examples_exclude,
 	exclude_files: 'meson.build')
 
 # build kernel modules if enabled