Patchwork Add package linux-pam (resending, fixed indentation, mentioned website)

login
register
mail settings
Submitter Dimitry Golubovsky
Date July 18, 2012, 9:34 p.m.
Message ID <1342647267-20745-1-git-send-email-golubovsky@gmail.com>
Download mbox | patch
Permalink /patch/171846/
State Superseded
Headers show

Comments

Dimitry Golubovsky - July 18, 2012, 9:34 p.m.
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
Thomas Petazzoni - July 20, 2012, 8:45 p.m.
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
Thomas Petazzoni - July 20, 2012, 9:58 p.m.
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
Dimitry Golubovsky - July 21, 2012, 1:19 a.m.
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.
Thomas Petazzoni - July 21, 2012, 12:47 p.m.
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

Patch

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))