[Buildroot] Move openldap package and enable LDAP server?

[Buildroot] Move openldap package and enable LDAP server?

[Andreas Ehmanns]

Dear all,
I was going to use openLDAP on my embedded device and found the 
configuration under Target Packages -> Libraries -> Networking.
You can enable openldap which only generates the ldap libraries but have 
an option to let the client applications being built.
Since I need the LDAP server I added an option to compile the LDAP 
server too. Everything works fine and I thought about submitting the 
patch but then I asked me if the configuration location is still the 
right place.
Wouldn't it be better to place the openldap configuration under
Target Packages -> Networking Applications
instead of
Target Packages -> Libraries -> Networking

What is your opinion?

Regards,
Andreas Ehmanns

[Buildroot] Move openldap package and enable LDAP server?

[Thomas Petazzoni]

Hello,

On Fri, 4 Dec 2015 09:13:12 +0100, Andreas Ehmanns wrote:

> I was going to use openLDAP on my embedded device and found the 
> configuration under Target Packages -> Libraries -> Networking.
> You can enable openldap which only generates the ldap libraries but have 
> an option to let the client applications being built.
> Since I need the LDAP server I added an option to compile the LDAP 
> server too. Everything works fine and I thought about submitting the 
> patch but then I asked me if the configuration location is still the 
> right place.
> Wouldn't it be better to place the openldap configuration under
> Target Packages -> Networking Applications
> instead of
> Target Packages -> Libraries -> Networking
> 
> What is your opinion?

Yes, I agree that it probably makes more sense to have OpenLDAP in
Networking Applications, even if it indeed also provides a client
library.

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] Move openldap package and enable LDAP server?

[Andreas Ehmanns]

Am 04.12.2015 um 09:19 schrieb Thomas Petazzoni:
> Hello,
>
> On Fri, 4 Dec 2015 09:13:12 +0100, Andreas Ehmanns wrote:
>
>> I was going to use openLDAP on my embedded device and found the
>> configuration under Target Packages -> Libraries -> Networking.
>> You can enable openldap which only generates the ldap libraries but have
>> an option to let the client applications being built.
>> Since I need the LDAP server I added an option to compile the LDAP
>> server too. Everything works fine and I thought about submitting the
>> patch but then I asked me if the configuration location is still the
>> right place.
>> Wouldn't it be better to place the openldap configuration under
>> Target Packages -> Networking Applications
>> instead of
>> Target Packages -> Libraries -> Networking
>>
>> What is your opinion?
> Yes, I agree that it probably makes more sense to have OpenLDAP in
> Networking Applications, even if it indeed also provides a client
> library.
>
> Thomas
O.k., I will prepare/propose a patch for adding the ldap server option 
and moving the configuration to Networking Applications.

Regards,
Andreas

[Buildroot] Move openldap package and enable LDAP server?

[Andreas Ehmanns]

Am 04.12.2015 um 09:21 schrieb Andreas Ehmanns:
> Am 04.12.2015 um 09:19 schrieb Thomas Petazzoni:
>> Hello,
>>
>> On Fri, 4 Dec 2015 09:13:12 +0100, Andreas Ehmanns wrote:
>>
>>> I was going to use openLDAP on my embedded device and found the
>>> configuration under Target Packages -> Libraries -> Networking.
>>> You can enable openldap which only generates the ldap libraries but 
>>> have
>>> an option to let the client applications being built.
>>> Since I need the LDAP server I added an option to compile the LDAP
>>> server too. Everything works fine and I thought about submitting the
>>> patch but then I asked me if the configuration location is still the
>>> right place.
>>> Wouldn't it be better to place the openldap configuration under
>>> Target Packages -> Networking Applications
>>> instead of
>>> Target Packages -> Libraries -> Networking
>>>
>>> What is your opinion?
>> Yes, I agree that it probably makes more sense to have OpenLDAP in
>> Networking Applications, even if it indeed also provides a client
>> library.
>>
>> Thomas
> O.k., I will prepare/propose a patch for adding the ldap server option 
> and moving the configuration to Networking Applications.
>
> Regards,
> Andreas

Thomas,
please find attached the patch for adding the openldap server. It 
contains the server option, moves the configuration to "Networking 
Applications" and adds an init script for the ldap server.

Please let me know what you think about the patch and if there are 
things that I should change.

Regards,
Andreas

> _______________________________________________
> buildroot mailing list
> buildroot at busybox.net
> http://lists.busybox.net/mailman/listinfo/buildroot

-------------- next part --------------
A non-text attachment was scrubbed...
Name: 0002-Added-server-to-openldap-package-and-changed-package.patch
Type: text/x-patch
Size: 4722 bytes
Desc: not available
URL: <http://lists.busybox.net/pipermail/buildroot/attachments/20151210/939c16b9/attachment.bin>

[Buildroot] Move openldap package and enable LDAP server?

[Thomas Petazzoni]

Hello Andreas,

On Thu, 10 Dec 2015 20:03:46 +0100, Andreas Ehmanns wrote:

> please find attached the patch for adding the openldap server. It 
> contains the server option, moves the configuration to "Networking 
> Applications" and adds an init script for the ldap server.
> 
> Please let me know what you think about the patch and if there are 
> things that I should change.

Thanks. But could you please send the patch with 'git send-email'
rather than as an attachment? It allows the patch to be reviewed
properly, and also easily applied.

