* [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).