All of 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 an external index of several public inboxes,
see mirroring instructions on how to clone and mirror
all data and code used by this external index.