[1/1] devtools: avoid installing static binaries

Message ID 20201207173319.1397740-1-thomas@monjalon.net (mailing list archive)
State Superseded, archived
Delegated to: Thomas Monjalon
Headers
Series [1/1] devtools: avoid installing static binaries |

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/iol-testing success Testing PASS
ci/iol-intel-Functional success Functional Testing PASS
ci/iol-intel-Performance success Performance Testing PASS
ci/Intel-compilation success Compilation OK
ci/travis-robot success Travis build: passed

Commit Message

Thomas Monjalon Dec. 7, 2020, 5:33 p.m. UTC
  When testing compilation and checking ABI compatibility,
there is no real need of static binaries eating disks.
The static linkage of applications are tested with GCC and Clang,
plus some examples are statically linked.
The after-installation build test is limited to "helloworld" example.
Note the meson static build test was already limited to "l3fwd" example.

The ABI compatibility is checked on shared libraries, so no need
running this test a second time on builds intended for static linking.
However, limiting ABI check to "shared builds" means all test cases
must have a "shared build" occurence.
As a consequence the 32-bit build test is switched to shared linking.

Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
---
 devtools/test-meson-builds.sh | 8 ++++++--
 1 file changed, 6 insertions(+), 2 deletions(-)
  

Comments

Bruce Richardson Dec. 7, 2020, 5:47 p.m. UTC | #1
On Mon, Dec 07, 2020 at 06:33:19PM +0100, Thomas Monjalon wrote:
> When testing compilation and checking ABI compatibility,
> there is no real need of static binaries eating disks.
> The static linkage of applications are tested with GCC and Clang,
> plus some examples are statically linked.
> The after-installation build test is limited to "helloworld" example.
> Note the meson static build test was already limited to "l3fwd" example.
> 
> The ABI compatibility is checked on shared libraries, so no need
> running this test a second time on builds intended for static linking.
> However, limiting ABI check to "shared builds" means all test cases
> must have a "shared build" occurence.
> As a consequence the 32-bit build test is switched to shared linking.
> 
> Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> ---
>  devtools/test-meson-builds.sh | 8 ++++++--
>  1 file changed, 6 insertions(+), 2 deletions(-)

I think this might be better as two patches - one for the ABI check changes
and a second for the example build changes with installed DPDK.

