From mboxrd@z Thu Jan  1 00:00:00 1970
From: Michael Tremer <michael.tremer@ipfire.org>
To: development@lists.ipfire.org
Subject: Re: [PATCH] Improvement of backup iso script
Date: Mon, 02 Jan 2017 14:47:49 +0000
Message-ID: <1483368469.13949.330.camel@ipfire.org>
In-Reply-To: <1483367788-911-1-git-send-email-jonatan.schlag@ipfire.org>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="===============6129210189158300238=="
List-Id: <development.lists.ipfire.org>

--===============6129210189158300238==
Content-Type: text/plain; charset="utf-8"
Content-Transfer-Encoding: quoted-printable

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.
>=20
> 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 )
>=20
> Fixes: 11258
>=20
> Signed-off-by: Jonatan Schlag <jonatan.schlag(a)ipfire.org>
> ---
> =C2=A0src/scripts/backupiso | 31 ++++++++++++++++++++++++++-----
> =C2=A01 file changed, 26 insertions(+), 5 deletions(-)
>=20
> 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 @@
> =C2=A0#!/bin/sh
> +arch=3D$(uname -m)
> +
> +case $arch in
> +	"i586")
> +	arch=3D"i586"
> +	echo "Your arch is $arch"
> +	;;
> +	"i686")
> +	arch=3D"i586"
> +	echo "Your arch is $arch"
> +	;;
> +	"x86_64")
> +	arch=3D"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 wou=
ld
recommend to not use quotes for the matches.

I also indent it like this:

case x in
=C2=A0 =C2=A0a)
=C2=A0 =C2=A0 =C2=A0 some code
=C2=A0 =C2=A0 =C2=A0 ;;
=C2=A0 =C2=A0b)
=C2=A0 =C2=A0 =C2=A0 ...
=C2=A0 =C2=A0 =C2=A0 ;;
esac

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

It would be nice to know what 24 is for.

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

Why is this block executed in a sub-shell?

-Michael

--===============6129210189158300238==
Content-Type: application/pgp-signature
Content-Transfer-Encoding: base64
Content-Disposition: attachment; filename="signature.asc"
MIME-Version: 1.0

LS0tLS1CRUdJTiBQR1AgU0lHTkFUVVJFLS0tLS0KVmVyc2lvbjogR251UEcgdjIKCmlRSWNCQUFC
Q2dBR0JRSllhbWdWQUFvSkVJQjU4UDl2a0FrSG1tOFFBSkVwbHVpZTBaSHNNZGVmVUZNZkhHUXQK
cnc2ZDlIQXk0OUNNY3krczg4emdoenBVMUdBNkdYUnpocjA2a2pqUlc1T0VmVUsvOFpuK0pKRHFH
NFRLU2lEVApWc0pMQmpDTmFCLzZzUGlETFBHWjMrbzFiUXdJYjQxeEdQN3E4T1c1ZjhVY3ZFWlRw
d1Z3TmI3YWVEVkRGZVlhCmh6bjJGRG53K0NBZFRHTFlqR3FFZU1helF6eDBpTGY0SVNDaFh5dVhR
ZXRWOVN6MUs0MHBwNUNHdGgvMm1wOUQKS1JSTk5jWUdUR3RZeS9JY0IwRDlhdFVOSVB4OCtnSE10
dkhpWVd1b2tXOCsxN0JoRHE5dUJvVWJ1S0RiNm5nTApheUoxNCtMRmk1RFFPTWVkalpWenVsTTB2
citGdFdqVS8rRG9oUytPaXMxNHZ2N2JFQ3hXVytyaDlBT0xRNllnCktJQ0pqdE1IN0RxNklPcFc4
QmNEZWZua3FwVmhQRHRzRkhraDRWcHY0YldEZzdoalErV1Q5L0MzOHA3alMrWU4KeW1XeUJld3dJ
VWRpNWtCZ1N5ZjJuYUNuUFhRTEF2TnRibjlKU0F0QUtDNWROZmJrQXJIcXVrazNuaTYxTmJObQpW
amJrZ0FqdlJXKzlFbFFNbk5tdXlEcTdXQ1FCcGEzTE13eTNyYTF3Nm4rV2hPa1VkNXU5M25qUFRN
T2t4eFBiCjVXeVdCdE5NMGFIcXlIYS96dmxmTWZ0V2ZFa1VWaTFXQmIvTm04dDVxSjFQYjNISng5
SFd5aUtwZ0tNYWtKNFYKSkVtSFg1c0FYSHppM29NQ2ppZ2NXN1VpV1FGQU1vSzJjZzlhTXhCZVZ5
TUtUMEdVZWpMZ3RjVWtPTm5WWnhVaApsRlBoeWlEeDNhRGd5RE40WkpTLwo9SDVmVwotLS0tLUVO
RCBQR1AgU0lHTkFUVVJFLS0tLS0K

--===============6129210189158300238==--