| Message ID | 1353197713-20796-1-git-send-email-stefan.froberg@petroprogram.com |
|---|---|
| State | Rejected |
| Headers | show |
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 -
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 - >
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
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
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
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
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
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
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
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)"
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(-)