Buildroot Archive on lore.kernel.org
 help / color / mirror / Atom feed
From: Maxime Ripard <maxime.ripard@free-electrons.com>
To: buildroot@busybox.net
Subject: [Buildroot] [PATCH] Add package linux-pam
Date: Wed, 22 Aug 2012 09:09:52 +0200	[thread overview]
Message-ID: <503485C0.1000406@free-electrons.com> (raw)
In-Reply-To: <1344594434-22756-1-git-send-email-golubovsky@gmail.com>

Hi Dmitry,

This looks fine for me, I still have some comments however.

Le 10/08/2012 12:27, Dmitry a ?crit :
> Signed-off-by: Dmitry <golubovsky@gmail.com>
> ---
>  package/Config.in                                 |    1 +
>  package/linux-pam/Config.in                       |   15 +++++++++
>  package/linux-pam/linux-pam-configure.patch       |   20 ++++++++++++
>  package/linux-pam/linux-pam-doc-makefile-am.patch |   33 +++++++++++++++++++++
>  package/linux-pam/linux-pam-group.patch           |   26 ++++++++++++++++
>  package/linux-pam/linux-pam-rhosts.patch          |   24 +++++++++++++++
>  package/linux-pam/linux-pam-succeed.patch         |   31 +++++++++++++++++++
>  package/linux-pam/linux-pam-time.patch            |   26 ++++++++++++++++
>  package/linux-pam/linux-pam.mk                    |   25 ++++++++++++++++
>  9 files changed, 201 insertions(+), 0 deletions(-)
>  create mode 100644 package/linux-pam/Config.in
>  create mode 100644 package/linux-pam/linux-pam-configure.patch
>  create mode 100644 package/linux-pam/linux-pam-doc-makefile-am.patch
>  create mode 100644 package/linux-pam/linux-pam-group.patch
>  create mode 100644 package/linux-pam/linux-pam-rhosts.patch
>  create mode 100644 package/linux-pam/linux-pam-succeed.patch
>  create mode 100644 package/linux-pam/linux-pam-time.patch
>  create mode 100644 package/linux-pam/linux-pam.mk

From what I can see, this patchset only removes calls to ruserok and
innetgr. Could you merge them together so that all the conditionnal
removal of ruserok is only in one patch and all the conditional removal
of inetgr in another ?

Basically, you would have two patches:
  - the ruserok one, which is the merge of linux-pam-configure.patch and
linux-pam-rhosts.patch,
  - the innetgr one, which is the merge of linux-pam-group.patch,
linux-pam-succeed.patch and linux-pam-time.patch

Also, could you be more explicit with regard to patch names ? Like
linux-pam-disable-ruserok.patch.

[...]

> +############################################
> +#
> +# linux-pam
> +# 
> +############################################
> +
> +LINUX_PAM_VERSION = 1.1.4
> +LINUX_PAM_SOURCE = Linux-PAM-$(LINUX_PAM_VERSION).tar.bz2

Since gz archives are also available, I guess you can drop this line.

> +LINUX_PAM_SITE = http://linux-pam.org/library/
> +LINUX_PAM_INSTALL_STAGING = YES
> +LINUX_PAM_CONF_OPT = \
> +	--disable-prelude \
> +	--disable-isadir \
> +	--disable-nis \
> +	--disable-regenerate-docu \
> +	--enable-securedir=/lib/security \
> +	--libdir=/lib
> +LINUX_PAM_DEPENDENCIES = $(if $(BR2_NEEDS_GETTEXT_IF_LOCALE),gettext libintl) flex

You can drop the libintl here. I know that a lot of packages do that,
but this is useless, since the libintl package doesn't exist.

> +LINUX_PAM_AUTORECONF = YES
> +
> +define LINUX_PAM_BUILD_CMDS
> +	$(MAKE) CC="$(TARGET_CC) -lintl -lfl" LD="$(TARGET_LD)" -C $(@D) all
> +endef

Why do you need to set -lintl -lfl options ?

Thanks,
Maxime

-- 
Maxime Ripard, Free Electrons
Kernel, drivers, real-time and embedded Linux
development, consulting, training and support.
http://free-electrons.com

  reply	other threads:[~2012-08-22  7:09 UTC|newest]

Thread overview: 12+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2012-08-10 10:27 [Buildroot] [PATCH] Add package linux-pam Dmitry
2012-08-22  7:09 ` Maxime Ripard [this message]
2012-08-23  1:53   ` Dmitry Golubovsky
2012-08-23  7:59     ` Maxime Ripard
  -- strict thread matches above, loose matches on Subject: below --
2012-08-11  2:50 Dmitry
2012-08-11 20:16 ` Thomas Petazzoni
2012-08-10  3:32 Dmitry
2012-08-10  7:41 ` Thomas Petazzoni
     [not found]   ` <CADiAo4+Ak_6ZLY0ND=ZYWz6wTsov4XnkpXvbK8ZSF-ukU7ALGw@mail.gmail.com>
2012-08-10 10:51     ` Dmitry Golubovsky
2012-08-10 20:04     ` Thomas Petazzoni
2012-08-11  2:52       ` Dmitry Golubovsky
2012-07-18 21:03 Dmitry

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=503485C0.1000406@free-electrons.com \
    --to=maxime.ripard@free-electrons.com \
    --cc=buildroot@busybox.net \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox