389-ds-base

Clone
Source Code
GIT

#48797 RFE: Support 389-ds on FreeBSD 10.2-RELEASE
Closed: wontfix None Opened 8 years ago by firstyear.

Closed: wontfix
Reopen Issue

This ticket is to add experimental support for FreeBSD to ns-slapd.

Notably, this includes fixes in portability for freebsd, corrections for kerberos library and header detection and man others.

These changes have also been tested to have no impact on the linux builds of 389.

This support has received only basic testing at this time. Known issues are the perl scripts do not work.

Try to use variable generated by libtool for "ldl" instead of custom condition specific for different platform.
{{{
80
81 if FREEBSD
82 DLOPEN_LINK =
83 else
80 84 DLOPEN_LINK = -ldl
85 endif
86
}}}
Libtool will create variable LIBADD_DL which you can use for the same purpose. If you do not have this variable in Makefile then I might take a look which option enable it. LIBADD_DL will work on any platform.

Are you sure that following change is safe?
{{{
21 #ifdef FreeBSD
22 #define ENODATA 96
23 #endif
}}}
FreeBSD might introduce different error code with the same value in future.
Is the error code ENODATA returned by external function(glibc, ...) or is it used only by internal code. In the 1st case, you might check which error code
is returned on FreeBSD. In the 2nd case, it would be better to use internal error code with different base
(96 + ERROR_BASE_389(0x03890000))

Did you test 389 with FreeBSD default kerberos implementation? It's based on older version of heimdal.
If no then much safer will be to test with heimal from ports which is more recent or even better with MIT kerberos from ports.

Following code is not very portable
{{{
175
176 #if defined(FreeBSD)
177 #include <sys/time.h>
178 #endif
179
}}}
or
{{{
43
44 #ifndef FreeBSD
43 45 #include <sys/sysinfo.h>
46 #endif
47
}}}
The usual way is to detect specific header files at configure time and if generated macros "HAVE_SYS_SYSINFO_H". You will safe some time when porting to other platforms (*BSD ...)

It would be good to separate changes into more commits with verbose explanation in commit message.
e.g.
{{{
45 } critical_t;
45 } ns_critical_t;
}}}
I assume that critical_t was already defined in FreeBSD

ACK to change related to malloc.h. Please do it in separate commit.

Setting the target milestone to FUTURE.

William, whenever you complete the task, please reset it to the next version at the time.

Thanks!

Hi, here is the patch and a set of broken out patches for review.

I am so sorry for late review. Initaillly I was bussy with other stuff and then I little bit forgot.

0002-Ticket-48797-Add-freebsd-support-to-ns-slapd-Configu.patch

Pleasee use macro LT_LIB_DLLOAD instead of LT_INIT
The result is the same and ''autoreconf -if'' generates less data.

0003-Ticket-48797-Add-freebsd-support-to-ns-slapd-Header-.patch
I am not sure about changes in ldap/servers/slapd/back-ldbm/back-ldbm.h

I would put change ldap/servers/slapd/tools/ldclt/ldclt.h into separate patch with explanation that
''sys/time.h'' is a header file which defines ''struct timeval''.

Otherwise 3rd patch LGTM.

0004-Ticket-48797-Add-freebsd-support-to-ns-slapd-Add-sup.patch
ACK

0006-Ticket-48797-Add-freebsd-support-to-ns-slapd-Add-fre.patch
ACK, but I would like to ask mhy malloc.h is handled differently in 7th patch. IMHO, it should be fixed in the 6th patch and in the same way and not with.
{{{
--- a/ldap/servers/slapd/main.c
+++ b/ldap/servers/slapd/main.c
@@ -11,7 +11,11 @@
# include <config.h>
#endif

+#ifdef FreeBSD
+#include <stdlib.h>
+#else
#include <malloc.h>
+#endif
}}}

BTW there is conditional include of header file "sys/sysinfo.h" in 7th patch. But ''sysinfo'' is used only in ''ldap/servers/slapd/main.c'' and ''ldap/systools/idsktune.c''. It can be removed from:
rsearch/infadd.c
 rsearch/rsearch.c
uniqueidgen.c
 uuid.c
and the sam applies to ''sys/utsname.h''. Function ''uname'' and ''struct utsname'' are used only in main.c and idsktune.c but header file is also included in:
uniqueidgen.c
 uuid.c

I will review rest later. Next review will be much much faster :-)

Note: I used following arguments for configure script on FreeBSD
{{{
./configure --with-openldap --with-openldap-inc=/usr/local/include --with-openldap-lib=/usr/local/lib --with-openldap-bin=/usr/local/bin --with-db-inc=/usr/local/include/db5/ --with-db-lib=/usr/local/lib --with-sasl-inc=/usr/local/include/sasl --with-sasl-lib=/usr/local/lib --with-netsnmp-inc=/usr/local/include/ --with-netsnmp-lib=/usr/local/lib/
}}}

