* [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking @ 2024-10-30 0:20 Scott Fan 2024-10-31 18:18 ` Thomas Petazzoni via buildroot 2024-11-28 3:50 ` Scott Fan 0 siblings, 2 replies; 13+ messages in thread From: Scott Fan @ 2024-10-30 0:20 UTC (permalink / raw) To: buildroot; +Cc: Scott Fan The previous build setup would check for pidfd_open using AC_CHECK_FUNC and would be incorrectly reported as true. Also, if pidfd_open() and __NR_pidfd_open were not present, pidwait would silently not be built. So, changes: compile a small programin using pidfd_open to test it properly conditionally try to find NR_pidfd_open if the function fails complain if neither are present have --disable-pidwait configure option so you are explicit in not wanting and knowing you wont get pidwait Backport patch from upstream: https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 Signed-off-by: Scott Fan <fancp2007@gmail.com> --- ...03-build-sys-Fix-pidfd_open-checking.patch | 99 +++++++++++++++++++ package/procps-ng/procps-ng.mk | 1 + 2 files changed, 100 insertions(+) create mode 100644 package/procps-ng/0003-build-sys-Fix-pidfd_open-checking.patch diff --git a/package/procps-ng/0003-build-sys-Fix-pidfd_open-checking.patch b/package/procps-ng/0003-build-sys-Fix-pidfd_open-checking.patch new file mode 100644 index 0000000000..bf89a07a20 --- /dev/null +++ b/package/procps-ng/0003-build-sys-Fix-pidfd_open-checking.patch @@ -0,0 +1,99 @@ +From ae7b0eb069f4add363e8c4d01833a06c3f5ee842 Mon Sep 17 00:00:00 2001 +From: Craig Small <csmall@dropbear.xyz> +Date: Mon, 30 Sep 2024 17:26:01 +1000 +Subject: [PATCH] build-sys: Fix pidfd_open checking + +The previous build setup would check for pidfd_open using +AC_CHECK_FUNC and would be incorrectly reported as true. + +Also, if pidfd_open() and __NR_pidfd_open were not present, +pidwait would silently not be built. + +So, changes: + compile a small programin using pidfd_open to test it properly + conditionally try to find NR_pidfd_open if the function fails + complain if neither are present + have --disable-pidwait configure option so you are explicit in + not wanting and knowing you wont get pidwait + +References: + #352 + commit d9c3e3676d86094abaa239b3218f57bf49d70b4f + commit 17f94796a9b3c4f1ff28829107a82107dcb362b4 + +Signed-off-by: Craig Small <csmall@dropbear.xyz> + +Upstream: https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 + +Signed-off-by: Scott Fan <fancp2007@gmail.com> +[Scott: backported to version 4.0.4] +--- + configure.ac | 45 ++++++++++++++++++++++++++++++--------------- + 1 file changed, 30 insertions(+), 15 deletions(-) + +diff --git a/configure.ac b/configure.ac +index fec27e3f..0719fcd1 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -170,21 +170,6 @@ AC_TRY_COMPILE([#include <errno.h>], + AC_MSG_RESULT(yes), + AC_MSG_RESULT(no)) + +-AC_CHECK_FUNC([pidfd_open], [enable_pidwait=yes], [ +- AC_MSG_CHECKING([for __NR_pidfd_open]) +- AC_COMPILE_IFELSE([AC_LANG_SOURCE([ +-#include <sys/syscall.h> +-#ifndef __NR_pidfd_open +-#error __NR_pidfd_open not defined +-#endif +- ])], [enable_pidwait=yes], [enable_pidwait=no]) +- AC_MSG_RESULT([$enable_pidwait]) +-]) +-if test "$enable_pidwait" = yes; then +- AC_DEFINE([ENABLE_PIDWAIT], [1], [Enable pidwait]) +-fi +-AM_CONDITIONAL([BUILD_PIDWAIT], [test x$enable_pidwait = xyes]) +- + dnl watch8bit must be before the AC_ARG_WITH set as it sets up ncurses + AC_SUBST([WITH_WATCH8BIT]) + AC_ARG_ENABLE([watch8bit], +@@ -321,6 +306,36 @@ AC_ARG_ENABLE([pidof], + ) + AM_CONDITIONAL(BUILD_PIDOF, test "x$enable_pidof" = xyes) + ++# If pidwait is enabled, we need either pidfd_open() or __NR_pidfd_open need to be defined ++# Cannot use AC_CHECK_FUNC as it (incorrectly) passes with pidfd_open missing ++AC_ARG_ENABLE([pidwait], ++ AS_HELP_STRING([--disable-pidwait], [do not build pidwait]), ++ [], [ ++ enable_pidwait=yes ++ AC_DEFINE(ENABLE_PIDWAIT, 1, [enable pidwait]) ++ ] ++) ++AM_CONDITIONAL(BUILD_PIDWAIT, test "x$enable_pidwait" = xyes) ++AC_MSG_CHECKING([for pidfd_open()]) ++AC_LINK_IFELSE([AC_LANG_PROGRAM([], [[ [pidfd_open(1,1)]]])], ++ have_pidfd_open=yes; AC_MSG_RESULT([yes]) , ++ have_pidfd_open=no; AC_MSG_RESULT([no]) ++ ) ++ ++AS_IF([[test "x$enable_pidwait" = xyes -a "x$have_pidfd_open" = xno]], ++ AC_MSG_CHECKING([for __NR_pidfd_open]) ++ AC_COMPILE_IFELSE([AC_LANG_SOURCE([ ++#include <sys/syscall.h> ++#ifndef __NR_pidfd_open ++#error __NR_pidfd_open not defined ++#endif ++ ])], ++ AC_MSG_RESULT([yes]), ++ AC_MSG_RESULT([no]) ++ AC_MSG_ERROR([Neither pidfd_open or __NR_pidfd_open found. Disable pidwait with configure option --disable-pidwait]) ++ ) ++ ,[]) ++ + AC_ARG_ENABLE([kill], + AS_HELP_STRING([--disable-kill], [do not build kill]), + [], [enable_kill=yes] +-- +2.43.0 + diff --git a/package/procps-ng/procps-ng.mk b/package/procps-ng/procps-ng.mk index c5185c2c8a..666c635ccc 100644 --- a/package/procps-ng/procps-ng.mk +++ b/package/procps-ng/procps-ng.mk @@ -16,6 +16,7 @@ PROCPS_NG_CONF_OPTS = LIBS=$(TARGET_NLS_LIBS) # Applying 0001-build-sys-Add-systemd-elogind-to-w.patch touches Makefile.am # Applying 0002-fix-ncurses-h-include.patch touches configure.ac +# Applying 0003-build-sys-Fix-pidfd_open-checking.patch touches configure.ac PROCPS_NG_AUTORECONF = YES ifeq ($(BR2_PACKAGE_SYSTEMD),y) -- 2.43.0 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-10-30 0:20 [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking Scott Fan @ 2024-10-31 18:18 ` Thomas Petazzoni via buildroot 2024-11-01 1:31 ` Scott Fan 2024-11-28 3:50 ` Scott Fan 1 sibling, 1 reply; 13+ messages in thread From: Thomas Petazzoni via buildroot @ 2024-10-31 18:18 UTC (permalink / raw) To: Scott Fan; +Cc: buildroot Hello Scott, On Wed, 30 Oct 2024 08:20:18 +0800 Scott Fan <fancp2007@gmail.com> wrote: > The previous build setup would check for pidfd_open using > AC_CHECK_FUNC and would be incorrectly reported as true. > > Also, if pidfd_open() and __NR_pidfd_open were not present, > pidwait would silently not be built. > > So, changes: > compile a small programin using pidfd_open to test it properly > conditionally try to find NR_pidfd_open if the function fails > complain if neither are present > have --disable-pidwait configure option so you are explicit in > not wanting and knowing you wont get pidwait > > Backport patch from upstream: > https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 > > Signed-off-by: Scott Fan <fancp2007@gmail.com> So you're no longer setting --disable-pidwait... but that will cause a failure: > ++AS_IF([[test "x$enable_pidwait" = xyes -a "x$have_pidfd_open" = xno]], > ++ AC_MSG_CHECKING([for __NR_pidfd_open]) > ++ AC_COMPILE_IFELSE([AC_LANG_SOURCE([ > ++#include <sys/syscall.h> > ++#ifndef __NR_pidfd_open > ++#error __NR_pidfd_open not defined > ++#endif > ++ ])], > ++ AC_MSG_RESULT([yes]), > ++ AC_MSG_RESULT([no]) > ++ AC_MSG_ERROR([Neither pidfd_open or __NR_pidfd_open found. Disable pidwait with configure option --disable-pidwait]) ... here. So I'm not sure my previous review was clear enough: what is wrong is the procps-ng patch itself, it shouldn't AC_MSG_ERROR(), but instead it should automatically disable pidwait, as if the user had passed --disable-pidwait. Or am I missing something here? Thanks! Thomas -- Thomas Petazzoni, co-owner and CEO, Bootlin Embedded Linux and Kernel engineering and training https://bootlin.com _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-10-31 18:18 ` Thomas Petazzoni via buildroot @ 2024-11-01 1:31 ` Scott Fan 2024-11-04 13:23 ` Scott Fan 2024-11-24 16:29 ` Thomas Petazzoni via buildroot 0 siblings, 2 replies; 13+ messages in thread From: Scott Fan @ 2024-11-01 1:31 UTC (permalink / raw) To: Thomas Petazzoni; +Cc: buildroot On Fri, Nov 1, 2024 at 2:18 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > So you're no longer setting --disable-pidwait... but that will cause a > failure: > > > ++AS_IF([[test "x$enable_pidwait" = xyes -a "x$have_pidfd_open" = xno]], > > ++ AC_MSG_CHECKING([for __NR_pidfd_open]) > > ++ AC_COMPILE_IFELSE([AC_LANG_SOURCE([ > > ++#include <sys/syscall.h> > > ++#ifndef __NR_pidfd_open > > ++#error __NR_pidfd_open not defined > > ++#endif > > ++ ])], > > ++ AC_MSG_RESULT([yes]), > > ++ AC_MSG_RESULT([no]) > > ++ AC_MSG_ERROR([Neither pidfd_open or __NR_pidfd_open found. Disable pidwait with configure option --disable-pidwait]) > > > ... here. > > So I'm not sure my previous review was clear enough: what is wrong is > the procps-ng patch itself, it shouldn't AC_MSG_ERROR(), but instead it > should automatically disable pidwait, as if the user had passed > --disable-pidwait. > I understand what you mean, but I may not have expressed my thoughts clearly. Before this patch was applied, I got a compile error (not configure error) when the package was built with linux-headers-4.19.322, Then I tried to figure out what was going wrong and I eventually found this patch from upstream. After this patch was applied, it threw a configure error (not compile error), which is what I expected. As the patch said: The previous build setup would check for pidfd_open using AC_CHECK_FUNC and would be incorrectly reported as true. Therefore, I think the key to this patch is that it successfully fixes the pidfd_open checking issue. If pidfd_open() and __NR_pidfd_open were not present, it will report an AC_MSG_ERROR message, and gives us the --disable-pidwait configure option, so you are explicit in not wanting and knowing you wont get pidwait. Whereas you mean, instead of AC_MSG_ERROR(), it should automatically disable pidwait, as if the user had passed --disable-pidwait. I'm not sure if this is acceptable to the upstream maintainers, who want you to be clear about the purpose of the option you passed. Even if it is approved , it would require a new patch to improve this behavior. To solve my compilation problem (with linux-headers-4.19.322), in the current case, I had to pass the --disable-pidwait option. If at some point in the future, when the patch you expect is successfully applied, I will remove it. Scott Fan _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-11-01 1:31 ` Scott Fan @ 2024-11-04 13:23 ` Scott Fan 2024-11-24 16:29 ` Thomas Petazzoni via buildroot 1 sibling, 0 replies; 13+ messages in thread From: Scott Fan @ 2024-11-04 13:23 UTC (permalink / raw) To: Thomas Petazzoni; +Cc: buildroot Hello Thomas, I have tried to submit a new patch to automatically disable pidwait when pidfd_open check fails. And requested the upstream project's maintainer to review it. But he said: Silently not making a program is not ideal. You could run make on a system and not get pidwait and then a few days later run the same command and now pidwait appears. It really should be explicit. See: https://gitlab.com/procps-ng/procps/-/merge_requests/236 I'm sorry to get such an unsatisfactory answer. Scott Fan On Fri, Nov 1, 2024 at 9:31 AM Scott Fan <fancp2007@gmail.com> wrote: > > On Fri, Nov 1, 2024 at 2:18 AM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > > > So you're no longer setting --disable-pidwait... but that will cause a > > failure: > > > > > ++AS_IF([[test "x$enable_pidwait" = xyes -a "x$have_pidfd_open" = xno]], > > > ++ AC_MSG_CHECKING([for __NR_pidfd_open]) > > > ++ AC_COMPILE_IFELSE([AC_LANG_SOURCE([ > > > ++#include <sys/syscall.h> > > > ++#ifndef __NR_pidfd_open > > > ++#error __NR_pidfd_open not defined > > > ++#endif > > > ++ ])], > > > ++ AC_MSG_RESULT([yes]), > > > ++ AC_MSG_RESULT([no]) > > > ++ AC_MSG_ERROR([Neither pidfd_open or __NR_pidfd_open found. Disable pidwait with configure option --disable-pidwait]) > > > > > > ... here. > > > > So I'm not sure my previous review was clear enough: what is wrong is > > the procps-ng patch itself, it shouldn't AC_MSG_ERROR(), but instead it > > should automatically disable pidwait, as if the user had passed > > --disable-pidwait. > > > > I understand what you mean, but I may not have expressed my thoughts clearly. > > Before this patch was applied, I got a compile error (not configure > error) when the package was built with linux-headers-4.19.322, > Then I tried to figure out what was going wrong and I eventually found > this patch from upstream. > > After this patch was applied, it threw a configure error (not compile > error), which is what I expected. > > As the patch said: > The previous build setup would check for pidfd_open using > AC_CHECK_FUNC and would be incorrectly reported as true. > > Therefore, I think the key to this patch is that it successfully fixes > the pidfd_open checking issue. > > If pidfd_open() and __NR_pidfd_open were not present, it will report > an AC_MSG_ERROR message, > and gives us the --disable-pidwait configure option, so you are > explicit in not wanting and knowing you wont get pidwait. > > Whereas you mean, instead of AC_MSG_ERROR(), it should automatically > disable pidwait, as if the user had passed --disable-pidwait. > I'm not sure if this is acceptable to the upstream maintainers, who > want you to be clear about the purpose of the option you passed. > Even if it is approved , it would require a new patch to improve this behavior. > > > To solve my compilation problem (with linux-headers-4.19.322), in the > current case, I had to pass the --disable-pidwait option. > If at some point in the future, when the patch you expect is > successfully applied, I will remove it. > > Scott Fan _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-11-01 1:31 ` Scott Fan 2024-11-04 13:23 ` Scott Fan @ 2024-11-24 16:29 ` Thomas Petazzoni via buildroot 2024-11-26 1:50 ` Scott Fan 1 sibling, 1 reply; 13+ messages in thread From: Thomas Petazzoni via buildroot @ 2024-11-24 16:29 UTC (permalink / raw) To: Scott Fan; +Cc: buildroot Hello Scott, Sorry for the slow feedback. Not sure what the status of this is and whether my answer is still useful/relevant. On Fri, 1 Nov 2024 09:31:08 +0800 Scott Fan <fancp2007@gmail.com> wrote: > > So I'm not sure my previous review was clear enough: what is wrong is > > the procps-ng patch itself, it shouldn't AC_MSG_ERROR(), but instead it > > should automatically disable pidwait, as if the user had passed > > --disable-pidwait. > > I understand what you mean, but I may not have expressed my thoughts clearly. > > Before this patch was applied, I got a compile error (not configure > error) when the package was built with linux-headers-4.19.322, > Then I tried to figure out what was going wrong and I eventually found > this patch from upstream. > > After this patch was applied, it threw a configure error (not compile > error), which is what I expected. > > As the patch said: > The previous build setup would check for pidfd_open using > AC_CHECK_FUNC and would be incorrectly reported as true. > > Therefore, I think the key to this patch is that it successfully fixes > the pidfd_open checking issue. > > If pidfd_open() and __NR_pidfd_open were not present, it will report > an AC_MSG_ERROR message, > and gives us the --disable-pidwait configure option, so you are > explicit in not wanting and knowing you wont get pidwait. > > Whereas you mean, instead of AC_MSG_ERROR(), it should automatically > disable pidwait, as if the user had passed --disable-pidwait. > I'm not sure if this is acceptable to the upstream maintainers, who > want you to be clear about the purpose of the option you passed. > Even if it is approved , it would require a new patch to improve this behavior. > > > To solve my compilation problem (with linux-headers-4.19.322), in the > current case, I had to pass the --disable-pidwait option. > If at some point in the future, when the patch you expect is > successfully applied, I will remove it. What you can do then is: ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_19),) PROCPS_NG_CONF_OPTS += disable-pidwait endif This will ensure to disable pidwait support when headers are < 4.19. Best regards, Thomas -- Thomas Petazzoni, co-owner and CEO, Bootlin Embedded Linux and Kernel engineering and training https://bootlin.com _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-11-24 16:29 ` Thomas Petazzoni via buildroot @ 2024-11-26 1:50 ` Scott Fan 2024-11-26 13:57 ` Thomas Petazzoni via buildroot 0 siblings, 1 reply; 13+ messages in thread From: Scott Fan @ 2024-11-26 1:50 UTC (permalink / raw) To: Thomas Petazzoni; +Cc: buildroot Hello Thomas, I think it is difficult to distinguish which kernel version to choose as the minimum version. But I found the link: https://man7.org/linux/man-pages/man2/pidfd_open.2.html I'm not sure if it's correct, if it is then it should be at least 5.3. On Mon, Nov 25, 2024 at 12:29 AM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > To solve my compilation problem (with linux-headers-4.19.322), in the > > current case, I had to pass the --disable-pidwait option. > > If at some point in the future, when the patch you expect is > > successfully applied, I will remove it. > > What you can do then is: > > ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_4_19),) > PROCPS_NG_CONF_OPTS += disable-pidwait > endif > > This will ensure to disable pidwait support when headers are < 4.19. Maybe it should be as follows: ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_3),y) PROCPS_NG_CONF_OPTS += --disable-pidwait endif Scott Fan _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-11-26 1:50 ` Scott Fan @ 2024-11-26 13:57 ` Thomas Petazzoni via buildroot 2024-11-27 0:35 ` Scott Fan 0 siblings, 1 reply; 13+ messages in thread From: Thomas Petazzoni via buildroot @ 2024-11-26 13:57 UTC (permalink / raw) To: Scott Fan; +Cc: buildroot On Tue, 26 Nov 2024 09:50:12 +0800 Scott Fan <fancp2007@gmail.com> wrote: > Maybe it should be as follows: > > ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_3),y) > PROCPS_NG_CONF_OPTS += --disable-pidwait > endif If you write it this way, it will disable pidwait on all configurations with headers >= 5.3. Don't you want to do exactly the opposite? Best regards, Thomas -- Thomas Petazzoni, co-owner and CEO, Bootlin Embedded Linux and Kernel engineering and training https://bootlin.com _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-11-26 13:57 ` Thomas Petazzoni via buildroot @ 2024-11-27 0:35 ` Scott Fan 2024-11-27 1:01 ` Scott Fan 2024-11-27 7:44 ` Thomas Petazzoni via buildroot 0 siblings, 2 replies; 13+ messages in thread From: Scott Fan @ 2024-11-27 0:35 UTC (permalink / raw) To: Thomas Petazzoni; +Cc: buildroot On Tue, Nov 26, 2024 at 9:57 PM Thomas Petazzoni <thomas.petazzoni@bootlin.com> wrote: > > On Tue, 26 Nov 2024 09:50:12 +0800 > Scott Fan <fancp2007@gmail.com> wrote: > > > Maybe it should be as follows: > > > > ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_3),y) > > PROCPS_NG_CONF_OPTS += --disable-pidwait > > endif > > If you write it this way, it will disable pidwait on all configurations > with headers >= 5.3. Don't you want to do exactly the opposite? > Sorry, I made a mistake, it should be: ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_3),) PROCPS_NG_CONF_OPTS += --disable-pidwait endif But another problem is that the --disable-pidwait option does not exist in the v4.0.4 version unless the following patch is applied. https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 Scott Fan _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-11-27 0:35 ` Scott Fan @ 2024-11-27 1:01 ` Scott Fan 2024-11-27 7:44 ` Thomas Petazzoni via buildroot 1 sibling, 0 replies; 13+ messages in thread From: Scott Fan @ 2024-11-27 1:01 UTC (permalink / raw) To: Thomas Petazzoni; +Cc: buildroot Hello Thomas, This issue does not need to be addressed immediately because another merge request is being discussed upstream. https://gitlab.com/procps-ng/procps/-/merge_requests/240 That fix will likely be approved by the project maintainer, let's wait a few days and see what happens. Scott Fan On Wed, Nov 27, 2024 at 8:35 AM Scott Fan <fancp2007@gmail.com> wrote: > > On Tue, Nov 26, 2024 at 9:57 PM Thomas Petazzoni > <thomas.petazzoni@bootlin.com> wrote: > > > > On Tue, 26 Nov 2024 09:50:12 +0800 > > Scott Fan <fancp2007@gmail.com> wrote: > > > > > Maybe it should be as follows: > > > > > > ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_3),y) > > > PROCPS_NG_CONF_OPTS += --disable-pidwait > > > endif > > > > If you write it this way, it will disable pidwait on all configurations > > with headers >= 5.3. Don't you want to do exactly the opposite? > > > > Sorry, I made a mistake, it should be: > > ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_3),) > PROCPS_NG_CONF_OPTS += --disable-pidwait > endif > > But another problem is that the --disable-pidwait option does not > exist in the v4.0.4 version unless the following patch is applied. > https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 > > Scott Fan _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-11-27 0:35 ` Scott Fan 2024-11-27 1:01 ` Scott Fan @ 2024-11-27 7:44 ` Thomas Petazzoni via buildroot 1 sibling, 0 replies; 13+ messages in thread From: Thomas Petazzoni via buildroot @ 2024-11-27 7:44 UTC (permalink / raw) To: Scott Fan; +Cc: buildroot On Wed, 27 Nov 2024 08:35:42 +0800 Scott Fan <fancp2007@gmail.com> wrote: > Sorry, I made a mistake, it should be: > > ifeq ($(BR2_TOOLCHAIN_HEADERS_AT_LEAST_5_3),) > PROCPS_NG_CONF_OPTS += --disable-pidwait > endif Correct. > But another problem is that the --disable-pidwait option does not > exist in the v4.0.4 version unless the following patch is applied. > https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 Well, we can simply backport this patch for the time being, until there's a newer release that includes this commit. Best regards, Thomas -- Thomas Petazzoni, co-owner and CEO, Bootlin Embedded Linux and Kernel engineering and training https://bootlin.com _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-10-30 0:20 [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking Scott Fan 2024-10-31 18:18 ` Thomas Petazzoni via buildroot @ 2024-11-28 3:50 ` Scott Fan 2024-12-05 16:57 ` Peter Korsgaard 1 sibling, 1 reply; 13+ messages in thread From: Scott Fan @ 2024-11-28 3:50 UTC (permalink / raw) To: buildroot; +Cc: thomas.petazzoni, Scott Fan The previous build setup would check for pidfd_open using AC_CHECK_FUNC and would be incorrectly reported as true. Backport patch from upstream: [1] https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 [2] https://gitlab.com/procps-ng/procps/-/commit/587efb47df7ddbfda4e6abdd1e7792a2531a238f [3] https://gitlab.com/procps-ng/procps/-/commit/5acbb5dc1587d688de646d739a97251eb893bbb0 Signed-off-by: Scott Fan <fancp2007@gmail.com> --- ...03-build-sys-Fix-pidfd_open-checking.patch | 99 +++++++++++++++++++ ...ld-sys-Fix-define-of-HAVE_PIDFD_OPEN.patch | 37 +++++++ ...lude-sys-syscall.h-if-making-pidwait.patch | 41 ++++++++ package/procps-ng/procps-ng.mk | 2 + 4 files changed, 179 insertions(+) create mode 100644 package/procps-ng/0003-build-sys-Fix-pidfd_open-checking.patch create mode 100644 package/procps-ng/0004-build-sys-Fix-define-of-HAVE_PIDFD_OPEN.patch create mode 100644 package/procps-ng/0005-pgrep-Include-sys-syscall.h-if-making-pidwait.patch diff --git a/package/procps-ng/0003-build-sys-Fix-pidfd_open-checking.patch b/package/procps-ng/0003-build-sys-Fix-pidfd_open-checking.patch new file mode 100644 index 0000000000..fc7e092347 --- /dev/null +++ b/package/procps-ng/0003-build-sys-Fix-pidfd_open-checking.patch @@ -0,0 +1,99 @@ +From f37d178d5c25c547835d054fbb1eda32c25034b3 Mon Sep 17 00:00:00 2001 +From: Craig Small <csmall@dropbear.xyz> +Date: Mon, 30 Sep 2024 17:26:01 +1000 +Subject: [PATCH] build-sys: Fix pidfd_open checking + +The previous build setup would check for pidfd_open using +AC_CHECK_FUNC and would be incorrectly reported as true. + +Also, if pidfd_open() and __NR_pidfd_open were not present, +pidwait would silently not be built. + +So, changes: + compile a small programin using pidfd_open to test it properly + conditionally try to find NR_pidfd_open if the function fails + complain if neither are present + have --disable-pidwait configure option so you are explicit in + not wanting and knowing you wont get pidwait + +References: + #352 + commit d9c3e3676d86094abaa239b3218f57bf49d70b4f + commit 17f94796a9b3c4f1ff28829107a82107dcb362b4 + +Signed-off-by: Craig Small <csmall@dropbear.xyz> + +Upstream: https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 + +Signed-off-by: Scott Fan <fancp2007@gmail.com> +[Scott: backported to version 4.0.4] +--- + configure.ac | 45 ++++++++++++++++++++++++++++++--------------- + 1 file changed, 30 insertions(+), 15 deletions(-) + +diff --git a/configure.ac b/configure.ac +index fec27e3f..0719fcd1 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -170,21 +170,6 @@ AC_TRY_COMPILE([#include <errno.h>], + AC_MSG_RESULT(yes), + AC_MSG_RESULT(no)) + +-AC_CHECK_FUNC([pidfd_open], [enable_pidwait=yes], [ +- AC_MSG_CHECKING([for __NR_pidfd_open]) +- AC_COMPILE_IFELSE([AC_LANG_SOURCE([ +-#include <sys/syscall.h> +-#ifndef __NR_pidfd_open +-#error __NR_pidfd_open not defined +-#endif +- ])], [enable_pidwait=yes], [enable_pidwait=no]) +- AC_MSG_RESULT([$enable_pidwait]) +-]) +-if test "$enable_pidwait" = yes; then +- AC_DEFINE([ENABLE_PIDWAIT], [1], [Enable pidwait]) +-fi +-AM_CONDITIONAL([BUILD_PIDWAIT], [test x$enable_pidwait = xyes]) +- + dnl watch8bit must be before the AC_ARG_WITH set as it sets up ncurses + AC_SUBST([WITH_WATCH8BIT]) + AC_ARG_ENABLE([watch8bit], +@@ -321,6 +306,36 @@ AC_ARG_ENABLE([pidof], + ) + AM_CONDITIONAL(BUILD_PIDOF, test "x$enable_pidof" = xyes) + ++# If pidwait is enabled, we need either pidfd_open() or __NR_pidfd_open need to be defined ++# Cannot use AC_CHECK_FUNC as it (incorrectly) passes with pidfd_open missing ++AC_ARG_ENABLE([pidwait], ++ AS_HELP_STRING([--disable-pidwait], [do not build pidwait]), ++ [], [ ++ enable_pidwait=yes ++ AC_DEFINE(ENABLE_PIDWAIT, 1, [enable pidwait]) ++ ] ++) ++AM_CONDITIONAL(BUILD_PIDWAIT, test "x$enable_pidwait" = xyes) ++AC_MSG_CHECKING([for pidfd_open()]) ++AC_LINK_IFELSE([AC_LANG_PROGRAM([], [[ [pidfd_open(1,1)]]])], ++ have_pidfd_open=yes; AC_MSG_RESULT([yes]) , ++ have_pidfd_open=no; AC_MSG_RESULT([no]) ++ ) ++ ++AS_IF([[test "x$enable_pidwait" = xyes -a "x$have_pidfd_open" = xno]], ++ AC_MSG_CHECKING([for __NR_pidfd_open]) ++ AC_COMPILE_IFELSE([AC_LANG_SOURCE([ ++#include <sys/syscall.h> ++#ifndef __NR_pidfd_open ++#error __NR_pidfd_open not defined ++#endif ++ ])], ++ AC_MSG_RESULT([yes]), ++ AC_MSG_RESULT([no]) ++ AC_MSG_ERROR([Neither pidfd_open or __NR_pidfd_open found. Disable pidwait with configure option --disable-pidwait]) ++ ) ++ ,[]) ++ + AC_ARG_ENABLE([kill], + AS_HELP_STRING([--disable-kill], [do not build kill]), + [], [enable_kill=yes] +-- +2.43.0 + diff --git a/package/procps-ng/0004-build-sys-Fix-define-of-HAVE_PIDFD_OPEN.patch b/package/procps-ng/0004-build-sys-Fix-define-of-HAVE_PIDFD_OPEN.patch new file mode 100644 index 0000000000..aa65df7706 --- /dev/null +++ b/package/procps-ng/0004-build-sys-Fix-define-of-HAVE_PIDFD_OPEN.patch @@ -0,0 +1,37 @@ +From 1e2d53e1d86cb75e8f39b42b9a5c409e5d3261ed Mon Sep 17 00:00:00 2001 +From: oli-ben <24815225-oli-ben@users.noreply.gitlab.com> +Date: Wed, 27 Nov 2024 20:17:11 +0000 +Subject: [PATCH] build-sys: Fix define of HAVE_PIDFD_OPEN + +Fix cross-compilation issues when using GCC-12.3 +* configure fails to detect that the sysroot does support the pidfd_open + syscall wrapper +* configure fails to define HAVE_PIDFD_OPEN which is used in pgrep.c, + so it fails building, because __NR_pidfd_open is then undefined + +Upstream: https://gitlab.com/procps-ng/procps/-/commit/587efb47df7ddbfda4e6abdd1e7792a2531a238f + +Signed-off-by: Scott Fan <fancp2007@gmail.com> +[Scott: backported to version 4.0.4] +--- + configure.ac | 4 ++-- + 1 file changed, 2 insertions(+), 2 deletions(-) + +diff --git a/configure.ac b/configure.ac +index 0719fcd1..6242a8f8 100644 +--- a/configure.ac ++++ b/configure.ac +@@ -317,8 +317,8 @@ AC_ARG_ENABLE([pidwait], + ) + AM_CONDITIONAL(BUILD_PIDWAIT, test "x$enable_pidwait" = xyes) + AC_MSG_CHECKING([for pidfd_open()]) +-AC_LINK_IFELSE([AC_LANG_PROGRAM([], [[ [pidfd_open(1,1)]]])], +- have_pidfd_open=yes; AC_MSG_RESULT([yes]) , ++AC_LINK_IFELSE([AC_LANG_PROGRAM([], [[pidfd_open(1,1)]])], ++ have_pidfd_open=yes; AC_DEFINE(HAVE_PIDFD_OPEN, 1, [pidfd_open is defined]) AC_MSG_RESULT([yes]), + have_pidfd_open=no; AC_MSG_RESULT([no]) + ) + +-- +2.43.0 + diff --git a/package/procps-ng/0005-pgrep-Include-sys-syscall.h-if-making-pidwait.patch b/package/procps-ng/0005-pgrep-Include-sys-syscall.h-if-making-pidwait.patch new file mode 100644 index 0000000000..74f93c8e49 --- /dev/null +++ b/package/procps-ng/0005-pgrep-Include-sys-syscall.h-if-making-pidwait.patch @@ -0,0 +1,41 @@ +From 81ded587d2484b6f470f2d5c837c5591491377ce Mon Sep 17 00:00:00 2001 +From: Craig Small <csmall@dropbear.xyz> +Date: Thu, 28 Nov 2024 07:20:42 +1100 +Subject: [PATCH] pgrep: Include sys/syscall.h if making pidwait + +sys/syscall.h would only be included if pidwait was made +and we found pidfd_open() The previous commit fixed the +finding part, but in fact we want sys/syscall.h either +way because syscall() is defined there too. + +Most of the time the header is included by other headers +but adding it explicitly means if that header is removed or +changed it still works. + +Signed-off-by: Craig Small <csmall@dropbear.xyz> + +Upstream: https://gitlab.com/procps-ng/procps/-/commit/5acbb5dc1587d688de646d739a97251eb893bbb0 + +Signed-off-by: Scott Fan <fancp2007@gmail.com> +[Scott: backported to version 4.0.4] +--- + src/pgrep.c | 2 -- + 1 file changed, 2 deletions(-) + +diff --git a/src/pgrep.c b/src/pgrep.c +index d8e57dff..a2607532 100644 +--- a/src/pgrep.c ++++ b/src/pgrep.c +@@ -44,9 +44,7 @@ + + #ifdef ENABLE_PIDWAIT + #include <sys/epoll.h> +-#ifndef HAVE_PIDFD_OPEN + #include <sys/syscall.h> +-#endif /* !HAVE_PIDFD_OPEN */ + #endif + + /* EXIT_SUCCESS is 0 */ +-- +2.43.0 + diff --git a/package/procps-ng/procps-ng.mk b/package/procps-ng/procps-ng.mk index c5185c2c8a..9200b6faa6 100644 --- a/package/procps-ng/procps-ng.mk +++ b/package/procps-ng/procps-ng.mk @@ -16,6 +16,8 @@ PROCPS_NG_CONF_OPTS = LIBS=$(TARGET_NLS_LIBS) # Applying 0001-build-sys-Add-systemd-elogind-to-w.patch touches Makefile.am # Applying 0002-fix-ncurses-h-include.patch touches configure.ac +# Applying 0003-build-sys-Fix-pidfd_open-checking.patch touches configure.ac +# Applying 0004-build-sys-Fix-define-of-HAVE_PIDFD_OPEN.patch touches configure.ac PROCPS_NG_AUTORECONF = YES ifeq ($(BR2_PACKAGE_SYSTEMD),y) -- 2.43.0 _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply related [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-11-28 3:50 ` Scott Fan @ 2024-12-05 16:57 ` Peter Korsgaard 2024-12-07 9:47 ` Peter Korsgaard 0 siblings, 1 reply; 13+ messages in thread From: Peter Korsgaard @ 2024-12-05 16:57 UTC (permalink / raw) To: Scott Fan; +Cc: buildroot, thomas.petazzoni >>>>> "Scott" == Scott Fan <fancp2007@gmail.com> writes: > The previous build setup would check for pidfd_open using > AC_CHECK_FUNC and would be incorrectly reported as true. > Backport patch from upstream: > [1] https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 > [2] https://gitlab.com/procps-ng/procps/-/commit/587efb47df7ddbfda4e6abdd1e7792a2531a238f > [3] https://gitlab.com/procps-ng/procps/-/commit/5acbb5dc1587d688de646d739a97251eb893bbb0 > Signed-off-by: Scott Fan <fancp2007@gmail.com> Committed after adding a reference to an autobuilder failure, thanks. -- Bye, Peter Korsgaard _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
* Re: [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking 2024-12-05 16:57 ` Peter Korsgaard @ 2024-12-07 9:47 ` Peter Korsgaard 0 siblings, 0 replies; 13+ messages in thread From: Peter Korsgaard @ 2024-12-07 9:47 UTC (permalink / raw) To: Scott Fan; +Cc: buildroot, thomas.petazzoni >>>>> "Peter" == Peter Korsgaard <peter@korsgaard.com> writes: >>>>> "Scott" == Scott Fan <fancp2007@gmail.com> writes: >> The previous build setup would check for pidfd_open using >> AC_CHECK_FUNC and would be incorrectly reported as true. >> Backport patch from upstream: >> [1] https://gitlab.com/procps-ng/procps/-/commit/2507bc475782ff5e0541d37c780dff1e293c9553 >> [2] https://gitlab.com/procps-ng/procps/-/commit/587efb47df7ddbfda4e6abdd1e7792a2531a238f >> [3] https://gitlab.com/procps-ng/procps/-/commit/5acbb5dc1587d688de646d739a97251eb893bbb0 >> Signed-off-by: Scott Fan <fancp2007@gmail.com> > Committed after adding a reference to an autobuilder failure, thanks. Committed to 2024.02.x and 2024.08.x, thanks. -- Bye, Peter Korsgaard _______________________________________________ buildroot mailing list buildroot@buildroot.org https://lists.buildroot.org/mailman/listinfo/buildroot ^ permalink raw reply [flat|nested] 13+ messages in thread
end of thread, other threads:[~2024-12-07 9:48 UTC | newest] Thread overview: 13+ messages (download: mbox.gz follow: Atom feed -- links below jump to the message on this page -- 2024-10-30 0:20 [Buildroot] [PATCH] package/procps-ng: fix pidfd_open checking Scott Fan 2024-10-31 18:18 ` Thomas Petazzoni via buildroot 2024-11-01 1:31 ` Scott Fan 2024-11-04 13:23 ` Scott Fan 2024-11-24 16:29 ` Thomas Petazzoni via buildroot 2024-11-26 1:50 ` Scott Fan 2024-11-26 13:57 ` Thomas Petazzoni via buildroot 2024-11-27 0:35 ` Scott Fan 2024-11-27 1:01 ` Scott Fan 2024-11-27 7:44 ` Thomas Petazzoni via buildroot 2024-11-28 3:50 ` Scott Fan 2024-12-05 16:57 ` Peter Korsgaard 2024-12-07 9:47 ` Peter Korsgaard
This is a public inbox, see mirroring instructions for how to clone and mirror all data and code used for this inbox; as well as URLs for NNTP newsgroup(s).