diff mbox

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

Message ID 1464178439-3333-1-git-send-email-sebastien.szymanski@armadeus.com
State Rejected
Headers show

Commit Message

Sébastien Szymanski May 25, 2016, 12:13 p.m. UTC
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(-)

Comments

Thomas Petazzoni May 25, 2016, 3:11 p.m. UTC | #1
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
Sébastien Szymanski May 25, 2016, 4:04 p.m. UTC | #2
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
>
Romain Naour July 5, 2016, 10:40 a.m. UTC | #3
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"
>
Romain Naour July 5, 2016, 11:05 a.m. UTC | #4
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"
>
diff mbox

Patch

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"