Replying to [comment:5 lslebodn]:

I am so sorry for late review. Initaillly I was bussy with other stuff and then I little bit forgot.

No problem. Thanks for getting back to it. I had some trouble with the rebase, but all sorted now.

Put the patches into a tarball, just easier to download for you and upload for me.

0002-Ticket-48797-Add-freebsd-support-to-ns-slapd-Configu.patch

Pleasee use macro LT_LIB_DLLOAD instead of LT_INIT
The result is the same and ''autoreconf -if'' generates less data.

Done.

0003-Ticket-48797-Add-freebsd-support-to-ns-slapd-Header-.patch
I am not sure about changes in ldap/servers/slapd/back-ldbm/back-ldbm.h

We previously used an errno code that wasn't in freebds. But we didn't actually check the result, and that fn is only called in one place, so defining our own error code here is not a problem given the highly private nature of the fn.

I would put change ldap/servers/slapd/tools/ldclt/ldclt.h into separate patch with explanation that
''sys/time.h'' is a header file which defines ''struct timeval''.

I'm going to actually squash these all back into one afterwards, so the split would only be for review purposes. I don't think it's worth the effort.

Otherwise 3rd patch LGTM.

0004-Ticket-48797-Add-freebsd-support-to-ns-slapd-Add-sup.patch
ACK

0006-Ticket-48797-Add-freebsd-support-to-ns-slapd-Add-fre.patch
ACK, but I would like to ask mhy malloc.h is handled differently in 7th patch. IMHO, it should be fixed in the 6th patch and in the same way and not with.

I think I did it this way, and realised I could just change to stdlib. Thanks for noticing the mistake, fixed.

BTW there is conditional include of header file "sys/sysinfo.h" in 7th patch. But ''sysinfo'' is used only in ''ldap/servers/slapd/main.c'' and ''ldap/systools/idsktune.c''. It can be removed from:
rsearch/infadd.c
 rsearch/rsearch.c
uniqueidgen.c
 uuid.c
and the sam applies to ''sys/utsname.h''. Function ''uname'' and ''struct utsname'' are used only in main.c and idsktune.c but header file is also included in:
uniqueidgen.c
 uuid.c

I think I would like to leave these for now, I'll remove them in a separate cleanup ticket. There are a lot of changes here already, so I don't want to cause unnecessary issues if I can avoid.

I will review rest later. Next review will be much much faster :-)

Thanks heaps for this review already. It's already starting to improve a lot. I think there is a single function in there which for Freebsd just has a comment telling me to implement the content, so I should probably do that soon. (It's for disk space checking).

I've managed to build and install this with the new python installer now. Had some issues with the unit tests, but manged to get:

