Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Yann E. MORIN <yann.morin.1998@free.fr>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] support/dependencies, scripts: accept patches with renames
Date: Wed, 19 May 2021 08:43:57 +0200	[thread overview]
Message-ID: <20210519064357.GB2268078@scaer> (raw)
In-Reply-To: <20210519032848.162695-1-pojiro.jp@gmail.com>

Ryota, All,

On 2021-05-19 12:28 +0900, pojiro.jp at gmail.com spake thusly:
> From: Ryota Kinukawa <pojiro.jp@gmail.com>
> 
> Currently, patches with renames are refused, as they reqire patch >= 2.7.
> So far, we did not require that version because it was too recent to be widely available.
> 
> But patch 2.7 has been released in 2012, almost 9 years ago now;
> it is old enough that we can start relying on it.
> 
> Add a check that patch is 2.7 or newer, and drop the check about
> renames in apply-patches.sh.
> 
> Signed-off-by: Ryota Kinukawa <pojiro.jp@gmail.com>

Thanks for the respin! :-)

Following Arnouts comments, some of my previous ones no longer applied,
so I did a few rework, see below...

> ---
>  support/dependencies/dependencies.sh | 12 +++++++++++-
>  support/scripts/apply-patches.sh     |  5 -----
>  2 files changed, 11 insertions(+), 6 deletions(-)
> 
> diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
> index 1954f038be..5aefc5c54a 100755
> --- a/support/dependencies/dependencies.sh
> +++ b/support/dependencies/dependencies.sh
> @@ -183,11 +183,21 @@ if test "${missing_progs}" = "yes" ; then
>  	exit 1
>  fi
>  
> +PATCH_VERSION="$(patch -v | sed -n 's/^GNU patch \(.*\)/\1/p')"

Since we now have an explicit check like that one, dropping the common
one, as you did initially, was indeed a good idea; I've done that Sorry
for misleading you on that part.

It also needs redirection 2>/dev/null to ignore errors (e.g. missing
patch, or patch that does not know of -v).

>  # apply-patches.sh needs patch with --no-backup-if-mismatch support (GNU, busybox w/DESKTOP)

This comment is now incorrect, because we no longer accept any patch
that is not explicitly GNU patch, so busybox w/desktop is no longer
accepted.

> -if ! patch --no-backup-if-mismatch </dev/null 2>/dev/null; then
> +if [ -z "${PATCH_VERSION}" ] ; then
> +	echo
>  	echo "Your patch program does not support the --no-backup-if-mismatch option. Install GNU patch"

The error message is now explicitlely about not being GNU patch, so I've
shorten it to: "You must install GNU patch"

>  	exit 1
>  fi
> +PATCH_VERSION_MAJOR="$(echo "${PATCH_VERSION}" | cut -d . -f 1)"
> +PATCH_VERSION_MINOR="$(echo "${PATCH_VERSION}" | cut -d . -f 2)"
> +if [ "${PATCH_VERSION_MAJOR}" -lt 2 ] || \
> +   [ "${PATCH_VERSION_MAJOR}" -eq 2 -a "${PATCH_VERSION_MINOR}" -lt 7 ] ; then

Aha, I see you used the long variable names I used in my review, and now
the line is too long. ;-) I've reverted to the original names you were
using, because they are explicit enough, and they also match other parts
of the script, and makes the line short enough again. :-)

> +	echo
> +	echo "You have GNU patch '$PATCH_VERSION' installed.  GNU patch >=2.7 is required"

You forgot to use curly braces for the expansion here.

Applied to next, with the few changes above, thanks! :-)

Regards,
Yann E. MORIN.

> +	exit 1;
> +fi
>  
>  if grep ^BR2_NEEDS_HOST_UTF8_LOCALE=y $BR2_CONFIG > /dev/null; then
>  	if ! which locale > /dev/null ; then
> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh
> index 9fb488c570..e5a2fdd09e 100755
> --- a/support/scripts/apply-patches.sh
> +++ b/support/scripts/apply-patches.sh
> @@ -113,11 +113,6 @@ function apply_patch {
>          echo "  to be applied  : ${path}/${patch}"
>          exit 1
>      fi
> -    if ${uncomp} "${path}/$patch" | grep -q "^rename from" && \
> -       ${uncomp} "${path}/$patch" | grep -q "^rename to" ; 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 --no-backup-if-mismatch -d "${builddir}" -t -N $silent
>      if [ $? != 0 ] ; then
> -- 
> 2.25.1
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-- 
.-----------------.--------------------.------------------.--------------------.
|  Yann E. MORIN  | Real-Time Embedded | /"\ ASCII RIBBON | Erics' conspiracy: |
| +33 662 376 056 | Software  Designer | \ / CAMPAIGN     |  ___               |
| +33 561 099 427 `------------.-------:  X  AGAINST      |  \e/  There is no  |
| http://ymorin.is-a-geek.org/ | _/*\_ | / \ HTML MAIL    |   v   conspiracy.  |
'------------------------------^-------^------------------^--------------------'

  reply	other threads:[~2021-05-19  6:43 UTC|newest]

Thread overview: 15+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-05-12 23:27 [Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames" Ryota Kinukawa
2021-05-15  6:44 ` Peter Korsgaard
2021-05-15  9:40   ` Yann E. MORIN
2021-05-15 10:57     ` Romain Naour
2021-05-15 11:19       ` Yann E. MORIN
2021-05-15 11:46         ` Romain Naour
2021-05-15 12:01           ` Yann E. MORIN
2021-05-16  0:51     ` Ryota Kinukawa
2021-05-16  8:18     ` Ryota Kinukawa
2021-05-17 12:46       ` Yann E. MORIN
2021-05-17 18:55       ` Arnout Vandecappelle
2021-05-19  3:28     ` [Buildroot] [PATCH] support/dependencies, scripts: accept patches with renames pojiro.jp at gmail.com
2021-05-19  6:43       ` Yann E. MORIN [this message]
2021-05-19  8:34     ` [Buildroot] [PATCH v2] " pojiro.jp at gmail.com
2021-05-19  8:46       ` Yann E. MORIN

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=20210519064357.GB2268078@scaer \
    --to=yann.morin.1998@free.fr \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox