| Message ID | 20130913181527.GA11933@harvey.netwinder.org |
|---|---|
| State | Superseded |
| Headers | show |
Ralph, All, On 2013-09-13 14:15 -0400, Ralph Siemsen spake thusly: > 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. > --- > > Hi Thomas, > Hopefully the formatting is now correct. > -Ralph > > 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; I was a bit surprised to read that you used "return 1" here, rather than a more explicit "exit 1". For example, if the patch failed to apply cleanly, the function does not "return 1" but does "exit 1" (just the line after your patch ends). So I would prefer this behaviour to be homogeneous across the different code-paths. For the records, "return 1" does work since we call apply_patch thus: apply_patch "${patchfile}" || exit 1 This is not really straightforward. Just remove the "|| exit 1" and change all the "return 1" into "exit 1", so the script really errors out right away. Regards, Yann E. MORIN.
On Fri, Sep 13, 2013 at 08:27:02PM +0200, Yann E. MORIN wrote: > > > > 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; > > I was a bit surprised to read that you used "return 1" here, rather than > a more explicit "exit 1". As you mentioned, both "return" and "exit" work here, so I opted for the lesser change, figuring it would be less controversial ;) > This is not really straightforward. Just remove the "|| exit 1" and > change all the "return 1" into "exit 1", so the script really errors out > right away. I'll submit v3 with that fixed. -R
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}" if [ $? != 0 ] ; then echo "Patch failed! Please fix ${patch}!"