| Message ID | 1342647267-20745-1-git-send-email-golubovsky@gmail.com |
|---|---|
| State | Superseded |
| Headers | show |
Hello Dmitry, Le Wed, 18 Jul 2012 17:34:27 -0400, Dmitry <golubovsky@gmail.com> a écrit : > 1. Add the linux-pam package building instructions > 2. If the linux-pam package is selected, make sure it builds before > busybox just in case busybox is configured with PAM support > (make linux-pam a dependency of busybox) > > Signed-off-by: Dmitry <golubovsky@gmail.com> > --- > package/Config.in | 1 + > package/busybox/busybox.mk | 3 +++ > package/linux-pam/Config.in | 15 +++++++++++++++ > package/linux-pam/linux-pam.mk | 37 +++++++++++++++++++++++++++++++++++++ > 4 files changed, 56 insertions(+), 0 deletions(-) > create mode 100644 package/linux-pam/Config.in > create mode 100644 package/linux-pam/linux-pam.mk > > diff --git a/package/Config.in b/package/Config.in > index 6aabb8e..4bbb293 100644 > --- a/package/Config.in > +++ b/package/Config.in > @@ -660,6 +660,7 @@ source "package/bootutils/Config.in" > endif > source "package/htop/Config.in" > source "package/kmod/Config.in" > +source "package/linux-pam/Config.in" linux-pam is a library, so it should rather go in Libraries -> Other in the menuconfig. > if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS > source "package/module-init-tools/Config.in" > endif > diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk > index 394a1ae..4a1de33 100644 > --- a/package/busybox/busybox.mk > +++ b/package/busybox/busybox.mk > @@ -11,6 +11,9 @@ else > BUSYBOX_VERSION = $(call qstrip,$(BR2_BUSYBOX_VERSION)) > BUSYBOX_SITE = http://www.busybox.net/downloads > endif > +ifeq ($(BR2_PACKAGE_LINUX_PAM),y) > +BUSYBOX_DEPENDENCIES += linux-pam > +endif > BUSYBOX_SOURCE = busybox-$(BUSYBOX_VERSION).tar.bz2 > BUSYBOX_BUILD_CONFIG = $(BUSYBOX_DIR)/.config This Busybox change should be part of a separate patch (so your patch series should have two patches: one adding the linux-pam package, one tweaking Busybox to use linux-pam). Also, shouldn't you ensure that CONFIG_PAM is enabled in the Busybox configuration in that case? > # Allows the build system to tweak CFLAGS > diff --git a/package/linux-pam/Config.in b/package/linux-pam/Config.in > new file mode 100644 > index 0000000..8a9bbca > --- /dev/null > +++ b/package/linux-pam/Config.in > @@ -0,0 +1,15 @@ > +config BR2_PACKAGE_LINUX_PAM > + bool "linux-pam" > + select BR2_PACKAGE_LIBINTL > + select BR2_PACKAGE_GETTEXT No, see http://buildroot.org/downloads/manual/manual.html#_gettext_integration_and_interaction_with_packages. > diff --git a/package/linux-pam/linux-pam.mk b/package/linux-pam/linux-pam.mk > new file mode 100644 > index 0000000..5024033 > --- /dev/null > +++ b/package/linux-pam/linux-pam.mk > @@ -0,0 +1,37 @@ > +############################################ > +# > +# linux-pam > +# > +############################################ > + > +LINUX_PAM_VERSION = 1.1.4 > +LINUX_PAM_SOURCE = Linux-PAM-$(LINUX_PAM_VERSION).tar.bz2 > +LINUX_PAM_SITE = http://linux-pam.org/library/ > +LINUX_PAM_INSTALL_STAGING = YES > +LINUX_PAM_INSTALL_TARGET = YES > +LINUX_PAM_CONF_OPT = --disable-prelude --disable-isadir --disable-nis --disable-regenerate-docu > +LINUX_PAM_CONF_OPT += --enable-securedir=/lib/security --libdir=/lib > +LINUX_PAM_DEPENDENCIES = gettext libintl flex > +LINUX_PAM_LICENSE = BSD > +LINUX_PAM_LICENSE_FILES = COPYING > + > +define LINUX_PAM_BUILD_CMDS > + $(MAKE) CC="$(TARGET_CC) -lintl -lfl" LD="$(TARGET_LD)" -C $(@D) all > +endef > + > +define LINUX_PAM_DISABLE_INNETGR > + echo >>$(@D)/config.h > + echo "#undef HAVE_RUSEROK">>$(@D)/config.h > + echo "#define HAVE_RUSEROK_AF">>$(@D)/config.h > + echo "#define ruserok_af(a, b, c, d, e) (-1)" >>$(@D)/config.h > + echo "#undef innetgr">>$(@D)/config.h > + echo "#define innetgr(a, b, c, d) 0" >>$(@D)/config.h > + echo "all:" >$(@D)/doc/Makefile > + echo "" >>$(@D)/doc/Makefile > + echo "install:" >>$(@D)/doc/Makefile > + echo "" >>$(@D)/doc/Makefile > +endef Can you add a comment above this justifying why this is needed? Thanks! Best regards, Thomas
Le Fri, 20 Jul 2012 22:45:38 +0200, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> a écrit : > > +define LINUX_PAM_DISABLE_INNETGR > > + echo >>$(@D)/config.h > > + echo "#undef HAVE_RUSEROK">>$(@D)/config.h > > + echo "#define HAVE_RUSEROK_AF">>$(@D)/config.h > > + echo "#define ruserok_af(a, b, c, d, e) (-1)" >>$(@D)/config.h > > + echo "#undef innetgr">>$(@D)/config.h > > + echo "#define innetgr(a, b, c, d) 0" >>$(@D)/config.h > > + echo "all:" >$(@D)/doc/Makefile > > + echo "" >>$(@D)/doc/Makefile > > + echo "install:" >>$(@D)/doc/Makefile > > + echo "" >>$(@D)/doc/Makefile > > +endef > > Can you add a comment above this justifying why this is needed? Also, isn't there a nicer way of doing that? Like applying a nice patch to linux-pam instead? Thomas
Thomas, Apart from the comment regarding config.h patching (this is a separate undertaking): On Fri, Jul 20, 2012 at 4:45 PM, Thomas Petazzoni <thomas.petazzoni@free-electrons.com> wrote: > linux-pam is a library, so it should rather go in Libraries -> Other in > the menuconfig. Agree, will move it over there. >> --- a/package/busybox/busybox.mk >> +++ b/package/busybox/busybox.mk >> @@ -11,6 +11,9 @@ else >> BUSYBOX_VERSION = $(call qstrip,$(BR2_BUSYBOX_VERSION)) >> BUSYBOX_SITE = http://www.busybox.net/downloads >> endif >> +ifeq ($(BR2_PACKAGE_LINUX_PAM),y) >> +BUSYBOX_DEPENDENCIES += linux-pam >> +endif >> BUSYBOX_SOURCE = busybox-$(BUSYBOX_VERSION).tar.bz2 >> BUSYBOX_BUILD_CONFIG = $(BUSYBOX_DIR)/.config > > This Busybox change should be part of a separate patch (so your patch > series should have two patches: one adding the linux-pam package, one > tweaking Busybox to use linux-pam). Well, I thought about splitting the patch into two, but both parts can only be applied together then. > > Also, shouldn't you ensure that CONFIG_PAM is enabled in the Busybox > configuration in that case? IMHO these things are orthogonal. If BR2_PACKAGE_LINUX_PAM is selected, it will be built before busybox - what's the difference? If BR2_PACKAGE_LINUX_PAM is not selected, busybox.mk extra dependencies are not in effect - correct? If CONFIG_PAM is not selected in busybox, it is not affected by PAM other than the fact that PAM will be built earlier. The only situation when bad outcome is possible: CONFIG_PAM is selected in busybox, and BR2_PACKAGE_LINUX_PAM is not selected. But this will be obvious to the user, why busybox does not build. I don't really want to force PAM support in busybox as a consequence of user selection of BR2_PACKAGE_LINUX_PAM alone. The reverse makes more sense: I may add a sub-item in busybox config which enables PAM in both buildroot an busybox. Please let me know if this is acceptable. > > No, see > http://buildroot.org/downloads/manual/manual.html#_gettext_integration_and_interaction_with_packages. > I probably missed this, will fix. For the rest of your comment (fixing config.h post-configure) I need a closer look and some more time. Thanks for your feedback.
Le Fri, 20 Jul 2012 21:19:23 -0400, Dmitry Golubovsky <golubovsky@gmail.com> a écrit : > Apart from the comment regarding config.h patching (this is a separate > undertaking): Be careful: you can't patch config.h, it is generated during ./configure. So the best option is probably to see *why* you need to do these changes and see if by passing options or environment variables to the ./configure it isn't possible to achieve the same goal. > > This Busybox change should be part of a separate patch (so your patch > > series should have two patches: one adding the linux-pam package, one > > tweaking Busybox to use linux-pam). > > Well, I thought about splitting the patch into two, but both parts can > only be applied together then. Yes, no problem. > > Also, shouldn't you ensure that CONFIG_PAM is enabled in the Busybox > > configuration in that case? > > IMHO these things are orthogonal. > > If BR2_PACKAGE_LINUX_PAM is selected, it will be built before busybox > - what's the difference? If BR2_PACKAGE_LINUX_PAM is not selected, > busybox.mk extra dependencies are not in effect - correct? Correct. > If CONFIG_PAM is not selected in busybox, it is not affected by PAM > other than the fact that PAM will be built earlier. The only situation > when bad outcome is possible: CONFIG_PAM is selected in busybox, and > BR2_PACKAGE_LINUX_PAM is not selected. But this will be obvious to the > user, why busybox does not build. > > I don't really want to force PAM support in busybox as a consequence > of user selection of BR2_PACKAGE_LINUX_PAM alone. The reverse makes > more sense: I may add a sub-item in busybox config which enables PAM > in both buildroot an busybox. > > Please let me know if this is acceptable. Well, the thing is that this generally not really how we handle things in Buildroot. In general, as soon as some library is available and provide a feature that can be used by a package, then we automatically use it. Some examples: (1) Having RPC or IPv6 support in the toolchain will automatically enable certain Busybox applet or options. (2) Many packages use a library if available, without having a specific option for it. See for example how bind, ctorrent or libecore use openssl when available. We could have thought of doing the opposite: if CONFIG_PAM is enabled in the Busybox configuration, then we add linux-pam to the dependencies. But unfortunately, this is not possible: we cannot add things to <foo>_DEPENDENCIES depending on the Busybox configuration :-( So, I'd say I would prefer to follow what we do for all other packages. Basically, my understanding of our rule is that we do something that makes sense by default, and if someone really wants linux-pam for something else, but not for Busybox, that someone can do a very simple modification to busybox.mk to change this. Just like if someone wants openssl for something in its system, but not for bind or ctorrent, (s)he would have to make changes in the bind.mk or ctorrent.mk files. > > No, see > > http://buildroot.org/downloads/manual/manual.html#_gettext_integration_and_interaction_with_packages. > > I probably missed this, will fix. > > For the rest of your comment (fixing config.h post-configure) I need > a closer look and some more time. Ok, no problem. Thomas
diff --git a/package/Config.in b/package/Config.in index 6aabb8e..4bbb293 100644 --- a/package/Config.in +++ b/package/Config.in @@ -660,6 +660,7 @@ source "package/bootutils/Config.in" endif source "package/htop/Config.in" source "package/kmod/Config.in" +source "package/linux-pam/Config.in" if BR2_PACKAGE_BUSYBOX_SHOW_OTHERS source "package/module-init-tools/Config.in" endif diff --git a/package/busybox/busybox.mk b/package/busybox/busybox.mk index 394a1ae..4a1de33 100644 --- a/package/busybox/busybox.mk +++ b/package/busybox/busybox.mk @@ -11,6 +11,9 @@ else BUSYBOX_VERSION = $(call qstrip,$(BR2_BUSYBOX_VERSION)) BUSYBOX_SITE = http://www.busybox.net/downloads endif +ifeq ($(BR2_PACKAGE_LINUX_PAM),y) +BUSYBOX_DEPENDENCIES += linux-pam +endif BUSYBOX_SOURCE = busybox-$(BUSYBOX_VERSION).tar.bz2 BUSYBOX_BUILD_CONFIG = $(BUSYBOX_DIR)/.config # Allows the build system to tweak CFLAGS diff --git a/package/linux-pam/Config.in b/package/linux-pam/Config.in new file mode 100644 index 0000000..8a9bbca --- /dev/null +++ b/package/linux-pam/Config.in @@ -0,0 +1,15 @@ +config BR2_PACKAGE_LINUX_PAM + bool "linux-pam" + select BR2_PACKAGE_LIBINTL + select BR2_PACKAGE_GETTEXT + select BR2_PACKAGE_FLEX + select BR2_PACKAGE_FLEX_LIBFL + depends on (BR2_ENABLE_LOCALE && BR2_USE_WCHAR) + help + A Security Framework that Provides Authentication for Applications + + http://linux-pam.org + +comment "linux-pam requires a toolchain with WCHAR and locale support" + depends on !(BR2_ENABLE_LOCALE && BR2_USE_WCHAR) + diff --git a/package/linux-pam/linux-pam.mk b/package/linux-pam/linux-pam.mk new file mode 100644 index 0000000..5024033 --- /dev/null +++ b/package/linux-pam/linux-pam.mk @@ -0,0 +1,37 @@ +############################################ +# +# linux-pam +# +############################################ + +LINUX_PAM_VERSION = 1.1.4 +LINUX_PAM_SOURCE = Linux-PAM-$(LINUX_PAM_VERSION).tar.bz2 +LINUX_PAM_SITE = http://linux-pam.org/library/ +LINUX_PAM_INSTALL_STAGING = YES +LINUX_PAM_INSTALL_TARGET = YES +LINUX_PAM_CONF_OPT = --disable-prelude --disable-isadir --disable-nis --disable-regenerate-docu +LINUX_PAM_CONF_OPT += --enable-securedir=/lib/security --libdir=/lib +LINUX_PAM_DEPENDENCIES = gettext libintl flex +LINUX_PAM_LICENSE = BSD +LINUX_PAM_LICENSE_FILES = COPYING + +define LINUX_PAM_BUILD_CMDS + $(MAKE) CC="$(TARGET_CC) -lintl -lfl" LD="$(TARGET_LD)" -C $(@D) all +endef + +define LINUX_PAM_DISABLE_INNETGR + echo >>$(@D)/config.h + echo "#undef HAVE_RUSEROK">>$(@D)/config.h + echo "#define HAVE_RUSEROK_AF">>$(@D)/config.h + echo "#define ruserok_af(a, b, c, d, e) (-1)" >>$(@D)/config.h + echo "#undef innetgr">>$(@D)/config.h + echo "#define innetgr(a, b, c, d) 0" >>$(@D)/config.h + echo "all:" >$(@D)/doc/Makefile + echo "" >>$(@D)/doc/Makefile + echo "install:" >>$(@D)/doc/Makefile + echo "" >>$(@D)/doc/Makefile +endef + +LINUX_PAM_POST_CONFIGURE_HOOKS += LINUX_PAM_DISABLE_INNETGR + +$(eval $(autotools-package))
1. Add the linux-pam package building instructions 2. If the linux-pam package is selected, make sure it builds before busybox just in case busybox is configured with PAM support (make linux-pam a dependency of busybox) Signed-off-by: Dmitry <golubovsky@gmail.com> --- package/Config.in | 1 + package/busybox/busybox.mk | 3 +++ package/linux-pam/Config.in | 15 +++++++++++++++ package/linux-pam/linux-pam.mk | 37 +++++++++++++++++++++++++++++++++++++ 4 files changed, 56 insertions(+), 0 deletions(-) create mode 100644 package/linux-pam/Config.in create mode 100644 package/linux-pam/linux-pam.mk