[Pkg-xen-devel] [PATCH 3/9] d/shuffle-binaries: Fix binary shuffling script for cross-building

[Hans van Kranenburg]

Hi,

On 7/17/20 8:37 AM, Elliott Mitchell wrote:
> `ldd` doesn't work with cross-builds,

...because XYZ(example here)... you're the expert here, spending so much
time on figuring this out, which we're all very thankful for, but we'd
like to know why to learn more.

> so use `file` to detect scripts
> and `strings | grep` for identifying linked libraries.  Even though
> debhelper depends on file, make the requirement explicit.

Why would we stop to depend on debhelper? I don't mind to have another
line with redundancy, but I'm just wondering what your thought behind
this would be.

> Heavily simplify script inner loop.  While the core of that inner loop
> was often executed only once, it is best to shrink inner loops.
> 
> With the reduced inner loop, less is being done inside pipes, so pipefail
> is lower value (the early commands in pipes are unlikely to fail).  This
> makes the script POSIX compliant so switch to /bin/sh.
> 
> Remove the useless extra argument passed to the script.

And that's 5? 6? 10? changes at once, and now I have to go dig into the
diff and find out which change belongs to which line in the comments.

You already went through all of the thought work doing all of this. And
that's much appreciated. Now make it explicit.

So, if you can split this out in a series of commits that just does one
logical change with one simple explanation, and even if there's just a
simple commit in the end that changes /bin/bash to /bin/sh with an
explanation of "yay, now we got rid of all the stuff that prevented us
from switching to /bin/sh for performance yay", no problem.

> Signed-off-by: Elliott Mitchell <ehem+debian at m5p.com>
> ---
>  debian/control          |  1 +
>  debian/rules            |  2 +-
>  debian/shuffle-binaries | 46 ++++++++++++++++++++++-------------------
>  3 files changed, 27 insertions(+), 22 deletions(-)
> 
> diff --git a/debian/control b/debian/control
> index 942bf6711c..90007f6a02 100644
> --- a/debian/control
> +++ b/debian/control
> @@ -6,6 +6,7 @@ Section: admin
>  Standards-Version: 4.5.0
>  Build-Depends:
>     debhelper (>= 10),
> +   file,
>     dh-exec,
>     dpkg-dev (>= 1.16.0~),
>     rdfind,
> diff --git a/debian/rules b/debian/rules
> index e6c51b4ede..3b7bb67550 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -268,7 +268,7 @@ xenstore_rm = $(addprefix debian/xen-utils-common/,		\
>  		))
>  
>  override_dh_install:
> -	debian/shuffle-binaries $(upstream_version) $(flavour)
> +	debian/shuffle-binaries $(upstream_version)
>  	:
>  	debian/shuffle-boot-files $(upstream_version) $(flavour)
>  	:
> diff --git a/debian/shuffle-binaries b/debian/shuffle-binaries
> index cff6de5428..7356531f1a 100755
> --- a/debian/shuffle-binaries
> +++ b/debian/shuffle-binaries
> @@ -1,6 +1,5 @@
> -#!/bin/bash
> +#!/bin/sh
>  set -e
> -set -o pipefail
>  
>  version=$1; shift
>  
> @@ -17,34 +16,39 @@ version=$1; shift
>  list=debian/libxenmiscV.install.vsn-in
>  
>  t=debian/tmp
> -vd=/usr/lib/xen-$version/bin
> +vd="/usr/lib/xen-$version/bin"
>  cd=/usr/lib/xen-common/bin
>  
> -mkdir -p $t/$vd
> +mkdir -p "$t/$vd"
>  
> -for binary in `find $t/usr/{bin,sbin} -type f`; do
> -	reason=''
> -	{ ldd "$binary" ||: ; } | { fgrep '=>' ||: ; } \
> -	| (
> -		while read lib dummy; do
> +find "$t"/usr/*bin -type f | while read binary; do
> +
> +	# filter out scripts
> +	file "$binary" | grep -q -eELF.\\+version.\\+interpreter || continue
> +
> +	# ldd doesn't work for cross-building
> +	reason=$(
> +		strings "$binary" | grep -e^lib.\\+\\.so\\.\[.0-9\]\\+\$ | \
> +		while read lib; do
>  			lib=${lib%.so.*}
> -			if grep -F "usr/lib/*/$lib.so.*" $list >/dev/null; then
> -				reason+=" $lib"
> +			if grep -q -F "usr/lib/*/$lib.so.*" $list; then
> +				printf " %s" "$lib"
>  			fi
>  		done
> +	)
>  
> -		if [ "x$reason" = x ]; then
> -			exit 0
> -		fi
> +	# if no reason, then skip
> +	[ -n "$reason" ] || continue
>  
> -		echo "shuffling $binary $reason"
> +	echo "shuffling $binary$reason"
>  
> -		leaf=${binary##*}
> -		mv -v $binary $t/$vd/$leaf
> -		ln -vs $cd/xen-utils-wrapper $binary
> +	mv -v "$binary" "$t/$vd"
> +	ln -vs "$cd/xen-utils-wrapper" "$binary"
>  
> -		touch debian/shuffle-binaries.stamp
> -	)
> +	touch "$0.stamp"
>  done
>  
> -ls debian/shuffle-binaries.stamp
> +if [ ! -e "$0.stamp" ]; then
> +	echo "Failed to shuffle binaries!" 1>&2
> +	exit 1
> +fi
>