[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Ryota Kinukawa]

Hi. Thank you for your great work, Buildroot team.
I'm writing this email for suggestions to support/scripts/apply-patches.sh.

Recently, I was working with buildroot to apply patches-4.14.164-rt73.
It included a patch((patches-4.14.164-rt73/0462-..)) which renames a file.
This caused the build to terminate with an error.

The reason for this is that apply-patches.sh exits when it finds a patch
that contains a rename.
This feature is kind for users who use legacy `patch` commands,
but not kind for current users.

So I'd like to suggest adding a kind of this line
  `grep -q "^BR2_*_LEGACY_PATCH_CMD=y" "$BR2_CONFIG" && \`
to below if statement.

```
if ${uncomp} "${path}/$patch" | grep -q "^rename from" && \
   ${uncomp} "${path}/$patch" | grep -q "^rename to" ; then
```

Parsing the results of the `patch` is dependent on the implementation of it,
so I think it would be better to get them who use `legacy patch` to set it
explicitly.
(For considering backward compatibility,
(it might be better if the environment variable had the opposite meaning.

How about this idea?

In making this suggestion I have read the following two threads.
- http://lists.busybox.net/pipermail/buildroot/2017-May/191794.html
- http://lists.busybox.net/pipermail/buildroot/2017-June/195335.html

Thank you.
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20210513/2cfdb0db/attachment.html>

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Peter Korsgaard]

>>>>> "Ryota" == Ryota Kinukawa <pojiro.jp@gmail.com> writes:

Hi,

 > Hi. Thank you for your great work, Buildroot team.
 > I'm writing this email for suggestions to support/scripts/apply-patches.sh.

 > Recently, I was working with buildroot to apply patches-4.14.164-rt73.
 > It included a patch((patches-4.14.164-rt73/0462-..)) which renames a file.
 > This caused the build to terminate with an error.

 > The reason for this is that apply-patches.sh exits when it finds a patch
 > that contains a rename.
 > This feature is kind for users who use legacy `patch` commands,
 > but not kind for current users.

 > So I'd like to suggest adding a kind of this line
 >   `grep -q "^BR2_*_LEGACY_PATCH_CMD=y" "$BR2_CONFIG" && \`
 > to below if statement.

 > ```
 > if ${uncomp} "${path}/$patch" | grep -q "^rename from" && \
 >    ${uncomp} "${path}/$patch" | grep -q "^rename to" ; then
 > ```

 > Parsing the results of the `patch` is dependent on the implementation of it,
 > so I think it would be better to get them who use `legacy patch` to set it
 > explicitly.
 > (For considering backward compatibility,
 > (it might be better if the environment variable had the opposite meaning.

 > How about this idea?

The problem with such an option is that it describes a property of the
build host, not of the Buildroot configuration - And we still wouldn't
be able to use rename patches as long as that option was supported.

So we want apply-patches.sh to work the same for everyone.

But we're now 4 years later, so perhaps it is time to unconditionally
drop support for GNU patch < 2.7?

GNU patch 2.7 was released in September 2012, so 8.5 years ago:

https://savannah.gnu.org/forum/forum.php?forum_id=7361

Thomas, Yann - Any ideas how common such old patch version are nowadays?

-- 
Bye, Peter Korsgaard

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Yann E. MORIN]

Peter, Ryota, All,

On 2021-05-15 08:44 +0200, Peter Korsgaard spake thusly:
> >>>>> "Ryota" == Ryota Kinukawa <pojiro.jp@gmail.com> writes:
>  > Hi. Thank you for your great work, Buildroot team.
>  > I'm writing this email for suggestions to support/scripts/apply-patches.sh.
> 
>  > Recently, I was working with buildroot to apply patches-4.14.164-rt73.
>  > It included a patch((patches-4.14.164-rt73/0462-..)) which renames a file.
>  > This caused the build to terminate with an error.
[--SNIP--]
> The problem with such an option is that it describes a property of the
> build host, not of the Buildroot configuration - And we still wouldn't
> be able to use rename patches as long as that option was supported.
> 
> So we want apply-patches.sh to work the same for everyone.
> 
> But we're now 4 years later, so perhaps it is time to unconditionally
> drop support for GNU patch < 2.7?
> 
> GNU patch 2.7 was released in September 2012, so 8.5 years ago:
> 
> https://savannah.gnu.org/forum/forum.php?forum_id=7361
> 
> Thomas, Yann - Any ideas how common such old patch version are nowadays?

One of the oldest supported distro that was rel;eased before patch 2.7,
is RHEL 6. It has entered Extended Support in late 2020, and is expected
to be supported until mid 2024. However, consdering this is an old
distro, Iand that there were CVes on patch < 2.7 I'd expect that RHEL 6
now packages a newer patch that is >= 2.7.

People that do not update their systems, and are stuck on so old a host
distro, will probably not update their Buildroot version either...

On the Ubuntu side (as another data point), the odest LTS is Trusty,
14.04, and it has patch 2.7.1.

As for Debian, the oldest still supported is Jessie, with Extended LTS
until mid 2022, and it was released in 2015, so presumably has patch >=
2.7 too.

So, I would be OK for requiring patch >= 2.7.

So, lets do it that way;

 1. add a check in support/dependencies/dependencies.sh that patch >=
    2.7 (just after/before the check for --no-backup-if-mismatch)

 2. drop the test for renames in support/scripts/apply-patches.sh

Ryota, would you care to work on that and send the patches to the list,
please?

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Romain Naour]

Hello Yann, All,

Le 15/05/2021 ? 11:40, Yann E. MORIN a ?crit?:
> Peter, Ryota, All,
> 
> On 2021-05-15 08:44 +0200, Peter Korsgaard spake thusly:
>>>>>>> "Ryota" == Ryota Kinukawa <pojiro.jp@gmail.com> writes:
>>  > Hi. Thank you for your great work, Buildroot team.
>>  > I'm writing this email for suggestions to support/scripts/apply-patches.sh.
>>
>>  > Recently, I was working with buildroot to apply patches-4.14.164-rt73.
>>  > It included a patch((patches-4.14.164-rt73/0462-..)) which renames a file.
>>  > This caused the build to terminate with an error.
> [--SNIP--]
>> The problem with such an option is that it describes a property of the
>> build host, not of the Buildroot configuration - And we still wouldn't
>> be able to use rename patches as long as that option was supported.
>>
>> So we want apply-patches.sh to work the same for everyone.
>>
>> But we're now 4 years later, so perhaps it is time to unconditionally
>> drop support for GNU patch < 2.7?
>>
>> GNU patch 2.7 was released in September 2012, so 8.5 years ago:
>>
>> https://savannah.gnu.org/forum/forum.php?forum_id=7361
>>
>> Thomas, Yann - Any ideas how common such old patch version are nowadays?
> 
> One of the oldest supported distro that was rel;eased before patch 2.7,
> is RHEL 6. It has entered Extended Support in late 2020, and is expected
> to be supported until mid 2024. However, consdering this is an old
> distro, Iand that there were CVes on patch < 2.7 I'd expect that RHEL 6
> now packages a newer patch that is >= 2.7.
> 
> People that do not update their systems, and are stuck on so old a host
> distro, will probably not update their Buildroot version either...
> 
> On the Ubuntu side (as another data point), the odest LTS is Trusty,
> 14.04, and it has patch 2.7.1.
> 
> As for Debian, the oldest still supported is Jessie, with Extended LTS
> until mid 2022, and it was released in 2015, so presumably has patch >=
> 2.7 too.
> 
> So, I would be OK for requiring patch >= 2.7.
> 
> So, lets do it that way;
> 
>  1. add a check in support/dependencies/dependencies.sh that patch >=
>     2.7 (just after/before the check for --no-backup-if-mismatch)

Based on the host gcc requirement on gcc 4.9 (released the 22 April 2014), we
already don't support such old distro. Ubuntu LTS Trusty provide gcc 4.8.

Best regards,
Romain

> 
>  2. drop the test for renames in support/scripts/apply-patches.sh
> 
> Ryota, would you care to work on that and send the patches to the list,
> please?
> 
> Regards,
> Yann E. MORIN.
> 

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Yann E. MORIN]

Romain, All,

On 2021-05-15 12:57 +0200, Romain Naour spake thusly:
> Le 15/05/2021 ? 11:40, Yann E. MORIN a ?crit?:
> > On the Ubuntu side (as another data point), the odest LTS is Trusty,
> > 14.04, and it has patch 2.7.1.
> Based on the host gcc requirement on gcc 4.9 (released the 22 April 2014), we
> already don't support such old distro. Ubuntu LTS Trusty provide gcc 4.8.

Ah, I don't thinks so. We still only require gcc >= 4.8:

  - the manual still references gcc 4.8, even the nightly snapshots:
        http://nightly.buildroot.org/#requirement

        2.1. Mandatory packages
        ...
        * gcc (version 4.8 or any later)
        * g++ (version 4.8 or any later) 

  - support/dependencies/dependencies.sh still only checks for gcc >=
    4.8:
        if [ $COMPILER_MAJOR -lt 4 -o $COMPILER_MAJOR -eq 4 -a $COMPILER_MINOR -lt 8 ] ; then

The last time (that I remember of) when we bumped our host gcc
requirement, was with those commits, dated October 2020:

  * 5ee815ab6a docs/manual: bump our gcc/g++ requirements to 4.8
  * 55fc80260b packages: host gcc >= 4.8 is now guaranteed

I could not find a reference to requiring host gcc >= 4.9 in the commit
logs...

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Romain Naour]

Yann, All,

Le 15/05/2021 ? 13:19, Yann E. MORIN a ?crit?:
> Romain, All,
> 
> On 2021-05-15 12:57 +0200, Romain Naour spake thusly:
>> Le 15/05/2021 ? 11:40, Yann E. MORIN a ?crit?:
>>> On the Ubuntu side (as another data point), the odest LTS is Trusty,
>>> 14.04, and it has patch 2.7.1.
>> Based on the host gcc requirement on gcc 4.9 (released the 22 April 2014), we
>> already don't support such old distro. Ubuntu LTS Trusty provide gcc 4.8.
> 
> Ah, I don't thinks so. We still only require gcc >= 4.8:

It seems the BR2_HOST_GCC_AT_LEAST_4_8 symbol has been removed with commit
55fc80260b packages: host gcc >= 4.8 is now guaranteed.

I just noticed it while looking at this discussion...

> 
>   - the manual still references gcc 4.8, even the nightly snapshots:
>         http://nightly.buildroot.org/#requirement
> 
>         2.1. Mandatory packages
>         ...
>         * gcc (version 4.8 or any later)
>         * g++ (version 4.8 or any later) 
> 
>   - support/dependencies/dependencies.sh still only checks for gcc >=
>     4.8:
>         if [ $COMPILER_MAJOR -lt 4 -o $COMPILER_MAJOR -eq 4 -a $COMPILER_MINOR -lt 8 ] ; then
> 
> The last time (that I remember of) when we bumped our host gcc
> requirement, was with those commits, dated October 2020:
> 
>   * 5ee815ab6a docs/manual: bump our gcc/g++ requirements to 4.8
>   * 55fc80260b packages: host gcc >= 4.8 is now guaranteed
> 
> I could not find a reference to requiring host gcc >= 4.9 in the commit
> logs...

Otherwise, agree gcc 4.8 should be the minimal supported version.

Best regards,
Romain


> 
> Regards,
> Yann E. MORIN.
> 

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Yann E. MORIN]

Romain, All,

On 2021-05-15 13:46 +0200, Romain Naour spake thusly:
> Le 15/05/2021 ? 13:19, Yann E. MORIN a ?crit?:
> > Romain, All,
> > 
> > On 2021-05-15 12:57 +0200, Romain Naour spake thusly:
> >> Le 15/05/2021 ? 11:40, Yann E. MORIN a ?crit?:
> >>> On the Ubuntu side (as another data point), the odest LTS is Trusty,
> >>> 14.04, and it has patch 2.7.1.
> >> Based on the host gcc requirement on gcc 4.9 (released the 22 April 2014), we
> >> already don't support such old distro. Ubuntu LTS Trusty provide gcc 4.8.
> > 
> > Ah, I don't thinks so. We still only require gcc >= 4.8:
> 
> It seems the BR2_HOST_GCC_AT_LEAST_4_8 symbol has been removed with commit
> 55fc80260b packages: host gcc >= 4.8 is now guaranteed.

Yes, the removal was on purpose: since gcc >= 4.8 is not guaranteed
(with a check in dependencies.sh), BR2_HOST_GCC_AT_LEAST_4_8 would
always be 'y'; there is no advantage to having a variable always set
to 'y', so we dropped it.

Regards,
Yann E. MORIN.

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Ryota Kinukawa]

Yann-san

> So, lets do it that way;
>  1. add a check in support/dependencies/dependencies.sh that patch >=
>     2.7 (just after/before the check for --no-backup-if-mismatch)
>  2. drop the test for renames in support/scripts/apply-patches.sh
> Ryota, would you care to work on that and send the patches to the list,
> please?


Thank you for the opportunity to contribute buildroot.
This is my first time to contribute to buildroot,
so it may take me a while to learn how to do it. But I'd like to, please
let me try.

Regards,
Ryota Kinukawa
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20210516/abefbef2/attachment.html>

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Ryota Kinukawa]

Yann-san, All

> So, lets do it that way;
>  1. add a check in support/dependencies/dependencies.sh that patch >=
>     2.7 (just after/before the check for --no-backup-if-mismatch)
>  2. drop the test for renames in support/scripts/apply-patches.sh
> Ryota, would you care to work on that and send the patches to the list,
> please?

Here is the patch, could you confirm this?

diff --git a/support/dependencies/dependencies.sh
b/support/dependencies/dependencies.sh
index 1954f038be..030a2196f9 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -163,7 +163,7 @@ fi

 # Check that a few mandatory programs are installed
 missing_progs="no"
-for prog in patch perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
+for prog in perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
        if ! which $prog > /dev/null ; then
                echo "You must install '$prog' on your build machine";
                missing_progs="yes"
@@ -183,11 +183,24 @@ if test "${missing_progs}" = "yes" ; then
        exit 1
 fi

+# Check patch
+if ! which patch > /dev/null ; then
+       echo "You must install GNU patch on your build machine";
+       exit 1
+fi
 # apply-patches.sh needs patch with --no-backup-if-mismatch support (GNU,
busybox w/DESKTOP)
 if ! patch --no-backup-if-mismatch </dev/null 2>/dev/null; then
        echo "Your patch program does not support the
--no-backup-if-mismatch option. Install GNU patch"
        exit 1
 fi
+PATCH_VERSION=$(patch --version 2>&1 | sed -e 's/^.* \([0-9\.]\)/\1/g' -e
's/[-\ ].*//g' -e '1q')
+PATCH_MAJOR=$(echo $PATCH_VERSION | sed -e "s/\..*//g")
+PATCH_MINOR=$(echo $PATCH_VERSION | sed -e "s/^$PATCH_MAJOR\.//g" -e
"s/\..*//g" -e "s/[a-zA-Z].*//g")
+if [ $PATCH_MAJOR -lt 2 ] || [ $PATCH_MAJOR -eq 2 -a $PATCH_MINOR -lt 7 ]
; then
+       echo
+       echo "You have GNU patch '$PATCH_VERSION' installed.  GNU patch
>=2.7 is required"
+       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

Regards,
Ryota Kinukawa
-------------- next part --------------
An HTML attachment was scrubbed...
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20210516/07366cfd/attachment.html>

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Yann E. MORIN]

Ryota, All,

Thanks for sending a patch so quickly! :-)

On 2021-05-16 17:18 +0900, Ryota Kinukawa spake thusly:
> > So, lets do it that way;
> > ?1. add a check in support/dependencies/dependencies.sh that patch >=
> > ? ? 2.7 (just after/before the check for --no-backup-if-mismatch)
> > ?2. drop the test for renames in support/scripts/apply-patches.sh
> > Ryota, would you care to work on that and send the patches to the list,
> > please?
> 
> Here is the patch, could you confirm this?

Please see my comments below. Also, please send it directly as its own
mail; using  "git send-email"  makes it easy to send a patch.

Also, please provide an actual commit log. For example:

    support/apply-patches: accept patches with renames

    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: Yor NAME <your@email>

> diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
> index 1954f038be..030a2196f9 100755
> --- a/support/dependencies/dependencies.sh
> +++ b/support/dependencies/dependencies.sh
> @@ -163,7 +163,7 @@ fi
> 
> ?# Check that a few mandatory programs are installed
> ?missing_progs="no"
> -for prog in patch perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
> +for prog in perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
> ? ? ? ? if ! which $prog > /dev/null ; then
> ? ? ? ? ? ? ? ? echo "You must install '$prog' on your build machine";
> ? ? ? ? ? ? ? ? missing_progs="yes"
> @@ -183,11 +183,24 @@ if test "${missing_progs}" = "yes" ; then
> ? ? ? ? exit 1
> ?fi
> 
> +# Check patch
> +if ! which patch > /dev/null ; then
> + ? ? ? echo "You must install GNU patch on your build machine";
> + ? ? ? exit 1
> +fi

There is no need for a dedicated check for patch; just leave it in the
list, above.

> ?# apply-patches.sh needs patch with --no-backup-if-mismatch support (GNU, busybox w/DESKTOP)
> ?if ! patch --no-backup-if-mismatch </dev/null 2>/dev/null; then
> ? ? ? ? echo "Your patch program does not support the --no-backup-if-mismatch option. Install GNU patch"
> ? ? ? ? exit 1
> ?fi
> +PATCH_VERSION=$(patch --version 2>&1 | sed -e 's/^.* \([0-9\.]\)/\1/g' -e 's/[-\ ].*//g' -e '1q')
> +PATCH_MAJOR=$(echo $PATCH_VERSION | sed -e "s/\..*//g")
> +PATCH_MINOR=$(echo $PATCH_VERSION | sed -e "s/^$PATCH_MAJOR\.//g" -e "s/\..*//g" -e "s/[a-zA-Z].*//g")

Although I pretty much like regular expressions, I prefer that we do
without when needed, which is the case here:

    PATCH_VERSION="$(patch -v |awk '{ print $(3); exit; }')"
    PATCH_VERSION_MAJOR="$(cut -d . -f 1 <<<"${PATCH_VERSION}")"
    PATCH_VERSION_MINOR="$(cut -d . -f 2 <<<"${PATCH_VERSION}")"

(note: quote all your expansions, both variable expansion and shell
expansion).

> +if [ $PATCH_MAJOR -lt 2 ] || [ $PATCH_MAJOR -eq 2 -a $PATCH_MINOR -lt 7 ] ; then

Quote your variable expansion, and expand variables with curly braces;

    "${PATCH_MAJOR}"

Note: I know you got inspiration from the rest of the script, which was
a good idea, but the script has been inherited from way back then a long
time ago, when we did not have coding rules... No reason not to do better
nowadays! ;-)

> + ? ? ? echo
> + ? ? ? echo "You have GNU patch '$PATCH_VERSION' installed.? GNU patch >=2.7 is required"
> + ? ? ? 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

ACK.

Care to resend with the above changes, please?

> ? ? ?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
> Regards,
> Ryota Kinukawa

-- 
.-----------------.--------------------.------------------.--------------------.
|  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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] Suggestion to "support/scripts/apply-patches.sh: do not apply patches with renames"

[Arnout Vandecappelle]

 Hi Ryota,

 In addition to Yann's comments, I have two of my own:

On 16/05/2021 10:18, Ryota Kinukawa wrote:
[snip]
> ?# apply-patches.sh needs patch with --no-backup-if-mismatch support (GNU,
> busybox w/DESKTOP)
> ?if ! patch --no-backup-if-mismatch </dev/null 2>/dev/null; then
> ? ? ? ? echo "Your patch program does not support the --no-backup-if-mismatch
> option. Install GNU patch"
> ? ? ? ? exit 1
> ?fi

 This test is no longer needed, the version test covers it.

> +PATCH_VERSION=$(patch --version 2>&1 | sed -e 's/^.* \([0-9\.]\)/\1/g' -e
> 's/[-\ ].*//g' -e '1q')

 Non-GNU patch implementations may not accept --version or produce otherwise
different output. I think a better sed expression would be:

patch --version | sed -n '/GNU patch \(.*\)/s//\1/p'

and add a check that PATCH_VERSION is not empty.

With Yann's cut-based extraction of MAJOR and MINOR, this will work even if
there's other distro-specific stuff at the end of the version.

 Regards,
 Arnout

> +PATCH_MAJOR=$(echo $PATCH_VERSION | sed -e "s/\..*//g")
> +PATCH_MINOR=$(echo $PATCH_VERSION | sed -e "s/^$PATCH_MAJOR\.//g" -e
> "s/\..*//g" -e "s/[a-zA-Z].*//g")
> +if [ $PATCH_MAJOR -lt 2 ] || [ $PATCH_MAJOR -eq 2 -a $PATCH_MINOR -lt 7 ] ; then
> + ? ? ? echo
> + ? ? ? echo "You have GNU patch '$PATCH_VERSION' installed.? GNU patch >=2.7 is
> required"
> + ? ? ? 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
> 
> Regards,
> Ryota Kinukawa
> 
> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
> 

[Buildroot] [PATCH] support/dependencies, scripts: accept patches with renames

[pojiro.jp at gmail.com]

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>
---
 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')"
 # apply-patches.sh needs patch with --no-backup-if-mismatch support (GNU, busybox w/DESKTOP)
-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"
 	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
+	echo
+	echo "You have GNU patch '$PATCH_VERSION' installed.  GNU patch >=2.7 is required"
+	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] [PATCH] support/dependencies, scripts: accept patches with renames

[Yann E. MORIN]

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.  |
'------------------------------^-------^------------------^--------------------'

[Buildroot] [PATCH v2] support/dependencies, scripts: accept patches with renames

[pojiro.jp at gmail.com]

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>
---
 support/dependencies/dependencies.sh | 16 ++++++++++++----
 support/scripts/apply-patches.sh     |  5 -----
 2 files changed, 12 insertions(+), 9 deletions(-)

diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
index 1954f038be..91e7cd60f9 100755
--- a/support/dependencies/dependencies.sh
+++ b/support/dependencies/dependencies.sh
@@ -163,7 +163,7 @@ fi
 
 # Check that a few mandatory programs are installed
 missing_progs="no"
-for prog in patch perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
+for prog in perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
 	if ! which $prog > /dev/null ; then
 		echo "You must install '$prog' on your build machine";
 		missing_progs="yes"
@@ -183,11 +183,19 @@ if test "${missing_progs}" = "yes" ; then
 	exit 1
 fi
 
-# apply-patches.sh needs patch with --no-backup-if-mismatch support (GNU, busybox w/DESKTOP)
-if ! patch --no-backup-if-mismatch </dev/null 2>/dev/null; then
-	echo "Your patch program does not support the --no-backup-if-mismatch option. Install GNU patch"
+PATCH_VERSION="$(patch -v 2> /dev/null | sed -n 's/^GNU patch \(.*\)/\1/p')"
+if [ -z "${PATCH_VERSION}" ] ; then
+	echo
+	echo "You must install GNU patch"
 	exit 1
 fi
+PATCH_MAJOR="$(echo "${PATCH_VERSION}" | cut -d . -f 1)"
+PATCH_MINOR="$(echo "${PATCH_VERSION}" | cut -d . -f 2)"
+if [ "${PATCH_MAJOR}" -lt 2 ] || [ "${PATCH_MAJOR}" -eq 2 -a "${PATCH_MINOR}" -lt 7 ] ; then
+	echo
+	echo "You have GNU patch "${PATCH_VERSION}" installed. GNU patch >=2.7 is required"
+	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] [PATCH v2] support/dependencies, scripts: accept patches with renames

[Yann E. MORIN]

Ryota, All,

On 2021-05-19 17:34 +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! However, there must have been a misunderstanding
:I already applied your previous patch with the few tweaks I pointed
out. See the git tree:

    https://git.buildroot.org/buildroot/commit/?h=next&id=f46e13f05fa04a180a416876cf1dd4252dd841ad

Thanks again for following up on this! :-)

Regards,
Yann E. MORIN.

> ---
>  support/dependencies/dependencies.sh | 16 ++++++++++++----
>  support/scripts/apply-patches.sh     |  5 -----
>  2 files changed, 12 insertions(+), 9 deletions(-)
> 
> diff --git a/support/dependencies/dependencies.sh b/support/dependencies/dependencies.sh
> index 1954f038be..91e7cd60f9 100755
> --- a/support/dependencies/dependencies.sh
> +++ b/support/dependencies/dependencies.sh
> @@ -163,7 +163,7 @@ fi
>  
>  # Check that a few mandatory programs are installed
>  missing_progs="no"
> -for prog in patch perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
> +for prog in perl tar wget cpio unzip rsync bc ${DL_TOOLS} ; do
>  	if ! which $prog > /dev/null ; then
>  		echo "You must install '$prog' on your build machine";
>  		missing_progs="yes"
> @@ -183,11 +183,19 @@ if test "${missing_progs}" = "yes" ; then
>  	exit 1
>  fi
>  
> -# apply-patches.sh needs patch with --no-backup-if-mismatch support (GNU, busybox w/DESKTOP)
> -if ! patch --no-backup-if-mismatch </dev/null 2>/dev/null; then
> -	echo "Your patch program does not support the --no-backup-if-mismatch option. Install GNU patch"
> +PATCH_VERSION="$(patch -v 2> /dev/null | sed -n 's/^GNU patch \(.*\)/\1/p')"
> +if [ -z "${PATCH_VERSION}" ] ; then
> +	echo
> +	echo "You must install GNU patch"
>  	exit 1
>  fi
> +PATCH_MAJOR="$(echo "${PATCH_VERSION}" | cut -d . -f 1)"
> +PATCH_MINOR="$(echo "${PATCH_VERSION}" | cut -d . -f 2)"
> +if [ "${PATCH_MAJOR}" -lt 2 ] || [ "${PATCH_MAJOR}" -eq 2 -a "${PATCH_MINOR}" -lt 7 ] ; then
> +	echo
> +	echo "You have GNU patch "${PATCH_VERSION}" installed. GNU patch >=2.7 is required"
> +	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.  |
'------------------------------^-------^------------------^--------------------'