Patchwork busybox: needs linux-pam dependency added if using custom busybox .config & busybox login applet

login
register
mail settings
Submitter Stefan Fröberg
Date Nov. 18, 2012, 12:15 a.m.
Message ID <1353197713-20796-1-git-send-email-stefan.froberg@petroprogram.com>
Download mbox | patch
Permalink /patch/199891/
State Rejected
Headers show

Comments

Stefan Fröberg - Nov. 18, 2012, 12:15 a.m.
If BusyBox custom .config has login applet (CONFIG_LOGIN) enabled
then the build process will complain about missing Linux PAM
headers.
This will add the needed linux-pam dependency

Signed-off-by: Stefan Fröberg <stefan.froberg@petroprogram.com>
---
 package/busybox/busybox.mk |    7 +++++++
 1 files changed, 7 insertions(+), 0 deletions(-)
Danomi Manchego - Nov. 18, 2012, 2:14 a.m.
On Sat, Nov 17, 2012 at 7:15 PM, Stefan Fröberg <
stefan.froberg@petroprogram.com> wrote:
>
> +ifeq ($(shell sed -n "s/CONFIG_LOGIN=\(y\)/\1/p"
> $(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
> +BUSYBOX_DEPENDENCIES += linux-pam
> +endif
>

Why bother with the sed?  Could you not do "ifeq
($(BR2_PACKAGE_LINUX_PAM),y)"?

Just a thought ...
Danomi -
Stefan Fröberg - Nov. 18, 2012, 11:51 a.m.
Hi Danomi
18.11.2012 4:14, Danomi Manchego kirjoitti:
>
>
>
> On Sat, Nov 17, 2012 at 7:15 PM, Stefan Fröberg
> <stefan.froberg@petroprogram.com
> <mailto:stefan.froberg@petroprogram.com>> wrote:
>
>     +ifeq ($(shell sed -n "s/CONFIG_LOGIN=\(y\)/\1/p"
>     $(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
>     +BUSYBOX_DEPENDENCIES += linux-pam
>     +endif
>
>  
> Why bother with the sed?  Could you not do "ifeq
> ($(BR2_PACKAGE_LINUX_PAM),y)"?
>
No because I don't have a need for full linux-pam login binary.

I only need linux-pam because I have login applet enabled inside my
custom BusyBox .config file
and that needs Linux Pam headers.


Regards
Stefan

> Just a thought ...
> Danomi -
>
Stefan Fröberg - Nov. 18, 2012, 12:11 p.m.
18.11.2012 13:51, Stefan Fröberg kirjoitti:
> Hi Danomi
> 18.11.2012 4:14, Danomi Manchego kirjoitti:
>>
>>
>>
>> On Sat, Nov 17, 2012 at 7:15 PM, Stefan Fröberg
>> <stefan.froberg@petroprogram.com
>> <mailto:stefan.froberg@petroprogram.com>> wrote:
>>
>>     +ifeq ($(shell sed -n "s/CONFIG_LOGIN=\(y\)/\1/p"
>>     $(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
>>     +BUSYBOX_DEPENDENCIES += linux-pam
>>     +endif
>>
>>  
>> Why bother with the sed?  Could you not do "ifeq
>> ($(BR2_PACKAGE_LINUX_PAM),y)"?
>>
> No because I don't have a need for full linux-pam login binary.
>
> I only need linux-pam because I have login applet enabled inside my
> custom BusyBox .config file
> and that needs Linux Pam headers.
>
Or to but it in another way:

BR2_PACKAGE_LINUX_PAM is not enough. BusyBox .config must also have
CONFIG_LOGIN enabled.
And that needs to be determined before the whole .mk is parsed, at a
runtime, so that proper
dependency can be added.

In normal default buildroot provided BusyBox .config there is no
CONFIG_LOGIN enabled in.
So for that case this will change nothing.

It seems that the stuff inside .mk files are really static (and so
called functions are not functions at all
but just variables containing text).

So I had to go outside of .mk file for a moment and determine if
CONFIG_LOGIN was enabled outside of extracted
busybox  dir and then add the needed dependency.

I know this is ugly but Im open to suggestions.

Regards
Stefan




>
> Regards
> Stefan
>
>> Just a thought ...
>> Danomi -
>>
>
>
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Stefan Fröberg - Nov. 18, 2012, 2:01 p.m.
18.11.2012 14:11, Stefan Fröberg kirjoitti:
> 18.11.2012 13:51, Stefan Fröberg kirjoitti:
>> Hi Danomi
>> 18.11.2012 4:14, Danomi Manchego kirjoitti:
>>>
>>>
>>>
>>> On Sat, Nov 17, 2012 at 7:15 PM, Stefan Fröberg
>>> <stefan.froberg@petroprogram.com
>>> <mailto:stefan.froberg@petroprogram.com>> wrote:
>>>
>>>     +ifeq ($(shell sed -n "s/CONFIG_LOGIN=\(y\)/\1/p"
>>>     $(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
>>>     +BUSYBOX_DEPENDENCIES += linux-pam
>>>     +endif
>>>
>>>  
>>> Why bother with the sed?  Could you not do "ifeq
>>> ($(BR2_PACKAGE_LINUX_PAM),y)"?
>>>
>> No because I don't have a need for full linux-pam login binary.
>>
>> I only need linux-pam because I have login applet enabled inside my
>> custom BusyBox .config file
>> and that needs Linux Pam headers.
>>
> Or to but it in another way:
>
> BR2_PACKAGE_LINUX_PAM is not enough. BusyBox .config must also have
> CONFIG_LOGIN enabled.
> And that needs to be determined before the whole .mk is parsed, at a
> runtime, so that proper
> dependency can be added.
>
> In normal default buildroot provided BusyBox .config there is no
> CONFIG_LOGIN enabled in.
> So for that case this will change nothing.
>
> It seems that the stuff inside .mk files are really static (and so
> called functions are not functions at all
> but just variables containing text).
>
> So I had to go outside of .mk file for a moment and determine if
> CONFIG_LOGIN was enabled outside of extracted
> busybox  dir and then add the needed dependency.
>
> I know this is ugly but Im open to suggestions.
>
> Regards
> Stefan
>
Actually, maybe that should be changed to host-linux-pam ...
Because AFAIK, BusyBox login applet needs only headers and nothing more ???

>
>
>
>>
>> Regards
>> Stefan
>>
>>> Just a thought ...
>>> Danomi -
>>>
>>
>>
>>
>> _______________________________________________
>> buildroot mailing list
>> buildroot@busybox.net
>> http://lists.busybox.net/mailman/listinfo/buildroot
>
>
>
> _______________________________________________
> buildroot mailing list
> buildroot@busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot
Thomas Petazzoni - Nov. 18, 2012, 4:40 p.m.
Dear Stefan Fröberg,

On Sun, 18 Nov 2012 02:15:13 +0200, Stefan Fröberg wrote:

> +# linux-pam must be built first if user has custom
> +# BusyBox .config file and that file has also login
> +# applet (CONFIG_LOGIN) enabled.
> +ifeq ($(shell sed -n "s/CONFIG_LOGIN=\(y\)/\1/p" $(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
> +BUSYBOX_DEPENDENCIES += linux-pam
> +endif

I don't think this will work nicely. BR2_PACKAGE_BUSYBOX_CONFIG is the
source for the Busybox configuration, but the user can do "make
busybox-menuconfig" do adjust it. In this case, the contents of the
BR2_PACKAGE_BUSYBOX_CONFIG file and the contents of the real
configuration file used by Busybox are different. If the latter has
CONFIG_LOGIN, but not the former, then you will not link against
linux-pam while you should.

It is really difficult to have dependencies that are needed only when
some specific Busybox features are enabled.

Best regards,

Thomas
Stefan Fröberg - Nov. 18, 2012, 5:39 p.m.
Hi Thomas

18.11.2012 18:40, Thomas Petazzoni kirjoitti:
> Dear Stefan Fröberg,
>
> On Sun, 18 Nov 2012 02:15:13 +0200, Stefan Fröberg wrote:
>
>> +# linux-pam must be built first if user has custom
>> +# BusyBox .config file and that file has also login
>> +# applet (CONFIG_LOGIN) enabled.
>> +ifeq ($(shell sed -n "s/CONFIG_LOGIN=\(y\)/\1/p" $(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
>> +BUSYBOX_DEPENDENCIES += linux-pam
>> +endif
> I don't think this will work nicely. BR2_PACKAGE_BUSYBOX_CONFIG is the
> source for the Busybox configuration, but the user can do "make
> busybox-menuconfig" do adjust it. In this case, the contents of the
> BR2_PACKAGE_BUSYBOX_CONFIG file and the contents of the real
> configuration file used by Busybox are different. If the latter has
> CONFIG_LOGIN, but not the former, then you will not link against
> linux-pam while you should.
I tested this and it seems that no matter what you do in make
busybox-menuconfig the
(enable login or disable login) the settings from
BR2_PACKAGE_BUSYBOX_CONFIG will
always override.
So even if setting CONFIG_LOGIN disabled from make busybox-menuconfig it
will still
build linux-pam first.

I try to get the logic of this thing:

First busybox is extracted and it copies $(BUSYBOX_CONFIG_FILE) (really
a BR2_PACKAGE_BUSYBOX_CONFIG)
over to just extracted output/busybox-xyz/.config and replacing it. That
will be done in BUSYBOX_POST_EXTRACT_HOOKS.

(BR2_PACKAGE_BUSYBOX_CONFIG here being either the default
package/busybox/busybox-xyz.x.config
 or user provided custom .config)

Then buildroot makes some of it's own changes to that
output/busybox/.config inside BUSYBOX_CONFIGURE_CMDS

Then build.

So what  make busybox-menuconfig   actually does is change 
output/busybox-xyz/.config.

So instead of getting CONFIG_LOG setting from BR2_PACKAGE_BUSYBOX_CONFIG
(aka BUSYBOX_CONFIG_FILE inside busybox.mk)
I should get it from output/busybox-xyz/.config (aka
BUSYBOX_BUILD_CONFIG inside busybox.mk).

And that CONFIG_LOGIN checking should be done after buybox tarball has
been extracted but by then, it is already too late for
determining dependencies ....
Aarrggggh!


> It is really difficult to have dependencies that are needed only when
> some specific Busybox features are enabled.
Tell me about it.
What IMHO is needed, is some function or way to get these kinds of
information.
Just before dependencies are determined.

There should be KCONFIG_GET_OPT or something similar inside
package/pkg-utils.mk
(like there already is KCONFIG_ENABLE_OPT, KCONFIG_DISABLE_OPT and
KCONFIG_SET_OPT)
that would help to determine not only buildroot features but also linux
features and any
else package that use kconfig like stuff.

It would also help in case of  various package conflicts (say if busybox
unzip applet and real unzip are mistakenly
being installed, then this could be detected beforehand)

Regards
Stefan


> Best regards,
>
> Thomas
Thomas Petazzoni - Nov. 18, 2012, 5:48 p.m.
Dear Stefan Fröberg,

On Sun, 18 Nov 2012 19:39:16 +0200, Stefan Fröberg wrote:

> And that CONFIG_LOGIN checking should be done after buybox tarball has
> been extracted but by then, it is already too late for
> determining dependencies ....
> Aarrggggh!

I think you understood what the problem is :)

> > It is really difficult to have dependencies that are needed only when
> > some specific Busybox features are enabled.
> Tell me about it.
> What IMHO is needed, is some function or way to get these kinds of
> information.
> Just before dependencies are determined.
> 
> There should be KCONFIG_GET_OPT or something similar inside
> package/pkg-utils.mk
> (like there already is KCONFIG_ENABLE_OPT, KCONFIG_DISABLE_OPT and
> KCONFIG_SET_OPT)
> that would help to determine not only buildroot features but also linux
> features and any
> else package that use kconfig like stuff.
> 
> It would also help in case of  various package conflicts (say if busybox
> unzip applet and real unzip are mistakenly
> being installed, then this could be detected beforehand)

KCONFIG_GET_OPT cannot work: you could only call it in a Makefile rule
(i.e inside BUILD_CMDS or CONFIGURE_CMDS or a hook or something like
that). But by the time those are executed, it is way too late to
declare a dependency. The dependencies (in BUSYBOX_DEPENDENCIES) must
be detailed when make parses all the .mk files, not when the rules are
executed.

Unless I'm wrong, linux-pam is only needed if ENABLE_PAM is defined
when building Busybox, correct? So, CONFIG_LOGIN builds just fine
without PAM, doesn't it?

So maybe you should rather do:

ifeq ($(BR2_PACKAGE_LINUX_PAM),y)
BUSYBOX_DEPENDENCIES += linux-pam
BUSYBOX_CFLAGS += -DENABLE_PAM
endif

No?

Thomas
Stefan Fröberg - Nov. 18, 2012, 6:09 p.m.
18.11.2012 19:48, Thomas Petazzoni kirjoitti:
> Dear Stefan Fröberg,
>
> On Sun, 18 Nov 2012 19:39:16 +0200, Stefan Fröberg wrote:
>
>> And that CONFIG_LOGIN checking should be done after buybox tarball has
>> been extracted but by then, it is already too late for
>> determining dependencies ....
>> Aarrggggh!
> I think you understood what the problem is :)
>
>>> It is really difficult to have dependencies that are needed only when
>>> some specific Busybox features are enabled.
>> Tell me about it.
>> What IMHO is needed, is some function or way to get these kinds of
>> information.
>> Just before dependencies are determined.
>>
>> There should be KCONFIG_GET_OPT or something similar inside
>> package/pkg-utils.mk
>> (like there already is KCONFIG_ENABLE_OPT, KCONFIG_DISABLE_OPT and
>> KCONFIG_SET_OPT)
>> that would help to determine not only buildroot features but also linux
>> features and any
>> else package that use kconfig like stuff.
>>
>> It would also help in case of  various package conflicts (say if busybox
>> unzip applet and real unzip are mistakenly
>> being installed, then this could be detected beforehand)
> KCONFIG_GET_OPT cannot work: you could only call it in a Makefile rule
> (i.e inside BUILD_CMDS or CONFIGURE_CMDS or a hook or something like
> that). But by the time those are executed, it is way too late to
> declare a dependency. The dependencies (in BUSYBOX_DEPENDENCIES) must
> be detailed when make parses all the .mk files, not when the rules are
> executed.
>
> Unless I'm wrong, linux-pam is only needed if ENABLE_PAM is defined
> when building Busybox, correct? So, CONFIG_LOGIN builds just fine
> without PAM, doesn't it?
Yes that is correct, there is a CONFIG_PAM in BusyBox .config  and althought
I have never actually tried to build it without pam it should in theory
work.

>
> So maybe you should rather do:
>
> ifeq ($(BR2_PACKAGE_LINUX_PAM),y)
> BUSYBOX_DEPENDENCIES += linux-pam
> BUSYBOX_CFLAGS += -DENABLE_PAM
> endif
>
> No?

Hmmm...
I will try that and see.

Thanks!

Regards
Stefan
> Thomas
Thomas Petazzoni - Nov. 18, 2012, 6:28 p.m.
Dear Stefan Fröberg,

On Sun, 18 Nov 2012 20:09:29 +0200, Stefan Fröberg wrote:

> > ifeq ($(BR2_PACKAGE_LINUX_PAM),y)
> > BUSYBOX_DEPENDENCIES += linux-pam
> > BUSYBOX_CFLAGS += -DENABLE_PAM
> > endif

Hum, wait, I didn't see there was a CONFIG_PAM option. So what you want
to do is:

ifeq ($(BR2_PACKAGE_LINUX_PAM),y)
BUSYBOX_DEPENDENCIES += linux-pam
endif

And then, in the Busybox configuration step, do:

ifeq ($(BR2_PACKAGE_LINUX_PAM),y)
	$(call KCONFIG_SET_OPT,CONFIG_PAM,/path/to/busybox/config/file)
endif

or something along those lines.

Thomas

Patch

diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk
index 549e150..f2a102a 100644
--- a/package/busybox/busybox.mk
+++ b/package/busybox/busybox.mk
@@ -30,6 +30,13 @@  BUSYBOX_CFLAGS += -I$(STAGING_DIR)/usr/include/tirpc/
 BUSYBOX_LDFLAGS += -ltirpc
 endif
 
+# linux-pam must be built first if user has custom
+# BusyBox .config file and that file has also login
+# applet (CONFIG_LOGIN) enabled.
+ifeq ($(shell sed -n "s/CONFIG_LOGIN=\(y\)/\1/p" $(BR2_PACKAGE_BUSYBOX_CONFIG)),y)
+BUSYBOX_DEPENDENCIES += linux-pam
+endif
+
 BUSYBOX_BUILD_CONFIG = $(BUSYBOX_DIR)/.config
 # Allows the build system to tweak CFLAGS
 BUSYBOX_MAKE_ENV = $(TARGET_MAKE_ENV) CFLAGS="$(BUSYBOX_CFLAGS)"