Pkg xen devel - Dec 2020 - [PATCH 4/9] d/shuffle-boot-files: Rework boot shuffling script

Hi,

On 7/17/20 9:05 PM, Elliott Mitchell wrote:
> Rather than passing two strings which are strictly joined and used to
> replace filename versions, turn them into a simple version replacement.
Nice. This sounds like a good simplification of things.
> Rework the script logic to specifically target the version string in
> filenames and replace *that*.  This fixes an issues with some Xen files
> having names distinct enough names to not match the assumed format.
Ok, now I have to go search in the diff which lines are about changing
the strings handling, and which ones are about "rework" things...
Which
fixes "an issue". The puzzle extends itself.
> While at it use structures which conform to POSIX, allowing the use of
> /bin/sh instead of bash.
And now a third thing gets intertwined, and maybe there are even
multiple changes happening to a single line. I can't review this, I'm
not a shell scripting guru who can see through all of this at once, you
really have to break it down and help me here. And even if I was, this
whole single diff would be a real challenge to look at for anyone else
in the future who's debugging a problem with the packaging and ends up
in here.
> Signed-off-by: Elliott Mitchell <ehem+debian at m5p.com>
> ---
>  debian/rules              |  2 +-
>  debian/shuffle-boot-files | 26 ++++++++++++++------------
>  2 files changed, 15 insertions(+), 13 deletions(-)
> 
> diff --git a/debian/rules b/debian/rules
> index 3b7bb67550..628ebe9c09 100755
> --- a/debian/rules
> +++ b/debian/rules
> @@ -270,7 +270,7 @@ xenstore_rm = $(addprefix debian/xen-utils-common/,		\
>  override_dh_install:
>  	debian/shuffle-binaries $(upstream_version)
>  	:
> -	debian/shuffle-boot-files $(upstream_version) $(flavour)
> +	debian/shuffle-boot-files $(upstream_version)-$(flavour)
>  	:
>  	dh_install $(dh_install_excludes)
>  	if test -d debian/xen-utils-common; then rm -v $(xenstore_rm); fi
> diff --git a/debian/shuffle-boot-files b/debian/shuffle-boot-files
> index f7492c0c72..7338032fe6 100755
> --- a/debian/shuffle-boot-files
> +++ b/debian/shuffle-boot-files
> @@ -1,14 +1,13 @@
> -#!/bin/bash
> +#!/bin/sh
>  
>  set -e
>  
> -version=$1; shift
> -flavour=$1; shift
> +newvers=$1; shift
>  t=debian/tmp
>  
> -hv=xen-hypervisor-$version-$flavour
> -dest=debian/$hv/boot
> -mkdir -p $dest
> +hv="xen-hypervisor-$newvers"
> +dest="debian/$hv/boot"
> +mkdir -p "$dest"
>  
>  # The upstream build system puts a pile of needless symlinks in /boot.
>  #
> @@ -21,11 +20,14 @@ mkdir -p $dest
>  # more useful when the -i386 flavour existed and was coinstallable
>  # with the -amd64 flavour.)
>  
> -verstring=$(readlink debian/tmp/boot/xen.gz ||
> -            readlink debian/tmp/boot/xen)
> -verstring=${verstring##*/}
> -verstring=${verstring%.gz}
> +xenvers="$(readlink debian/tmp/boot/xen.?z* ||
> +            readlink debian/tmp/boot/xen)"
> +xenvers="${xenvers##*xen-}"
> +xenvers="${xenvers%.?z*}"
>  
> -for f in `cd $t/boot && find * -type f -print`; do
> -	cp -v $t/boot/$f $dest/${f/$verstring/xen-$version-$flavour}
> +find "$t/boot" -type f -print | while read f
> +do
> +	head="${f%$xenvers*}"
> +	tail="${f#*$xenvers}"
> +	cp -vl "$f" "$dest${head#$t/boot}$newvers${tail}"
>  done
> 
K

On Wed, Dec 02, 2020 at 12:20:49AM +0100, Hans van Kranenburg
wrote:
> On 7/17/20 9:05 PM, Elliott Mitchell wrote:
> 
> > While at it use structures which conform to POSIX, allowing the use of
> > /bin/sh instead of bash.
> 
> And now a third thing gets intertwined, and maybe there are even
> multiple changes happening to a single line. I can't review this,
I'm
> not a shell scripting guru who can see through all of this at once, you
> really have to break it down and help me here. And even if I was, this
> whole single diff would be a real challenge to look at for anyone else
> in the future who's debugging a problem with the packaging and ends up
> in here.
Sometimes there are small tiny little bits which don't really feel like
they need their own commits.  It really felt like one substantive thing,
plus a few tiny little niggling bits to get out of the way while at it.

I read what you typed as desiring things the way they were redone in the
latest, smash the commits into the smallest bits possible.  I tend to
think many of those smaller bits don't rate their own commits, but I'm
willing to break things into the smallest sensible bits if possible.


-- 
(\___(\___(\______          --=> 8-) EHM <=--          ______/)___/)___/)
 \BS (    |         ehem+sigmsg at m5p.com  PGP 87145445         |    )   /
  \_CS\   |  _____  -O #include <stddisclaimer.h> O-   _____  |   /  _/
8A19\___\_|_/58D2 7E3D DDF4 7BA6 <-PGP-> 41D1 B375 37D0 8714\_|_/___/5445