Re: [PATCH] Improvement of backup iso script

[Michael Tremer <michael.tremer@ipfire.org>]

[-- Attachment #1: Type: text/plain, Size: 2766 bytes --]

Hi,

thanks for having a look at this :)

On Mon, 2017-01-02 at 15:36 +0100, Jonatan Schlag wrote:
> The backup iso script did not check the arch of the host. On x86_64 host
> the wrong iso was downloaded.
> 
> Furthermore, there were some if clauses which could cause trouble which
> I also tried to improve.
> (For example: -e is valid if we have a directory or a file, but we want
> to check for a file only )
> 
> Fixes: 11258
> 
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
>  src/scripts/backupiso | 31 ++++++++++++++++++++++++++-----
>  1 file changed, 26 insertions(+), 5 deletions(-)
> 
> diff --git a/src/scripts/backupiso b/src/scripts/backupiso
> index 014e8e9..e92f385 100644
> --- a/src/scripts/backupiso
> +++ b/src/scripts/backupiso
> @@ -1,9 +1,29 @@
>  #!/bin/sh
> +arch=$(uname -m)
> +
> +case $arch in
> +	"i586")
> +	arch="i586"
> +	echo "Your arch is $arch"
> +	;;
> +	"i686")
> +	arch="i586"
> +	echo "Your arch is $arch"
> +	;;
> +	"x86_64")
> +	arch="x86_64"
> +	echo "Your arch is $arch"
> +	;;
> +	*)
> +	echo "Arch is not supported"
> +	exit 1
> +	;;
> +esac

You could make this a bit shorter by using i?86 for i586 and i686. I also would
recommend to not use quotes for the matches.

I also indent it like this:

case x in
   a)
      some code
      ;;
   b)
      ...
      ;;
esac

>  COREVER=$(cat /opt/pakfire/db/core/mine)
>  # FIXME: edit this lines before release
>  URL="http://download.ipfire.org/releases/ipfire-2.x/2.19-core$COREVER/"
> -ISO="ipfire-2.19.i586-full-core$COREVER.iso"
> +ISO="ipfire-2.19.$arch-full-core$COREVER.iso"
>  
>  if [ -z $1 ]; then
>  	echo usage: $0 backup-file
> @@ -13,9 +33,9 @@ fi
>  TS=$1
>  
>  mkdir -p /var/tmp/backupiso
> -cd /var/tmp/backupiso
> +(cd /var/tmp/backupiso || exit
>  
> -if [ ! -e ${ISO} ]
> +if [ ! -f ${ISO} ]
>  then
>  	echo "Fetching ${URL}${ISO}"
>  	wget --quiet -c ${URL}${ISO}
> @@ -26,7 +46,7 @@ wget --quiet -O ${ISO}.md5 ${URL}${ISO}.md5
>  
>  echo "Checking md5 of ${ISO}"
>  md5sum --status -c ${ISO}.md5
> -if [ $? -eq 0 -o $? -eq 24 ]
> +if [ $? -eq 0 ] || [ $? -eq 24 ]
>  then
>  	echo "md5 is OK"
>  else

It would be nice to know what 24 is for.

> @@ -35,7 +55,7 @@ else
>  	wget --quiet -O ${ISO} ${URL}${ISO}
>  	echo "Checking again md5 of ${ISO}"
>  	md5sum --status -c ${ISO}.md5
> -	if [ $? -eq 0 -o $? -eq 24 ]
> +	if [ $? -eq 0 ] || [ $? -eq 24 ]
>  	then
>  		echo "md5 is OK"
>  	else
> @@ -64,3 +84,4 @@ isohybrid $(basename ${ISO} .iso)-${TS}.iso
>  
>  echo "Cleaning up"
>  rm -rf backupiso.${TS}
> +)

Why is this block executed in a sub-shell?

-Michael

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]