diff mbox

busybox: linux-pam support requires threads

Message ID 6d2d86487b691cfe8c1c66a9630d0e51461290c4.1437388422.git.baruch@tkos.co.il
State Rejected
Headers show

Commit Message

Baruch Siach July 20, 2015, 10:33 a.m. UTC
Commit 083aeac94e19 (busybox: added linux-pam support, 2015-07-14) enabled
CONFIG_PAM when the linux-pam package is selected. Busybox build system adds
-lpthread unconditionally when CONFIG_PAM is enabled, which leads to build
failure when the toolchain does not support pthreads. Enable CONFIG_PAM only
when the toolchain supports threads.

Fixes:
http://autobuild.buildroot.net/results/8df/8df14bde19a41b78aa6422a1649a570914f747ee/
http://autobuild.buildroot.net/results/30d/30dd11b576b590de513031fce5e034353f2f5544/
http://autobuild.buildroot.net/results/de6/de628d84b0ed49be8f5d021c789357b2e66f2b02/

Cc: Matt Weber <matthew.weber@rockwellcollins.com>
Signed-off-by: Baruch Siach <baruch@tkos.co.il>
---
 package/busybox/busybox.mk | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

Comments

Thomas Petazzoni July 20, 2015, 12:38 p.m. UTC | #1
Dear Baruch Siach,

On Mon, 20 Jul 2015 13:33:42 +0300, Baruch Siach wrote:
> Commit 083aeac94e19 (busybox: added linux-pam support, 2015-07-14) enabled
> CONFIG_PAM when the linux-pam package is selected. Busybox build system adds
> -lpthread unconditionally when CONFIG_PAM is enabled, which leads to build
> failure when the toolchain does not support pthreads. Enable CONFIG_PAM only
> when the toolchain supports threads.
> 
> Fixes:
> http://autobuild.buildroot.net/results/8df/8df14bde19a41b78aa6422a1649a570914f747ee/
> http://autobuild.buildroot.net/results/30d/30dd11b576b590de513031fce5e034353f2f5544/
> http://autobuild.buildroot.net/results/de6/de628d84b0ed49be8f5d021c789357b2e66f2b02/
> 
> Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> Signed-off-by: Baruch Siach <baruch@tkos.co.il>

I was pretty sure I already fixed that one. And I indeed already fixed
it. Except that after that I merged another patch adding again
Linux-PAM support in Busybox. Big mistake on my side:

083aeac94e19cde6abd4b29cf29db589327d8c49 busybox: added linux-pam support
b0b3dc5612a012dd29293bee0cd848aa1b7c0b7d busybox: selinux support
cdcd83f31ded1dbf6d13d16b4aafdf594b964d94 package: fix fallout from checking kconfig config files
3ff5f63e48237cf569aaba6be35e179bf8556a19 package/busybox: check for config file
89d39fc7a392530be043bac541ade0bef746edb6 initscripts: new package
b98dd2355780c732bf440042fb8faafe731efc94 busybox: improve support for telnetd
e1baac68bcb9fb3cb3d2603d238a2ce8e7eb1125 busybox: Install udhcpc script only if enabled
aa1362f220e733689fe4210b0bb2433f6d595e6b busybox: adjust configuration to add fancy options to the sleep applet
98e1848db11e707f96504ccf9cb2ed09e6787dd1 busybox: PAM support needs thread support
f21b2558a097d07b53b6f20a47770468e9e4f7ed busybox: added linux-pam support

f21b2558a097d07b53b6f20a47770468e9e4f7ed was merged first, I fixed the
thread dependency in 98e1848db11e707f96504ccf9cb2ed09e6787dd1 but then
later on, I merged again a linux-pam dependency addition, which didn't
conflict because the piece of code was added at a different place.

So really, the proper fix is to revert
083aeac94e19cde6abd4b29cf29db589327d8c49. Can you send a patch doing
this revert?

Sorry for the mess :-/

Thomas
Baruch Siach July 20, 2015, 1:03 p.m. UTC | #2
Hi Thomas,

On Mon, Jul 20, 2015 at 02:38:32PM +0200, Thomas Petazzoni wrote:
> On Mon, 20 Jul 2015 13:33:42 +0300, Baruch Siach wrote:
> > Commit 083aeac94e19 (busybox: added linux-pam support, 2015-07-14) enabled
> > CONFIG_PAM when the linux-pam package is selected. Busybox build system adds
> > -lpthread unconditionally when CONFIG_PAM is enabled, which leads to build
> > failure when the toolchain does not support pthreads. Enable CONFIG_PAM only
> > when the toolchain supports threads.
> > 
> > Fixes:
> > http://autobuild.buildroot.net/results/8df/8df14bde19a41b78aa6422a1649a570914f747ee/
> > http://autobuild.buildroot.net/results/30d/30dd11b576b590de513031fce5e034353f2f5544/
> > http://autobuild.buildroot.net/results/de6/de628d84b0ed49be8f5d021c789357b2e66f2b02/
> > 
> > Cc: Matt Weber <matthew.weber@rockwellcollins.com>
> > Signed-off-by: Baruch Siach <baruch@tkos.co.il>
> 
> I was pretty sure I already fixed that one. And I indeed already fixed
> it. Except that after that I merged another patch adding again
> Linux-PAM support in Busybox. Big mistake on my side:
> 
> 083aeac94e19cde6abd4b29cf29db589327d8c49 busybox: added linux-pam support
> b0b3dc5612a012dd29293bee0cd848aa1b7c0b7d busybox: selinux support
> cdcd83f31ded1dbf6d13d16b4aafdf594b964d94 package: fix fallout from checking kconfig config files
> 3ff5f63e48237cf569aaba6be35e179bf8556a19 package/busybox: check for config file
> 89d39fc7a392530be043bac541ade0bef746edb6 initscripts: new package
> b98dd2355780c732bf440042fb8faafe731efc94 busybox: improve support for telnetd
> e1baac68bcb9fb3cb3d2603d238a2ce8e7eb1125 busybox: Install udhcpc script only if enabled
> aa1362f220e733689fe4210b0bb2433f6d595e6b busybox: adjust configuration to add fancy options to the sleep applet
> 98e1848db11e707f96504ccf9cb2ed09e6787dd1 busybox: PAM support needs thread support
> f21b2558a097d07b53b6f20a47770468e9e4f7ed busybox: added linux-pam support
> 
> f21b2558a097d07b53b6f20a47770468e9e4f7ed was merged first, I fixed the
> thread dependency in 98e1848db11e707f96504ccf9cb2ed09e6787dd1 but then
> later on, I merged again a linux-pam dependency addition, which didn't
> conflict because the piece of code was added at a different place.
> 
> So really, the proper fix is to revert
> 083aeac94e19cde6abd4b29cf29db589327d8c49. Can you send a patch doing
> this revert?

Just did. Thanks for the analysis.

baruch
diff mbox

Patch

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index ce6652b017d7..54351b115e90 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -141,7 +141,7 @@  define BUSYBOX_SET_INIT
 endef
 endif
 
-ifeq ($(BR2_PACKAGE_LINUX_PAM),y)
+ifeq ($(BR2_PACKAGE_LINUX_PAM)$(BR2_TOOLCHAIN_HAS_THREADS),yy)
 define BUSYBOX_LINUX_PAM
 	$(call KCONFIG_ENABLE_OPT,CONFIG_PAM,$(BUSYBOX_BUILD_CONFIG))
 endef