[v2] support/scripts/apply-patches.sh: do not apply patches with renames

Submitted by Thomas Petazzoni on June 24, 2017, 7:59 p.m.

Details

Message ID 20170624195936.31528-1-thomas.petazzoni@free-electrons.com
State New
Headers show

Commit Message

Thomas Petazzoni June 24, 2017, 7:59 p.m.
Patches with renames apply properly with patch >= 2.7, but not with
older patch versions. Since "git format-patch" by default generates
patches with renames, Buildroot developers often don't realize that
their patches will not apply properly on build machines that have
patch < 2.7. In order to prevent such a situation from happening
again, this commit adds some logic in apply-patches.sh to refuse
applying patches that contain renames.

Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>
---
Changes since v1:
 - Use "&&" for the "grep -q && grep -q" condition, as noticed by Yann
   E. Morin.
---
 support/scripts/apply-patches.sh | 5 +++++
 1 file changed, 5 insertions(+)

Comments

Arnout Vandecappelle June 24, 2017, 10:08 p.m.
On 24-06-17 21:59, Thomas Petazzoni wrote:
> Patches with renames apply properly with patch >= 2.7, but not with
> older patch versions. Since "git format-patch" by default generates
> patches with renames, Buildroot developers often don't realize that
> their patches will not apply properly on build machines that have
> patch < 2.7. In order to prevent such a situation from happening
> again, this commit adds some logic in apply-patches.sh to refuse
> applying patches that contain renames.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

Reviewed-by: Arnout Vandecappelle (Essensium/Mind) <arnout@mind.be>

 Perhaps add to the commit message why you don't just grep for '^rename':

Since the patch commit message may contain the words "rename from" or
"rename to" as well, the grep expression is made as accurate as possible,
checking both.

 Ideally the test should start from the first ^diff, but that makes things a lot
more complicated...

 An alternative would be to do

patch --dry-run -i ${path}/${patch} | grep "(renamed from"

but I can't guarantee that that works with all patch versions that understand
rename. This the current patch is OK.

 Regards,
 Arnout


> ---
> Changes since v1:
>  - Use "&&" for the "grep -q && grep -q" condition, as noticed by Yann
>    E. Morin.
> ---
>  support/scripts/apply-patches.sh | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> index 7ccb39d..0c24175 100755
> --- a/support/scripts/apply-patches.sh
> +++ b/support/scripts/apply-patches.sh
> @@ -113,6 +113,11 @@ function apply_patch {
>          echo "  to be applied  : ${path}/${patch}"
>          exit 1
>      fi
> +    if grep -q "^rename from" ${path}/${patch} && \
> +       grep -q "^rename to" ${path}/${patch} ; then
> +	echo "Error: patch contains some renames, not supported by old patch versions"
> +	exit 1
> +    fi
>      echo "${path}/${patch}" >> ${builddir}/.applied_patches_list
>      ${uncomp} "${path}/$patch" | patch -g0 -p1 -E -d "${builddir}" -t -N $silent
>      if [ $? != 0 ] ; then
>
Yann E. MORIN June 25, 2017, 8:42 a.m.
Thomas, All,

On 2017-06-24 21:59 +0200, Thomas Petazzoni spake thusly:
> Patches with renames apply properly with patch >= 2.7, but not with
> older patch versions. Since "git format-patch" by default generates
> patches with renames, Buildroot developers often don't realize that
> their patches will not apply properly on build machines that have
> patch < 2.7. In order to prevent such a situation from happening
> again, this commit adds some logic in apply-patches.sh to refuse
> applying patches that contain renames.
> 
> Signed-off-by: Thomas Petazzoni <thomas.petazzoni@free-electrons.com>

A small nit below...

> ---
> Changes since v1:
>  - Use "&&" for the "grep -q && grep -q" condition, as noticed by Yann
>    E. Morin.
> ---
>  support/scripts/apply-patches.sh | 5 +++++
>  1 file changed, 5 insertions(+)
> 
> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> index 7ccb39d..0c24175 100755
> --- a/support/scripts/apply-patches.sh
> +++ b/support/scripts/apply-patches.sh
> @@ -113,6 +113,11 @@ function apply_patch {
>          echo "  to be applied  : ${path}/${patch}"
>          exit 1
>      fi
> +    if grep -q "^rename from" ${path}/${patch} && \
> +       grep -q "^rename to" ${path}/${patch} ; then
> +	echo "Error: patch contains some renames, not supported by old patch versions"
> +	exit 1

This file a space-leading, but here you have a tab...

Otherwise:

Reviewed-by: "Yann E. MORIN" <yann.morin.1998@free.fr>

Regards,
Yann E. MORIN.

> +    fi
>      echo "${path}/${patch}" >> ${builddir}/.applied_patches_list
>      ${uncomp} "${path}/$patch" | patch -g0 -p1 -E -d "${builddir}" -t -N $silent
>      if [ $? != 0 ] ; then
> -- 
> 2.9.4
>

Patch hide | download patch | download mbox

diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
index 7ccb39d..0c24175 100755
--- a/support/scripts/apply-patches.sh
+++ b/support/scripts/apply-patches.sh
@@ -113,6 +113,11 @@  function apply_patch {
         echo "  to be applied  : ${path}/${patch}"
         exit 1
     fi
+    if grep -q "^rename from" ${path}/${patch} && \
+       grep -q "^rename to" ${path}/${patch} ; then
+	echo "Error: patch contains some renames, not supported by old patch versions"
+	exit 1
+    fi
     echo "${path}/${patch}" >> ${builddir}/.applied_patches_list
     ${uncomp} "${path}/$patch" | patch -g0 -p1 -E -d "${builddir}" -t -N $silent
     if [ $? != 0 ] ; then