[Buildroot] [PATCH 1/1] support/scripts/apply-patches.sh: handle tar and mkdir failures

[Buildroot] [PATCH 1/1] support/scripts/apply-patches.sh: handle tar and mkdir failures

[Sébastien Szymanski]

With several archives in a space-separated list of patches, if one archive
cannot be untar (i.e because it is not readable), Buildroot keeps going instead
of aborting. Fix this by checking the tar return code.
While at it, check if the unpack archive directory creation is successful.

Signed-off-by: S?bastien Szymanski <sebastien.szymanski@armadeus.com>
---
 support/scripts/apply-patches.sh | 6 +++++-
 1 file changed, 5 insertions(+), 1 deletion(-)

diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
index 201278d..b0a8ad4 100755
--- a/support/scripts/apply-patches.sh
+++ b/support/scripts/apply-patches.sh
@@ -132,7 +132,11 @@ function scan_patchdir {
                 unpackedarchivedir="$builddir/.patches-$(basename $i)-unpacked"
                 rm -rf "$unpackedarchivedir" 2> /dev/null
                 mkdir "$unpackedarchivedir"
-                tar -C "$unpackedarchivedir" -xaf "${path}/$i"
+                if [ $? != 0 ] ; then
+                    echo "Error: cannot create $unpackedarchivedir directory."
+                    exit 1
+                fi
+                tar -C "$unpackedarchivedir" -xaf "${path}/$i" || exit 1
                 scan_patchdir "$unpackedarchivedir"
             else
                 apply_patch "$path" "$i"
-- 
2.3.6

[Buildroot] [PATCH 1/1] support/scripts/apply-patches.sh: handle tar and mkdir failures

[Thomas Petazzoni]

Hello,

On Wed, 25 May 2016 14:13:59 +0200, S?bastien Szymanski wrote:
> With several archives in a space-separated list of patches, if one archive
> cannot be untar (i.e because it is not readable), Buildroot keeps going instead
> of aborting. Fix this by checking the tar return code.
> While at it, check if the unpack archive directory creation is successful.
> 
> Signed-off-by: S?bastien Szymanski <sebastien.szymanski@armadeus.com>
> ---
>  support/scripts/apply-patches.sh | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)

Thanks for the patch. One question though: are you actually using the
capability of Buildroot to take a tarball, and apply the patches that
it contains? If so, for what use case?

Thanks,

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] [PATCH 1/1] support/scripts/apply-patches.sh: handle tar and mkdir failures

[Sébastien Szymanski]

Hi,

On 05/25/2016 05:11 PM, Thomas Petazzoni wrote:
> Hello,
> 
> On Wed, 25 May 2016 14:13:59 +0200, S?bastien Szymanski wrote:
>> With several archives in a space-separated list of patches, if one archive
>> cannot be untar (i.e because it is not readable), Buildroot keeps going instead
>> of aborting. Fix this by checking the tar return code.
>> While at it, check if the unpack archive directory creation is successful.
>>
>> Signed-off-by: S?bastien Szymanski <sebastien.szymanski@armadeus.com>
>> ---
>>  support/scripts/apply-patches.sh | 6 +++++-
>>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> Thanks for the patch. One question though: are you actually using the
> capability of Buildroot to take a tarball, and apply the patches that
> it contains? If so, for what use case?
> 

Yes, we use this Buildroot feature. We have an old kernel with several
tarballs of patches for our i.MX28-based SoM. They was provided that way
by Freescale I guess.

Regards,

> Thanks,
> 
> Thomas
> 


-- 
S?bastien Szymanski
Software Engineer
Armadeus Systems - A new vision of the embedded world
sebastien.szymanski at armadeus.com
Tel: +33 (0)9 72 29 41 44
Fax: +33 (0)9 72 28 79 26

[Buildroot] [PATCH 1/1] support/scripts/apply-patches.sh: handle tar and mkdir failures

[Romain Naour]

Hi S?bastien,

Le 25/05/2016 ? 14:13, S?bastien Szymanski a ?crit :
> With several archives in a space-separated list of patches, if one archive
> cannot be untar (i.e because it is not readable), Buildroot keeps going instead
> of aborting. Fix this by checking the tar return code.
> While at it, check if the unpack archive directory creation is successful.
> 
> Signed-off-by: S?bastien Szymanski <sebastien.szymanski@armadeus.com>
> ---
>  support/scripts/apply-patches.sh | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> index 201278d..b0a8ad4 100755
> --- a/support/scripts/apply-patches.sh
> +++ b/support/scripts/apply-patches.sh
> @@ -132,7 +132,11 @@ function scan_patchdir {
>                  unpackedarchivedir="$builddir/.patches-$(basename $i)-unpacked"
>                  rm -rf "$unpackedarchivedir" 2> /dev/null
>                  mkdir "$unpackedarchivedir"
> -                tar -C "$unpackedarchivedir" -xaf "${path}/$i"
> +                if [ $? != 0 ] ; then
> +                    echo "Error: cannot create $unpackedarchivedir directory."
> +                    exit 1
> +                fi

I don't see why mkdir can fail here.

> +                tar -C "$unpackedarchivedir" -xaf "${path}/$i" || exit 1

Ok with the check.

Best regards,
Romain


>                  scan_patchdir "$unpackedarchivedir"
>              else
>                  apply_patch "$path" "$i"
> 

[Buildroot] [PATCH 1/1] support/scripts/apply-patches.sh: handle tar and mkdir failures

[Romain Naour]

Hi S?bastien,

Le 25/05/2016 ? 14:13, S?bastien Szymanski a ?crit :
> With several archives in a space-separated list of patches, if one archive
> cannot be untar (i.e because it is not readable), Buildroot keeps going instead
> of aborting. Fix this by checking the tar return code.
> While at it, check if the unpack archive directory creation is successful.

As discussed with other Buildroot developers, we prefer to use "set -e" in
apply-patches.sh

See:
http://patchwork.ozlabs.org/patch/644705

I'll mark you patch rejected in patchwork.

Thanks!

Best regards,
Romain


> 
> Signed-off-by: S?bastien Szymanski <sebastien.szymanski@armadeus.com>
> ---
>  support/scripts/apply-patches.sh | 6 +++++-
>  1 file changed, 5 insertions(+), 1 deletion(-)
> 
> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> index 201278d..b0a8ad4 100755
> --- a/support/scripts/apply-patches.sh
> +++ b/support/scripts/apply-patches.sh
> @@ -132,7 +132,11 @@ function scan_patchdir {
>                  unpackedarchivedir="$builddir/.patches-$(basename $i)-unpacked"
>                  rm -rf "$unpackedarchivedir" 2> /dev/null
>                  mkdir "$unpackedarchivedir"
> -                tar -C "$unpackedarchivedir" -xaf "${path}/$i"
> +                if [ $? != 0 ] ; then
> +                    echo "Error: cannot create $unpackedarchivedir directory."
> +                    exit 1
> +                fi
> +                tar -C "$unpackedarchivedir" -xaf "${path}/$i" || exit 1
>                  scan_patchdir "$unpackedarchivedir"
>              else
>                  apply_patch "$path" "$i"
>