* [Buildroot] Move openldap package and enable LDAP server? @ 2015-12-04 8:13 Andreas Ehmanns 2015-12-04 8:19 ` Thomas Petazzoni 0 siblings, 1 reply; 7+ messages in thread From: Andreas Ehmanns @ 2015-12-04 8:13 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] Move openldap package and enable LDAP server? 2015-12-04 8:13 [Buildroot] Move openldap package and enable LDAP server? Andreas Ehmanns @ 2015-12-04 8:19 ` Thomas Petazzoni 2015-12-04 8:21 ` Andreas Ehmanns 0 siblings, 1 reply; 7+ messages in thread From: Thomas Petazzoni @ 2015-12-04 8:19 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] Move openldap package and enable LDAP server? 2015-12-04 8:19 ` Thomas Petazzoni @ 2015-12-04 8:21 ` Andreas Ehmanns 2015-12-10 19:03 ` Andreas Ehmanns 0 siblings, 1 reply; 7+ messages in thread From: Andreas Ehmanns @ 2015-12-04 8:21 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] Move openldap package and enable LDAP server? 2015-12-04 8:21 ` Andreas Ehmanns @ 2015-12-10 19:03 ` Andreas Ehmanns 2015-12-12 13:01 ` Thomas Petazzoni 0 siblings, 1 reply; 7+ messages in thread From: Andreas Ehmanns @ 2015-12-10 19:03 UTC (permalink / raw) To: buildroot 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> ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] Move openldap package and enable LDAP server? 2015-12-10 19:03 ` Andreas Ehmanns @ 2015-12-12 13:01 ` Thomas Petazzoni 2015-12-12 17:20 ` Andreas Ehmanns 2015-12-17 20:43 ` Andreas Ehmanns 0 siblings, 2 replies; 7+ messages in thread From: Thomas Petazzoni @ 2015-12-12 13:01 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] Move openldap package and enable LDAP server? 2015-12-12 13:01 ` Thomas Petazzoni @ 2015-12-12 17:20 ` Andreas Ehmanns 2015-12-17 20:43 ` Andreas Ehmanns 1 sibling, 0 replies; 7+ messages in thread From: Andreas Ehmanns @ 2015-12-12 17:20 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
* [Buildroot] Move openldap package and enable LDAP server? 2015-12-12 13:01 ` Thomas Petazzoni 2015-12-12 17:20 ` Andreas Ehmanns @ 2015-12-17 20:43 ` Andreas Ehmanns 1 sibling, 0 replies; 7+ messages in thread From: Andreas Ehmanns @ 2015-12-17 20:43 UTC (permalink / raw) To: buildroot 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 ^ permalink raw reply [flat|nested] 7+ messages in thread
end of thread, other threads:[~2015-12-17 20:43 UTC | newest] Thread overview: 7+ messages (download: mbox.gz / follow: Atom feed) -- links below jump to the message on this page -- 2015-12-04 8:13 [Buildroot] Move openldap package and enable LDAP server? Andreas Ehmanns 2015-12-04 8:19 ` Thomas Petazzoni 2015-12-04 8:21 ` Andreas Ehmanns 2015-12-10 19:03 ` Andreas Ehmanns 2015-12-12 13:01 ` Thomas Petazzoni 2015-12-12 17:20 ` Andreas Ehmanns 2015-12-17 20:43 ` Andreas Ehmanns
This is an external index of several public inboxes, see mirroring instructions on how to clone and mirror all data and code used by this external index.