> 
> diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
> index 7280b7a93d..ed44d4ffb1 100755
> --- a/devtools/test-meson-builds.sh
> +++ b/devtools/test-meson-builds.sh
> @@ -166,6 +166,9 @@ build () # <directory> <target compiler | cross file> <meson options>
>  	config $srcdir $builds_dir/$targetdir $cross --werror $*
>  	compile $builds_dir/$targetdir
>  	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
> +		if echo $* | grep -qw -- '--default-library=static' ; then
> +			return # skip ABI check for static build
> +		fi
>  		abirefdir=${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION
>  		if [ ! -d $abirefdir/$targetdir ]; then
>  			# clone current sources
> @@ -230,7 +233,7 @@ if check_cc_flags '-m32' ; then
>  		export PKG_CONFIG_LIBDIR='/usr/lib/pkgconfig'
>  	fi
>  	target_override='i386-pc-linux-gnu'
> -	build build-32b cc -Dc_args='-m32' -Dc_link_args='-m32'
> +	build build-32b cc -Dc_args='-m32' -Dc_link_args='-m32' $use_shared
>  	target_override=
>  	unset PKG_CONFIG_LIBDIR
>  fi
> @@ -274,7 +277,8 @@ if pkg-config --define-prefix libdpdk >/dev/null 2>&1; then
>  	export PKGCONF="pkg-config --define-prefix"
>  	for example in $examples; do
>  		echo "## Building $example"
> +		[ $example = helloworld ] && static=static || static= # save disk space
>  		$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example \
> -			clean shared static >&$veryverbose
> +			clean shared $static >&$veryverbose
>  	done
>  fi

Just wonder are we likely to miss things with this change? Would changing
the order to do a clean at the end to free back up the disk space not
achieve much the same result while still saving disk space?
  
Thomas Monjalon Dec. 7, 2020, 6:12 p.m. UTC | #2
07/12/2020 18:47, Bruce Richardson:
> On Mon, Dec 07, 2020 at 06:33:19PM +0100, Thomas Monjalon wrote:
> > When testing compilation and checking ABI compatibility,
> > there is no real need of static binaries eating disks.
> > The static linkage of applications are tested with GCC and Clang,
> > plus some examples are statically linked.
> > The after-installation build test is limited to "helloworld" example.
> > Note the meson static build test was already limited to "l3fwd" example.
> > 
> > The ABI compatibility is checked on shared libraries, so no need
> > running this test a second time on builds intended for static linking.
> > However, limiting ABI check to "shared builds" means all test cases
> > must have a "shared build" occurence.
> > As a consequence the 32-bit build test is switched to shared linking.
> > 
> > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > ---
> >  devtools/test-meson-builds.sh | 8 ++++++--
> >  1 file changed, 6 insertions(+), 2 deletions(-)
> 
> I think this might be better as two patches - one for the ABI check changes
> and a second for the example build changes with installed DPDK.

Yes could be 2 patches.


> >  	for example in $examples; do
> >  		echo "## Building $example"
> > +		[ $example = helloworld ] && static=static || static= # save disk space
> >  		$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example \
> > -			clean shared static >&$veryverbose
> > +			clean shared $static >&$veryverbose
> >  	done
> >  fi
> 
> Just wonder are we likely to miss things with this change? Would changing
> the order to do a clean at the end to free back up the disk space not
> achieve much the same result while still saving disk space?

Not building static flavour of most examples is also faster.
Ideally we should not rebuild an example if the libs did not change.

To the question "will we miss something", the difference between static
and shared examples is just the pkg-config call in the Makefile.
I think the risk is small.
  
Bruce Richardson Dec. 8, 2020, 9:33 a.m. UTC | #3
On Mon, Dec 07, 2020 at 07:12:32PM +0100, Thomas Monjalon wrote:
> 07/12/2020 18:47, Bruce Richardson:
> > On Mon, Dec 07, 2020 at 06:33:19PM +0100, Thomas Monjalon wrote:
> > > When testing compilation and checking ABI compatibility,
> > > there is no real need of static binaries eating disks.
> > > The static linkage of applications are tested with GCC and Clang,
> > > plus some examples are statically linked.
> > > The after-installation build test is limited to "helloworld" example.
> > > Note the meson static build test was already limited to "l3fwd" example.
> > > 
> > > The ABI compatibility is checked on shared libraries, so no need
> > > running this test a second time on builds intended for static linking.
> > > However, limiting ABI check to "shared builds" means all test cases
> > > must have a "shared build" occurence.
> > > As a consequence the 32-bit build test is switched to shared linking.
> > > 
> > > Signed-off-by: Thomas Monjalon <thomas@monjalon.net>
> > > ---
> > >  devtools/test-meson-builds.sh | 8 ++++++--
> > >  1 file changed, 6 insertions(+), 2 deletions(-)
> > 
> > I think this might be better as two patches - one for the ABI check changes
> > and a second for the example build changes with installed DPDK.
> 
> Yes could be 2 patches.
> 
> 
> > >  	for example in $examples; do
> > >  		echo "## Building $example"
> > > +		[ $example = helloworld ] && static=static || static= # save disk space
> > >  		$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example \
> > > -			clean shared static >&$veryverbose
> > > +			clean shared $static >&$veryverbose
> > >  	done
> > >  fi
> > 
> > Just wonder are we likely to miss things with this change? Would changing
> > the order to do a clean at the end to free back up the disk space not
> > achieve much the same result while still saving disk space?
> 
> Not building static flavour of most examples is also faster.
> Ideally we should not rebuild an example if the libs did not change.
> 
> To the question "will we miss something", the difference between static
> and shared examples is just the pkg-config call in the Makefile.
> I think the risk is small.
> 
Yes, for the majority of the apps that is the case. However, the only
concern I have is for a number of the apps which link directly against a
driver or two. Looking at vm_power_manager example, which links against 3
drivers, I see that the extra flags are only added for shared builds so we
should be ok for that one anyway.

Therefore ok with this change exactly as you suggest.

/Bruce
  
David Marchand Dec. 8, 2020, 3:37 p.m. UTC | #4
On Mon, Dec 7, 2020 at 6:33 PM Thomas Monjalon <thomas@monjalon.net> wrote:
>
> When testing compilation and checking ABI compatibility,
> there is no real need of static binaries eating disks.
> The static linkage of applications are tested with GCC and Clang,
> plus some examples are statically linked.
> The after-installation build test is limited to "helloworld" example.
> Note the meson static build test was already limited to "l3fwd" example.
>
> The ABI compatibility is checked on shared libraries, so no need
> running this test a second time on builds intended for static linking.
> However, limiting ABI check to "shared builds" means all test cases
> must have a "shared build" occurence.
> As a consequence the 32-bit build test is switched to shared linking.

I see no reason to tie the ABI check to default-library.

What about the mingw target?


What you want is to avoid doing duplicate ABI checks.
This happens for the gcc/clang x86 builds, so I'd rather control the
ABI checks out of the build() function (passing a new parameter?).
  
Thomas Monjalon Dec. 8, 2020, 3:52 p.m. UTC | #5
08/12/2020 16:37, David Marchand:
> On Mon, Dec 7, 2020 at 6:33 PM Thomas Monjalon <thomas@monjalon.net> wrote:
> >
> > When testing compilation and checking ABI compatibility,
> > there is no real need of static binaries eating disks.
> > The static linkage of applications are tested with GCC and Clang,
> > plus some examples are statically linked.
> > The after-installation build test is limited to "helloworld" example.
> > Note the meson static build test was already limited to "l3fwd" example.
> >
> > The ABI compatibility is checked on shared libraries, so no need
> > running this test a second time on builds intended for static linking.
> > However, limiting ABI check to "shared builds" means all test cases
> > must have a "shared build" occurence.
> > As a consequence the 32-bit build test is switched to shared linking.
> 
> I see no reason to tie the ABI check to default-library.

The only reason is that ABI check triggers binary installation,
which is big when statically linked.

> What about the mingw target?

ABI check is not required for Windows.
BTW there are issues with DLL support.

> What you want is to avoid doing duplicate ABI checks.
> This happens for the gcc/clang x86 builds, so I'd rather control the
> ABI checks out of the build() function (passing a new parameter?).

Yes, it would be cleaner to separate ABI check requirement
and static linking.
In v2, ABI check will be enabled explicitly when calling "build" function
for shared builds.
  

Patch

diff --git a/devtools/test-meson-builds.sh b/devtools/test-meson-builds.sh
index 7280b7a93d..ed44d4ffb1 100755
--- a/devtools/test-meson-builds.sh
+++ b/devtools/test-meson-builds.sh
@@ -166,6 +166,9 @@  build () # <directory> <target compiler | cross file> <meson options>
 	config $srcdir $builds_dir/$targetdir $cross --werror $*
 	compile $builds_dir/$targetdir
 	if [ -n "$DPDK_ABI_REF_VERSION" ]; then
+		if echo $* | grep -qw -- '--default-library=static' ; then
+			return # skip ABI check for static build
+		fi
 		abirefdir=${DPDK_ABI_REF_DIR:-reference}/$DPDK_ABI_REF_VERSION
 		if [ ! -d $abirefdir/$targetdir ]; then
 			# clone current sources
@@ -230,7 +233,7 @@  if check_cc_flags '-m32' ; then
 		export PKG_CONFIG_LIBDIR='/usr/lib/pkgconfig'
 	fi
 	target_override='i386-pc-linux-gnu'
-	build build-32b cc -Dc_args='-m32' -Dc_link_args='-m32'
+	build build-32b cc -Dc_args='-m32' -Dc_link_args='-m32' $use_shared
 	target_override=
 	unset PKG_CONFIG_LIBDIR
 fi
@@ -274,7 +277,8 @@  if pkg-config --define-prefix libdpdk >/dev/null 2>&1; then
 	export PKGCONF="pkg-config --define-prefix"
 	for example in $examples; do
 		echo "## Building $example"
+		[ $example = helloworld ] && static=static || static= # save disk space
 		$MAKE -C $DESTDIR/usr/local/share/dpdk/examples/$example \
-			clean shared static >&$veryverbose
+			clean shared $static >&$veryverbose
 	done
 fi