* Re: [PATCH] Improvement of backup iso script
[not found] <1483369499.2414.6@mail01.ipfire.org>
@ 2017-01-02 15:07 ` Michael Tremer
0 siblings, 0 replies; 3+ messages in thread
From: Michael Tremer @ 2017-01-02 15:07 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 3935 bytes --]
On Mon, 2017-01-02 at 16:04 +0100, Jonatan Schlag wrote:
>
> Am Mo, 2. Jan, 2017 um 3:47 schrieb Michael Tremer
> <michael.tremer(a)ipfire.org>:
> >
> > 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
>
> I will send a new patch with better indention.
> >
> >
> > >
> > > 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
> > >
> Here 1
>
> >
> > >
> > >
> > > -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.
>
> Yes, but I do not know it and after some search in the web I did not
> found a explanation.
> I would keep the 24 till we can sure why it was added and If we could
> remove it without problems.
> >
> >
> >
> > >
> > > @@ -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?
>
> To fail gracefully (Here1) when /var/tmp/backupiso not exists.
That should never exist. Please use mktemp to create a temporary file.
-Michael
>
> >
> >
> >
> >
> > -Michael
>
> Jonatan
>
[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 819 bytes --]
^ permalink raw reply [flat|nested] 3+ messages in thread
* [PATCH] Improvement of backup iso script
@ 2017-01-02 14:36 Jonatan Schlag
2017-01-02 14:47 ` Michael Tremer
0 siblings, 1 reply; 3+ messages in thread
From: Jonatan Schlag @ 2017-01-02 14:36 UTC (permalink / raw)
To: development
[-- Attachment #1: Type: text/plain, Size: 2029 bytes --]
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
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
@@ -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}
+)
--
2.1.4
^ permalink raw reply [flat|nested] 3+ messages in thread* Re: [PATCH] Improvement of backup iso script
2017-01-02 14:36 Jonatan Schlag
@ 2017-01-02 14:47 ` Michael Tremer
0 siblings, 0 replies; 3+ messages in thread
From: Michael Tremer @ 2017-01-02 14:47 UTC (permalink / raw)
To: development
[-- 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 --]
^ permalink raw reply [flat|nested] 3+ messages in thread
end of thread, other threads:[~2017-01-02 15:07 UTC | newest]
Thread overview: 3+ messages (download: mbox.gz / follow: Atom feed)
-- links below jump to the message on this page --
[not found] <1483369499.2414.6@mail01.ipfire.org>
2017-01-02 15:07 ` [PATCH] Improvement of backup iso script Michael Tremer
2017-01-02 14:36 Jonatan Schlag
2017-01-02 14:47 ` Michael Tremer
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox