* [Buildroot] [PATCH 1/1] support/scripts/: remove -E flag from patch call @ 2021-12-14 12:55 Andrey Nechypurenko 2021-12-30 21:47 ` Yann E. MORIN 0 siblings, 1 reply; 5+ messages in thread From: Andrey Nechypurenko @ 2021-12-14 12:55 UTC (permalink / raw) To: buildroot; +Cc: Andrey Nechypurenko -E flag instructs patch to remove empty files. However, in some cases empty files are essential. If they are missing, build could be broken or other bad things can happen. Signed-off-by: Andrey Nechypurenko <andreynech@gmail.com> --- support/scripts/apply-patches.sh | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh index e5a2fdd09e..6da83f6826 100755 --- a/support/scripts/apply-patches.sh +++ b/support/scripts/apply-patches.sh @@ -114,7 +114,7 @@ function apply_patch { 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 + ${uncomp} "${path}/$patch" | patch -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent if [ $? != 0 ] ; then echo "Patch failed! Please fix ${patch}!" exit 1 -- 2.25.1 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH 1/1] support/scripts/: remove -E flag from patch call 2021-12-14 12:55 [Buildroot] [PATCH 1/1] support/scripts/: remove -E flag from patch call Andrey Nechypurenko @ 2021-12-30 21:47 ` Yann E. MORIN 2022-01-03 11:29 ` Andrey Nechypurenko 2022-01-03 17:11 ` Arnout Vandecappelle 0 siblings, 2 replies; 5+ messages in thread From: Yann E. MORIN @ 2021-12-30 21:47 UTC (permalink / raw) To: Andrey Nechypurenko; +Cc: buildroot Andrey, All, On 2021-12-14 13:55 +0100, Andrey Nechypurenko spake thusly: > -E flag instructs patch to remove empty files. However, in some cases > empty files are essential. If they are missing, build could be broken > or other bad things can happen. Do you have an example of a publicly accessible package that has this issue? > Signed-off-by: Andrey Nechypurenko <andreynech@gmail.com> I was going to apply this, but then I was wondering; what about patches that actually *want* to remove files? It turns out that we do have a few patches that remove files: $ git grep -E '\+\+\+ /dev/null' board/roseapplepi/patches/uboot/0001-compiler-.h-sync-include-linux-compiler-.h-with-Linu.patch:+++ /dev/null board/roseapplepi/patches/uboot/0001-compiler-.h-sync-include-linux-compiler-.h-with-Linu.patch:+++ /dev/null board/roseapplepi/patches/uboot/0001-compiler-.h-sync-include-linux-compiler-.h-with-Linu.patch:+++ /dev/null boot/grub2/0033-verifiers-Move-verifiers-API-to-kernel-image.patch:+++ /dev/null boot/grub2/0034-efi-Move-the-shim_lock-verifier-to-the-GRUB-core.patch:+++ /dev/null boot/lpc32xxcdl/0002-delete_redundant_files.patch:+++ /dev/null2012-01-01 16:39:47.918907000 +0100 boot/lpc32xxcdl/0002-delete_redundant_files.patch:+++ /dev/null2012-01-01 16:39:47.918907000 +0100 package/babeltrace2/0001-configure-simplify-warning-flags-detection.patch:+++ /dev/null package/babeltrace2/0001-configure-simplify-warning-flags-detection.patch:+++ /dev/null package/babeltrace2/0001-configure-simplify-warning-flags-detection.patch:+++ /dev/null package/babeltrace2/0001-configure-simplify-warning-flags-detection.patch:+++ /dev/null package/gcc/10.3.0/0001-Revert-re-PR-target-92095-internal-error-with-O1-mcp.patch:+++ /dev/null package/gcc/8.4.0/0002-Revert-re-PR-target-92095-internal-error-with-O1-mcp.patch:+++ /dev/null package/gcc/9.4.0/0003-Revert-re-PR-target-92095-internal-error-with-O1-mcp.patch:+++ /dev/null package/openpgm/0001-Rename-openpgm-5.2.pc.in.patch:+++ /dev/null package/openvmtools/0003-Rename-poll-h-into-vm_poll-h-to-fix-build-failure-on-musl.patch:+++ /dev/null package/screen/0005-rename-sched_h.patch:+++ /dev/null While a few of them could be rewriten so that the are actual renames (now that we require a patch version that supports renames), some really do want to remove files; that's notably the case with the uboot patch for the roseapplepi board, or the babeltrace2 patch, as well as the gcc patches... Furthermore, some users may have alaready relied on this behaviour, that empty files get removed, for their packages in theor br2-external trees... So we have an antagonist situation: some patches will want to rem ove files, and some patches will want to not remove files. We can't satisfy both, obviously. So, unfortunately, it is not possible to apply your patch. If your package really requires that the files be present and empty, you should then use a post-patch hook to touch those files. Unless someone comes up with a better idea... Regards, Yann E. MORIN. > --- > support/scripts/apply-patches.sh | 2 +- > 1 file changed, 1 insertion(+), 1 deletion(-) > > diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh > index e5a2fdd09e..6da83f6826 100755 > --- a/support/scripts/apply-patches.sh > +++ b/support/scripts/apply-patches.sh > @@ -114,7 +114,7 @@ function apply_patch { > 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 > + ${uncomp} "${path}/$patch" | patch -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent > if [ $? != 0 ] ; then > echo "Patch failed! Please fix ${patch}!" > exit 1 > -- > 2.25.1 > > _______________________________________________ > buildroot mailing list > buildroot@buildroot.org > https://lists.buildroot.org/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 mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH 1/1] support/scripts/: remove -E flag from patch call 2021-12-30 21:47 ` Yann E. MORIN @ 2022-01-03 11:29 ` Andrey Nechypurenko 2022-01-03 17:11 ` Arnout Vandecappelle 1 sibling, 0 replies; 5+ messages in thread From: Andrey Nechypurenko @ 2022-01-03 11:29 UTC (permalink / raw) To: Yann E. MORIN; +Cc: buildroot Hi Yann, >> -E flag instructs patch to remove empty files. However, in some cases >> empty files are essential. If they are missing, build could be broken >> or other bad things can happen. > > Do you have an example of a publicly accessible package that has this > issue? No I have not. In my case this is not a mainlined driver for the touch controller provided by the display manufacturer. > So we have an antagonist situation: some patches will want to rem ove > files, and some patches will want to not remove files. We can't satisfy > both, obviously. > > So, unfortunately, it is not possible to apply your patch. I understand your concern. So it sounds like a need for configurability here. > Unless someone comes up with a better idea... Not sure if it is a good idea, but maybe the problem could be solved using series files? Currently, there is already -p1 parameter which could be specified in the series files. Brief look at the relevant part of the apply-patches.sh reveals that it is ignored :-) but it could be changed. After that, the script can walk through patch directories where patches with +++ /dev/null are found, generate or update series files to use the -E flag together with -p1. This solution would not break existing packages and provide flexibility for the new packages where developers can decide whether to add -E (or maybe some other flags) to patch invocation using series files. What do you think about it? Best regards, Andrey. _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH 1/1] support/scripts/: remove -E flag from patch call 2021-12-30 21:47 ` Yann E. MORIN 2022-01-03 11:29 ` Andrey Nechypurenko @ 2022-01-03 17:11 ` Arnout Vandecappelle 2022-01-03 20:18 ` Yann E. MORIN 1 sibling, 1 reply; 5+ messages in thread From: Arnout Vandecappelle @ 2022-01-03 17:11 UTC (permalink / raw) To: Yann E. MORIN, Andrey Nechypurenko; +Cc: buildroot On 30/12/2021 22:47, Yann E. MORIN wrote: > Andrey, All, > > On 2021-12-14 13:55 +0100, Andrey Nechypurenko spake thusly: >> -E flag instructs patch to remove empty files. However, in some cases >> empty files are essential. If they are missing, build could be broken >> or other bad things can happen. > > Do you have an example of a publicly accessible package that has this > issue? > >> Signed-off-by: Andrey Nechypurenko <andreynech@gmail.com> > > I was going to apply this, but then I was wondering; what about patches > that actually *want* to remove files? > > It turns out that we do have a few patches that remove files: > > $ git grep -E '\+\+\+ /dev/null' > board/roseapplepi/patches/uboot/0001-compiler-.h-sync-include-linux-compiler-.h-with-Linu.patch:+++ /dev/null > board/roseapplepi/patches/uboot/0001-compiler-.h-sync-include-linux-compiler-.h-with-Linu.patch:+++ /dev/null > board/roseapplepi/patches/uboot/0001-compiler-.h-sync-include-linux-compiler-.h-with-Linu.patch:+++ /dev/null > boot/grub2/0033-verifiers-Move-verifiers-API-to-kernel-image.patch:+++ /dev/null > boot/grub2/0034-efi-Move-the-shim_lock-verifier-to-the-GRUB-core.patch:+++ /dev/null > boot/lpc32xxcdl/0002-delete_redundant_files.patch:+++ /dev/null2012-01-01 16:39:47.918907000 +0100 > boot/lpc32xxcdl/0002-delete_redundant_files.patch:+++ /dev/null2012-01-01 16:39:47.918907000 +0100 > package/babeltrace2/0001-configure-simplify-warning-flags-detection.patch:+++ /dev/null > package/babeltrace2/0001-configure-simplify-warning-flags-detection.patch:+++ /dev/null > package/babeltrace2/0001-configure-simplify-warning-flags-detection.patch:+++ /dev/null > package/babeltrace2/0001-configure-simplify-warning-flags-detection.patch:+++ /dev/null Have you actually tested this? With modern patch, the /dev/null indicates exactly what we want: that the file must be removed. So even without -E, the file gets removed. What -E does is remove a file in the following case: --- a/foo +++ a/foo @@ -1,1 +0,0 @@ - foo git grep -B1 '+0,0 @@' only yields /dev/null cases, so I think it's OK. So I think the only real concern is: will a patch version that supports renames always behave like this? I *think* the answer is yes, but it's hard to be sure of course. Regards, Arnout > package/gcc/10.3.0/0001-Revert-re-PR-target-92095-internal-error-with-O1-mcp.patch:+++ /dev/null > package/gcc/8.4.0/0002-Revert-re-PR-target-92095-internal-error-with-O1-mcp.patch:+++ /dev/null > package/gcc/9.4.0/0003-Revert-re-PR-target-92095-internal-error-with-O1-mcp.patch:+++ /dev/null > package/openpgm/0001-Rename-openpgm-5.2.pc.in.patch:+++ /dev/null > package/openvmtools/0003-Rename-poll-h-into-vm_poll-h-to-fix-build-failure-on-musl.patch:+++ /dev/null > package/screen/0005-rename-sched_h.patch:+++ /dev/null > > While a few of them could be rewriten so that the are actual renames > (now that we require a patch version that supports renames), some really > do want to remove files; that's notably the case with the uboot patch > for the roseapplepi board, or the babeltrace2 patch, as well as the gcc > patches... > > Furthermore, some users may have alaready relied on this behaviour, that > empty files get removed, for their packages in theor br2-external > trees... > > So we have an antagonist situation: some patches will want to rem ove > files, and some patches will want to not remove files. We can't satisfy > both, obviously. > > So, unfortunately, it is not possible to apply your patch. > > If your package really requires that the files be present and empty, you > should then use a post-patch hook to touch those files. > > Unless someone comes up with a better idea... > > Regards, > Yann E. MORIN. > >> --- >> support/scripts/apply-patches.sh | 2 +- >> 1 file changed, 1 insertion(+), 1 deletion(-) >> >> diff --git a/support/scripts/apply-patches.sh b/support/scripts/apply-patches.sh >> index e5a2fdd09e..6da83f6826 100755 >> --- a/support/scripts/apply-patches.sh >> +++ b/support/scripts/apply-patches.sh >> @@ -114,7 +114,7 @@ function apply_patch { >> 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 >> + ${uncomp} "${path}/$patch" | patch -g0 -p1 --no-backup-if-mismatch -d "${builddir}" -t -N $silent >> if [ $? != 0 ] ; then >> echo "Patch failed! Please fix ${patch}!" >> exit 1 >> -- >> 2.25.1 >> >> _______________________________________________ >> buildroot mailing list >> buildroot@buildroot.org >> https://lists.buildroot.org/mailman/listinfo/buildroot > _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 5+ messages in thread
* Re: [Buildroot] [PATCH 1/1] support/scripts/: remove -E flag from patch call 2022-01-03 17:11 ` Arnout Vandecappelle @ 2022-01-03 20:18 ` Yann E. MORIN 0 siblings, 0 replies; 5+ messages in thread From: Yann E. MORIN @ 2022-01-03 20:18 UTC (permalink / raw) To: Arnout Vandecappelle; +Cc: Andrey Nechypurenko, buildroot Arnout, All, On 2022-01-03 18:11 +0100, Arnout Vandecappelle spake thusly: > On 30/12/2021 22:47, Yann E. MORIN wrote: > >On 2021-12-14 13:55 +0100, Andrey Nechypurenko spake thusly: > >>-E flag instructs patch to remove empty files. However, in some cases > >>empty files are essential. If they are missing, build could be broken > >>or other bad things can happen. [--SNIP--] > >I was going to apply this, but then I was wondering; what about patches > >that actually *want* to remove files? > >It turns out that we do have a few patches that remove files: > Have you actually tested this? With modern patch, the /dev/null indicates > exactly what we want: that the file must be removed. So even without -E, the > file gets removed. > > What -E does is remove a file in the following case: > > --- a/foo > +++ a/foo > @@ -1,1 +0,0 @@ > - foo Ah, yeah, I was a bit unsure... Now I tried, adn indeed it behaves as expected. > git grep -B1 '+0,0 @@' only yields /dev/null cases, so I think it's OK. > So I think the only real concern is: will a patch version that supports > renames always behave like this? I *think* the answer is yes, but it's hard > to be sure of course. OK, I'm sold on the idea, and I believe this is sane enough. Applied to master, thanks. 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 mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 5+ messages in thread
end of thread, other threads:[~2022-01-03 20:18 UTC | newest] Thread overview: 5+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2021-12-14 12:55 [Buildroot] [PATCH 1/1] support/scripts/: remove -E flag from patch call Andrey Nechypurenko 2021-12-30 21:47 ` Yann E. MORIN 2022-01-03 11:29 ` Andrey Nechypurenko 2022-01-03 17:11 ` Arnout Vandecappelle 2022-01-03 20:18 ` Yann E. MORIN
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.