{{{
INFO:LogCapture.SetupDs:Running setup with verbose
INFO:LogCapture.SetupDs:READY: preparing installation
INFO:LogCapture.SetupDs:PASSED: using config settings 99999
INFO:LogCapture.SetupDs:PASSED: user / group checking
INFO:LogCapture.SetupDs:PASSED: Hostname strict checking
INFO:LogCapture.SetupDs:PASSED: prefix checking
INFO:lib389:dir (sys) : /opt/dirsrv/etc/sysconfig
INFO:lib389:dir (priv): /root/.dirsrv
INFO:LogCapture.SetupDs:PASSED: instance checking
INFO:LogCapture.SetupDs:PASSED: root user checking
INFO:LogCapture.SetupDs:PASSED: network avaliability checking
INFO:LogCapture.SetupDs:READY: beginning installation
INFO:LogCapture.SetupDs:ACTION: creating /opt/dirsrv/var/lib/dirsrv/slapd-standalone/bak
INFO:LogCapture.SetupDs:ACTION: creating /opt/dirsrv/etc/dirsrv/slapd-standalone
INFO:LogCapture.SetupDs:ACTION: creating /opt/dirsrv/etc/dirsrv/slapd-standalone
INFO:LogCapture.SetupDs:ACTION: creating /opt/dirsrv/var/lib/dirsrv/slapd-standalone/db
INFO:LogCapture.SetupDs:ACTION: creating /opt/dirsrv/var/lib/dirsrv/slapd-standalone/ldif
INFO:LogCapture.SetupDs:ACTION: creating /opt/dirsrv/var/lock/dirsrv/slapd-standalone
INFO:LogCapture.SetupDs:ACTION: creating /opt/dirsrv/var/log/dirsrv/slapd-standalone
INFO:LogCapture.SetupDs:ACTION: creating /opt/dirsrv/var/run/dirsrv
INFO:LogCapture.SetupDs:ACTION: Creating certificate database is /opt/dirsrv/etc/dirsrv/slapd-standalone
INFO:LogCapture.SetupDs:ACTION: Creating dse.ldif
INFO:lib389:Allocate <class 'lib389.DirSrv'> with localhost:54321
INFO:lib389:Allocate <class 'lib389.DirSrv'> with localhost:54321
INFO:lib389:dir (sys) : /opt/dirsrv/etc/sysconfig
INFO:lib389:dir (priv): /root/.dirsrv
INFO:lib389:List from /root/.dirsrv
INFO:lib389:list instance {'server-id': 'standalone', 'SERVER_ID': 'standalone', 'PRODUCT_NAME': 'slapd', 'ldapi_enabled': None, 'INST_DIR': '/opt/dirsrv/var/lib/dirsrv/slapd-standalone', 'SERVERBIN_DIR': '/opt/dirsrv/sbin', 'ldap-port': 54321, 'deployed-dir': '/opt/dirsrv', 'root-dn': b'cn=Directory Manager', 'suffix': None, 'user-id': b'dirsrv', 'ldapi_autobind': None, 'SERVER_DIR': '/opt/dirsrv/usr/lib64/dirsrv', 'RUN_DIR': '/opt/dirsrv/var/run/dirsrv', 'ldap-secureport': None, 'DS_ROOT': '', 'hostname': b'fds11b3', 'CONFIG_DIR': '/opt/dirsrv/etc/dirsrv/slapd-standalone', 'ldapi_socket': None}

DEBUG:lib389.tools:Starting server <lib389.DirSrv object at 0x806b03a20>
INFO:lib389:dir (sys) : /opt/dirsrv/etc/sysconfig
INFO:lib389:dir (priv): /root/.dirsrv
INFO:lib389:List from /root/.dirsrv
INFO:lib389:list instance {'server-id': 'standalone', 'SERVER_ID': 'standalone', 'PRODUCT_NAME': 'slapd', 'ldapi_enabled': None, 'INST_DIR': '/opt/dirsrv/var/lib/dirsrv/slapd-standalone', 'SERVERBIN_DIR': '/opt/dirsrv/sbin', 'ldap-port': 54321, 'deployed-dir': '/opt/dirsrv', 'root-dn': b'cn=Directory Manager', 'suffix': None, 'user-id': b'dirsrv', 'ldapi_autobind': None, 'SERVER_DIR': '/opt/dirsrv/usr/lib64/dirsrv', 'RUN_DIR': '/opt/dirsrv/var/run/dirsrv', 'ldap-secureport': None, 'DS_ROOT': '', 'hostname': b'fds11b3', 'CONFIG_DIR': '/opt/dirsrv/etc/dirsrv/slapd-standalone', 'ldapi_socket': None}

INFO:lib389.tools:Setup error log
WARNING:lib389.tools:Running command: '/opt/dirsrv/sbin/start-dirsrv standalone' - timeout(60)

fds11b3% ldapsearch -H ldap://localhost:54321 -x -b '' -s base +

extended LDIF

LDAPv3

base <> with scope baseObject

filter: (objectclass=*)

requesting: +

dn:
creatorsName: cn=server,cn=plugins,cn=config
modifiersName: cn=server,cn=plugins,cn=config
createTimestamp: 20160905043038Z
modifyTimestamp: 20160905043038Z
subschemaSubentry: cn=schema
supportedExtension: 2.16.840.1.113730.3.5.7
supportedExtension: 2.16.840.1.113730.3.5.8
supportedExtension: 1.3.6.1.4.1.4203.1.11.3
supportedExtension: 1.3.6.1.4.1.4203.1.11.1
supportedControl: 2.16.840.1.113730.3.4.2
supportedControl: 2.16.840.1.113730.3.4.3
supportedControl: 2.16.840.1.113730.3.4.4
supportedControl: 2.16.840.1.113730.3.4.5
supportedControl: 1.2.840.113556.1.4.473
supportedControl: 2.16.840.1.113730.3.4.9
supportedControl: 2.16.840.1.113730.3.4.16
supportedControl: 2.16.840.1.113730.3.4.15
supportedControl: 2.16.840.1.113730.3.4.17
supportedControl: 2.16.840.1.113730.3.4.19
supportedControl: 1.3.6.1.1.13.1
supportedControl: 1.3.6.1.1.13.2
supportedControl: 1.3.6.1.4.1.42.2.27.8.5.1
supportedControl: 1.3.6.1.4.1.42.2.27.9.5.2
supportedControl: 1.2.840.113556.1.4.319
supportedControl: 1.3.6.1.4.1.42.2.27.9.5.8
supportedControl: 1.3.6.1.4.1.4203.666.5.16
supportedControl: 2.16.840.1.113730.3.4.14
supportedControl: 2.16.840.1.113730.3.4.20
supportedControl: 1.3.6.1.4.1.1466.29539.12
supportedControl: 2.16.840.1.113730.3.4.12
supportedControl: 2.16.840.1.113730.3.4.18
supportedFeatures: 1.3.6.1.4.1.4203.1.5.1
supportedSASLMechanisms: EXTERNAL
supportedLDAPVersion: 2
supportedLDAPVersion: 3
vendorName: 389 Project
vendorVersion: 389-Directory/1.3.6.0.20160905git64468e3 B2016.249.420

}}}

