buildroot.buildroot.org archive mirror
 help / color / mirror / Atom feed
* [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).