diff mbox

apply-patches.sh: detect missing patches

Message ID 20130911120630.GA14745@harvey.netwinder.org
State Superseded
Headers show

Commit Message

Ralph Siemsen Sept. 11, 2013, 12:06 p.m. UTC 
Hi Thomas,

Sorry for delayed response.

On Thu, Sep 05, 2013 at 09:20:36PM +0200, Thomas De Schampheleire wrote:
> 
> Would you mind sending a second patch that changes the 'unknown patch
> format' case so that an error is thrown?

I've added it. Note that it is possible to fool this check, by using
for example a name like mydiff.patch.zzz. This matches "*.patch*" and
therefore is allowed, even though we don't know how to deal with ".zzz".

> Additionally, in your current patch, I would suggest changing the 'if
> test ...' into 'if [ ... ], for the sake of lining up with the style
> in the rest of the script.

Also done. Revised patch follows.

Cheers,
-Ralph

The return status of the "cat <patchfile> | patch ..." pipeline
is zero (success) even if the patchfile does not exist. This is
because patch receives no input, which is not an error condition.
Therefore, explicitly check that patch file exists.

Based on feedback on buildroot mailing list, also changed the
check for unsupported file format. The build will now error out,
rather than continuing on silently.
---
 support/scripts/apply-patches.sh |   10 +++++++---
 1 files changed, 7 insertions(+), 3 deletions(-)

Comments

Thomas De Schampheleire Sept. 12, 2013, 8:08 a.m. UTC | #1 
Hi Ralph,

On Wed, Sep 11, 2013 at 2:06 PM, Ralph Siemsen <ralphs@netwinder.org> wrote:
> Hi Thomas,
>
> Sorry for delayed response.
>
> On Thu, Sep 05, 2013 at 09:20:36PM +0200, Thomas De Schampheleire wrote:
>>
>> Would you mind sending a second patch that changes the 'unknown patch
>> format' case so that an error is thrown?
>
> I've added it. Note that it is possible to fool this check, by using
> for example a name like mydiff.patch.zzz. This matches "*.patch*" and
> therefore is allowed, even though we don't know how to deal with ".zzz".

True, I think this is something we cannot fix.
Some patches in the tree are named .patch.avr32, or
.patch.conditional, but they are in the regular patch format so should
be accepted by apply_patches.sh.

>
>> Additionally, in your current patch, I would suggest changing the 'if
>> test ...' into 'if [ ... ], for the sake of lining up with the style
>> in the rest of the script.
>
> Also done. Revised patch follows.

If you send a patch, the general format is:

commit message
--- (three dashes)
any other comments _not_ in the commit message

actual patch (diff --git ...)


The mail you sent is in the format:
other comments
commit message
actual patch

and this cannot be applied as such. Also, it makes the patch less
visible than in a separate mail.
So, please send it again, and put 'PATCH v2' in the subject line, so
it's easy to keep track of the different versions.

Thanks a lot,
Thomas
diff mbox

Patch

diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
index 7d5856c..d6e8983 100755
--- a/support/scripts/apply-patches.sh
+++ b/support/scripts/apply-patches.sh
@@ -70,13 +70,17 @@  function apply_patch {
 	*.patch*)
 	type="patch"; uncomp="cat"; ;;
 	*)
-	echo "Unsupported format file for ${patch}, skip it";
-	return 0;
+	echo "Unsupported format file for ${path}/${patch}";
+	return 1;
 	;;
     esac
     echo ""
     echo "Applying $patch using ${type}: "
-	echo $patch >> ${builddir}/.applied_patches_list
+    if [ ! -e "${path}/$patch" ] ; then
+	echo "Error: missing patch file ${path}/$patch"
+	return 1
+    fi
+    echo $patch >> ${builddir}/.applied_patches_list
     ${uncomp} "${path}/$patch" | patch -g0 -p1 -E -d "${builddir}" -t
     if [ $? != 0 ] ; then
         echo "Patch failed!  Please fix ${patch}!"