84728d52cd637245adfe19bdc83e2371 0002-Ticket-48797-Add-freebsd-support-to-ns-slapd-Configu.patch
ACK

9d57d52bfe3642193dbc212553ae4c80 0004-Ticket-48797-Add-freebsd-support-to-ns-slapd-Add-sup.patch
ACK

cbba88283b2647bf81f39da2d26b31b9 0005-Ticket-48797-Add-freebsd-support-to-ns-slapd-Add-sup.patch
ACK

56a9594fa7568fd5b2506869b2c59fc4 0006-Ticket-48797-Add-freebsd-support-to-ns-slapd-Add-fre.patch
ACK

f6c5e4611baed0277b85fa4bbccb8872 0003-Ticket-48797-Add-freebsd-support-to-ns-slapd-Header-.patch
Conditional ACK if you remove E_NOENTRYRDN from the patch

{{{
diff --git a/ldap/servers/slapd/back-ldbm/back-ldbm.h b/ldap/servers/slapd/back-ldbm/back-ldbm.h
index f18db1b..ecef956 100644
--- a/ldap/servers/slapd/back-ldbm/back-ldbm.h
+++ b/ldap/servers/slapd/back-ldbm/back-ldbm.h
@@ -16,10 +16,6 @@
#ifndef BACK_LDBM_H
#define BACK_LDBM_H

-/ This is used in _entryrdn_open_index only, and not checked /
-/ So it's not a problem to make our own! /
-#define E_NOENTRYRDN -2
-
#if defined(HPUX11) || defined(OS_solaris) || defined(linux)
/ built-in 64-bit file I/O support /
#define DB_USE_64LFS
}}}

BTW you cannot replace malloc.h in ldap/servers/slapd/main.c with stdlib.h because the mallopt function is used on linux.

It would be good if you could push ACK-ed patches and we can improve rest later

commit 251424e
commit 9789753
commit b7cda0d
commit 2d53f14
commit eb4c740
Writing objects: 100% (46/46), 6.66 KiB | 0 bytes/s, done.
Total 46 (delta 36), reused 2 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
b1f434e..251424e master -> master

correction of an ldbm error code.
{{{
1604 rc = ENODATA;
1604 rc = -2;
}}}
First, I thought this code is trying to fix some logic error, and I searched the code why the particular value -2 is needed there... But looking at the following comment, it seems it's just for eliminating ENODATA?
{{{
-/ This is used in _entryrdn_open_index only, and not checked /
-/ So it's not a problem to make our own! /
-#define E_NOENTRYRDN -2
}}}
If so, I'd prefer not to set -2, but leave it with -1 (the default value of rc in _entryrdn_open_index) and add some error log message there. Thanks.

Updated patch based on Noriko's comments.
0001-Ticket-48797-Add-freebsd-support-to-ns-slapd-main.2.patch

Thanks Noriko, I have updated the patch with your feedback!

Thank you for taking care my request, William!

Note: Resetting the milestone to 1.3.6...

commit 4026046b743c65a204ecb0fe21c8a940ed65b52e
Writing objects: 100% (19/19), 2.85 KiB | 0 bytes/s, done.
Total 19 (delta 17), reused 0 (delta 0)
To ssh://git.fedorahosted.org/git/389/ds.git
98c88d0..4ce95a7 master -> master

I've closed this issue now as the major port work is done. From this point it will be minor platform specific issues which I will open individual tickets for as they arise. Thanks.

Metadata Update from @nhosoi:
- Issue assigned to firstyear
- Issue set to the milestone: 1.3.6.0
7 years ago

389-ds-base is moving from Pagure to Github. This means that new issues and pull requests
will be accepted only in 389-ds-base's github repository.

This issue has been cloned to Github and is available here:
- https://github.com/389ds/389-ds-base/issues/1857

If you want to receive further updates on the issue, please navigate to the github issue
and click on subscribe button.

Thank you for understanding. We apologize for all inconvenience.

Metadata Update from @spichugi:
- Issue close_status updated to: wontfix (was: Fixed)
3 years ago

Login to comment on this ticket.

Metadata
None
None
None
major
ack
None
Community
Powered by Pagure 5.13.3
Documentation • File an Issue • About • SSH Hostkey/Fingerprint
© Red Hat, Inc. and others.