A few comments though:

 * The commit title is too long. Something like:

	openldap: add support to build the server

   would be preferable.

 * I think you should keep the "default y" on
   BR2_PACKAGE_OPENLDAP_CLIENTS in order to preserve the existing
   behavior, and not breaking things for current users.

 * In the S75sldapd script, I believe checking for the presence of the
   daemon and config file is not really needed.

   If the -4 in ARGS is to force IPv4 only, I believe it's not really
   great. Why not also support IPv6 ?

 * There is a fix of tabs and spaces for the indentation in the shell
   script, please try to be consistent.

 * Please use "printf" rather than "echo -n" in the shell script, since
   printf is POSIX, while "echo -n" is not.

 * We normally use a PID file, see S50dropbear for example. Is slapd
   already managing its own PID file ?

 * In the .mk file, the OPENLDAP_USERS definition is inside a condition
   that is commented out, it doesn't look good.

 * Don't do ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),n), but just ifeq
   ($(BR2_PACKAGE_OPENLDAP_SERVER),). An option will never have the
   value 'n': when an option is disabled, its value is the empty string.

Could you take those comments into account and send an updated version?

Thanks a lot!

Thomas
-- 
Thomas Petazzoni, CTO, Free Electrons
Embedded Linux, Kernel and Android engineering
http://free-electrons.com

[Buildroot] Move openldap package and enable LDAP server?

[Andreas Ehmanns]

Hi Thomas,

Am 12.12.2015 um 14:01 schrieb Thomas Petazzoni:
> Hello Andreas,
>
> On Thu, 10 Dec 2015 20:03:46 +0100, Andreas Ehmanns wrote:
>
>> please find attached the patch for adding the openldap server. It
>> contains the server option, moves the configuration to "Networking
>> Applications" and adds an init script for the ldap server.
>>
>> Please let me know what you think about the patch and if there are
>> things that I should change.
> Thanks. But could you please send the patch with 'git send-email'
> rather than as an attachment? It allows the patch to be reviewed
> properly, and also easily applied.
>
> A few comments though:
>
>   * The commit title is too long. Something like:
>
> 	openldap: add support to build the server
>
>     would be preferable.
>
>   * I think you should keep the "default y" on
>     BR2_PACKAGE_OPENLDAP_CLIENTS in order to preserve the existing
>     behavior, and not breaking things for current users.
>
>   * In the S75sldapd script, I believe checking for the presence of the
>     daemon and config file is not really needed.
>
>     If the -4 in ARGS is to force IPv4 only, I believe it's not really
>     great. Why not also support IPv6 ?
>
>   * There is a fix of tabs and spaces for the indentation in the shell
>     script, please try to be consistent.
>
>   * Please use "printf" rather than "echo -n" in the shell script, since
>     printf is POSIX, while "echo -n" is not.
>
>   * We normally use a PID file, see S50dropbear for example. Is slapd
>     already managing its own PID file ?
>
>   * In the .mk file, the OPENLDAP_USERS definition is inside a condition
>     that is commented out, it doesn't look good.
>
>   * Don't do ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),n), but just ifeq
>     ($(BR2_PACKAGE_OPENLDAP_SERVER),). An option will never have the
>     value 'n': when an option is disabled, its value is the empty string.
>
> Could you take those comments into account and send an updated version?
>
> Thanks a lot!
>
> Thomas
thanks for your comments. I will incorporate the changes next week and 
send a patch using git.

Regards,
Andreas

[Buildroot] Move openldap package and enable LDAP server?

[Andreas Ehmanns]

Thomas, All,

Am 12.12.2015 um 14:01 schrieb Thomas Petazzoni:
> Hello Andreas,
>
> On Thu, 10 Dec 2015 20:03:46 +0100, Andreas Ehmanns wrote:
>
>> please find attached the patch for adding the openldap server. It
>> contains the server option, moves the configuration to "Networking
>> Applications" and adds an init script for the ldap server.
>>
>> Please let me know what you think about the patch and if there are
>> things that I should change.
> Thanks. But could you please send the patch with 'git send-email'
> rather than as an attachment? It allows the patch to be reviewed
> properly, and also easily applied.
>
> A few comments though:
>
>   * The commit title is too long. Something like:
>
> 	openldap: add support to build the server
>
>     would be preferable.
>
>   * I think you should keep the "default y" on
>     BR2_PACKAGE_OPENLDAP_CLIENTS in order to preserve the existing
>     behavior, and not breaking things for current users.
>
>   * In the S75sldapd script, I believe checking for the presence of the
>     daemon and config file is not really needed.
>
>     If the -4 in ARGS is to force IPv4 only, I believe it's not really
>     great. Why not also support IPv6 ?
>
>   * There is a fix of tabs and spaces for the indentation in the shell
>     script, please try to be consistent.
>
>   * Please use "printf" rather than "echo -n" in the shell script, since
>     printf is POSIX, while "echo -n" is not.
>
>   * We normally use a PID file, see S50dropbear for example. Is slapd
>     already managing its own PID file ?
>
>   * In the .mk file, the OPENLDAP_USERS definition is inside a condition
>     that is commented out, it doesn't look good.
>
>   * Don't do ifeq ($(BR2_PACKAGE_OPENLDAP_SERVER),n), but just ifeq
>     ($(BR2_PACKAGE_OPENLDAP_SERVER),). An option will never have the
>     value 'n': when an option is disabled, its value is the empty string.
>
> Could you take those comments into account and send an updated version?
>
> Thanks a lot!
>
> Thomas
I incorporated your proposed changes and re-sent the patch using git.
Note: slapd is managing it's own PID file. So there was no need to add 
this to the init script.

Regards,
Andreas