Bug 210947
Summary: | Processed: parameterizing the hardcoded paths (phase 3. installed binaries, change log, setup) | ||||||||||||||||
---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
Product: | [Retired] 389 | Reporter: | Noriko Hosoi <nhosoi> | ||||||||||||||
Component: | Unknown | Assignee: | Noriko Hosoi <nhosoi> | ||||||||||||||
Status: | CLOSED CURRENTRELEASE | QA Contact: | Viktor Ashirov <vashirov> | ||||||||||||||
Severity: | medium | Docs Contact: | |||||||||||||||
Priority: | medium | ||||||||||||||||
Version: | 1.0.2 | CC: | nkinder, rmeggins | ||||||||||||||
Target Milestone: | --- | ||||||||||||||||
Target Release: | --- | ||||||||||||||||
Hardware: | All | ||||||||||||||||
OS: | All | ||||||||||||||||
Whiteboard: | |||||||||||||||||
Fixed In Version: | Doc Type: | Bug Fix | |||||||||||||||
Doc Text: | Story Points: | --- | |||||||||||||||
Clone Of: | Environment: | ||||||||||||||||
Last Closed: | 2015-12-07 16:48:23 UTC | Type: | --- | ||||||||||||||
Regression: | --- | Mount Type: | --- | ||||||||||||||
Documentation: | --- | CRM: | |||||||||||||||
Verified Versions: | Category: | --- | |||||||||||||||
oVirt Team: | --- | RHEL 7.3 requirements from Atomic Host: | |||||||||||||||
Cloudforms Team: | --- | Target Upstream Version: | |||||||||||||||
Embargoed: | |||||||||||||||||
Bug Depends On: | |||||||||||||||||
Bug Blocks: | 152373, 240316, 427409 | ||||||||||||||||
Attachments: |
|
Description
Noriko Hosoi
2006-10-16 17:27:33 UTC
Created attachment 139176 [details]
ldapserver files to be changed
Created attachment 139180 [details]
cvs diffs (ldapserver)
Changes:
0) lots of makefiles are changed to adjust to the new FHS location and new
library names, which are anyway replaced with makefiles autogenerated by
autotools. So, they can be ignored.
1) Since ds_newinst.pl is also generated by autotools, I'd like to ask to make
these changes to ds_newinst.am, too. With this change, ds_newinst program can
get the prefix and branding info.
Index: ldap/admin/src/ds_newinst.pl
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/admin/src/ds_newinst.pl,v
retrieving revision 1.8
diff -t -w -U4 -r1.8 ds_newinst.pl
--- ldap/admin/src/ds_newinst.pl 27 Sep 2006 23:40:50 -0000 1.8
+++ ldap/admin/src/ds_newinst.pl 23 Oct 2006 21:10:44 -0000
@@ -164,8 +164,9 @@
# next, construct a hash table with our arguments
my %cgiargs = ();
+my $brand_ds = "fedora-ds";
# the following items are always required
addAndCheck(\%cgiargs, "sroot", \%table, "General", "ServerRoot");
addAndCheck(\%cgiargs, "servname", \%table, "General", "FullMachineName");
@@ -213,9 +214,14 @@
$cgiargs{user_ldap_url} = $cgiargs{ldap_url};
}
if ($table{General}->{prefix}) {
- $cgiargs{prefix} = $table{General}->{prefix};
+ $prefix = $table{General}->{prefix};
+}
+$cgiargs{prefix} = $prefix;
+
+if ($table{General}->{BrandDs}) {
+ $brand_ds = $table{General}->{BrandDs};
}
# populate the DS with this file - the suffix in this file must
# be the suffix specified in the suffix argument above
@@ -229,9 +235,9 @@
my $sroot = $cgiargs{sroot};
my $rc = &cgiFake($sroot, $verbose,
- $sroot . "/bin/slapd/admin/bin/ds_newinst",
+ $prefix . "/usr/lib/$brand_ds/ds_newinst",
\%cgiargs);
if (!$rc) {
print "Success! Your new directory server instance was created\n";
As you could easily guess from this line:
+$cgiargs{prefix} = $prefix;
you can send these key-value paris:
$cgiargs(config_dir) = ... === dse.ldif is put
$cgiargs(schema_dir) = ... === schema files are put
$cgiargs(lock_dir) = ... === lock file is put
$cgiargs(log_dir) = ... === errors, access, and audit logs are put
$cgiargs(run_dir) = ... === pid file is put
$cgiargs(db_dir) = ... === db dir
$cgiargs(bak_dir) = ... === back up dir
$cgiargs(ldif_dir) = ... === default location db2ldif uses
$cgiargs(tmp_dir) = ... === snmp puts the stat file here
$cgiargs(cert_dir) = ... === key and cert db's are put
2) As for perl task scripts, would it be okay to set PATH and LD_LIBRARY_PATH
like this and run the command? It could call LDAP C SDK tools as well as libdb
tools.
Index: ldap/admin/src/scripts/template-db2bak.pl
===================================================================
RCS file: /cvs/dirsec/ldapserver/ldap/admin/src/scripts/template-db2bak.pl,v
retrieving revision 1.5
diff -t -w -U4 -r1.5 template-db2bak.pl
--- ldap/admin/src/scripts/template-db2bak.pl 13 Oct 2006 01:06:21 -0000
1.5
+++ ldap/admin/src/scripts/template-db2bak.pl 23 Oct 2006 21:10:44 -0000
@@ -114,8 +114,10 @@
$nsdbtype = "nsDatabaseType: $dbtype\n";
$entry = "${dn}${misc}${cn}${nsarchivedir}${nsdbtype}";
$vstr = "";
if ($verbose != 0) { $vstr = "-v"; }
-chdir("$prefix{{SEP}}shared{{SEP}}bin");
-open(FOO, "| $prefix{{SEP}}shared{{SEP}}bin{{SEP}}ldapmodify $vstr -h
{{SERVER-NAME}} -p {{SERVER-PORT}} -D \"$rootdn\" -w \"$passwd\" -a" );
+$ENV{'PATH'} =
'$prefix{{SEP}}usr{{SEP}}lib:{{SEP}}usr{{SEP}}lib{{SEP}}mozldap6';
+$ENV{'LD_LIBRARY_PATH'} =
'$prefix{{SEP}}usr{{SEP}}lib:{{SEP}}usr{{SEP}}lib{{SEP}}dirsec';
+$ENV{'SHLIB_PATH'} =
'$prefix{{SEP}}usr{{SEP}}lib:{{SEP}}usr{{SEP}}lib{{SEP}}dirsec';
+open(FOO, "| ldapmodify $vstr -h {{SERVER-NAME}} -p {{SERVER-PORT}} -D
\"$rootdn\" -w \"$passwd\" -a" );
print(FOO "$entry");
close(FOO);
Nice work! One thing though - the paths set in the perl scripts - we should not hard code in paths like /usr/lib/mozldap6 or /usr/lib/dirsec - these paths should be obtained during configure, like we do in nspr.m4, nss.m4, mozldap.m4, etc. (In reply to comment #6) > Nice work! > > One thing though - the paths set in the perl scripts - we should not hard code > in paths like /usr/lib/mozldap6 or /usr/lib/dirsec - these paths should be > obtained during configure, like we do in nspr.m4, nss.m4, mozldap.m4, etc. Thanks, Rich! Nathan showed me how to do it using ds_newinst.am. I'm following the method and generating the scripts! Hi Nathan, To generate the perl task scripts easier, can I ask you to add these paths to the configure? For now, ldapsdk tools are in /usr/lib/mozldap6, which is independent from -lib or -include. We also need the path for db_verify and db_printlog, which is in /usr/bin by default. --with-ldapsdk-tool=PATH Mozilla LDAP SDK tool directory --with-db-tool=PATH Berkeley DB tool directory And could you please tell me once again how to get the -lib path without "-L"? Thanks!! --noriko To strip the -L off, you can either use sed -e 's/-L//' or use pkg-config --variable=libdir mozldap6 For the db tools, we should probably plan on support for FC5/6 and RHEL5, which use bdb 4.3 by default. The db42 tools are called db_recover42, db_printlog42, etc. We don't have to worry about this now - let's make sure it works on rhel4 first - but we need to keep this in mind. Thanks, Rich! (In reply to comment #9) > To strip the -L off, you can either use sed -e 's/-L//' or use > pkg-config --variable=libdir mozldap6 > > For the db tools, we should probably plan on support for FC5/6 and RHEL5, which > use bdb 4.3 by default. The db42 tools are called db_recover42, db_printlog42, > etc. We don't have to worry about this now - let's make sure it works on rhel4 > first - but we need to keep this in mind. Nathan pointed out we don't have to have a dependency on tools (e.g., mozldap6-tools) at the build time. That's true. But at the same time, we need to have the paths to use them in the task scripts... Passing the tool path to configure regardless of the tools package installation is not a good idea? Too ugly? I've been using the sed approach to remove the "-L". If you go this route, I've noticed that pkg-config will leave some spaces at the end of the output, so you need to strip those off with sed as well (sed -e 's/-L//' | sed -e 's/ *$//'). I just tested the "variable=libdir" approach that Rich mentioned, and it doesn't leave any spaces, so you may want to use that route instead. I've added support to our m4 files to set the libdir for nspr, nss, mozldap, sasl, db, and netsnmp. You should be able to use my work to get these paths. I'll have that bug created shortly, and I'll cross-ref it here. There's not really an easy way to have configure know where the tools are located. Right now, configure doesn't even need the tools (db or mozldap) to be present since they are not a build dependency. Unfortunately, pkg-config is also only designed for providing inforation about installed libraries, not tools, so there doesn't seem to be an easy way to use it to find the location of the tools. We could add support to pkg-config for the bin paths so you could use pkg-config --variable=bindir mozldap6 or something like that. The usual way to do this is to just install everything in /usr/bin and use a prefix or suffix to differentiate among versions e.g. /usr/bin/ldapsearch6 instead of /usr/lib/mozldap6/ldapsearch. But we'd still have to know what the suffix is and/or the version+naming convention. autoconf has a macro that can find executables, given a PATH style list of paths. I suppose we might be able to use those, but the pkgname-tools package is not usually installed at build time. Worst case, we just set the default bin paths in configure, but allow the user to override them at runtime. I have created a bug that deals with setting the component libdir variables so we can substitute them in wrappers and scripts. Details are in bug 212038. (In reply to comment #13) > I have created a bug that deals with setting the component libdir variables so > we can substitute them in wrappers and scripts. Details are in bug 212038. Yep. Saw them when reviewing your code. Thanks!! (In reply to comment #12) > We could add support to pkg-config for the bin paths so you could use > pkg-config --variable=bindir mozldap6 > or something like that. The usual way to do this is to just install everything > in /usr/bin and use a prefix or suffix to differentiate among versions e.g. > /usr/bin/ldapsearch6 instead of /usr/lib/mozldap6/ldapsearch. We (i.e., m4 files and Makefile.am) have the package version info: m4/mozldap.m4: ldapsdk_lib=`$PKG_CONFIG --libs-only-L mozldap6` Makefile.am:LDAPSDK_LINK = @ldapsdk_lib@ -lssldap60 -lldap60 -lprldap60 -lldif60 > But we'd still > have to know what the suffix is and/or the version+naming convention. So, we can say we already know the suffix, can't we? Assuming the ldap c sdk tools accessible as <fixed_path>/<command_line><major#> (e.g., /usr/bin/ldapsearch6) sounds easy and straightforward to me... I think for now we are going to have to just use a default path in configure.ac or Makefile.am e.g. ldapsdk_bindir=/usr/lib/mozldap6 nss_bindir=/usr/bin All right! I'll add them to m4 files in the m4 directory. Is it okay? I think putting them in the m4 files is best. > All right! I'll add them to m4 files in the m4 directory. Is it okay?
Yes, that's fine. I don't think these will change very often.
The diffs look good so far (I still have a bit more to go through). There are a few minor things: - Could we just remove the ntsync plugin stuff from create_instance.c? I know it's ifdef'd, but we don't really need it anymore since our windows sync stuff is now in the replication plugin. - The collation plugin name was changed for the new autoconf build system. It is now "libcollation-plugin", not liblcoll. This should be changed in create_instance.c. Thank you, Nathan! (In reply to comment #20) > The diffs look good so far (I still have a bit more to go through). There are a > few minor things: > > - Could we just remove the ntsync plugin stuff from create_instance.c? I know > it's ifdef'd, but we don't really need it anymore since our windows sync stuff > is now in the replication plugin. All right. Let me completely delete them. These fields are also to be removed, aren't they? conf->ntsynch = "off"; conf->ntsynchssl = "on"; conf->ntsynchport = "5009"; > - The collation plugin name was changed for the new autoconf build system. It > is now "libcollation-plugin", not liblcoll. This should be changed in > create_instance.c. Done: 3652 fprintf(f, "cn: Internationalization Plugin\n"); 3653 fprintf(f, "nsslapd-pluginpath: %s/libcollation-plugin%s\n", cf->plu gin_dir, shared_lib); 3654 fprintf(f, "nsslapd-plugininitfunc: orderingRule_init\n"); Yep, those changes look good. Created attachment 139289 [details] cvs diffs (ldapserver -- revised) Revised diffs for the ldapserver. Additional changes: 1) fixed 2 issues Nathan pointed out in "Comment #20" 2) replaced ldap/admin/src/scripts/template-*.pl with the files generated by autoconf. Regarding the autoconf related changes, I'm opening a new bug and work under the bug#. The new set of diffs looks good. Created attachment 139291 [details]
cvs commit message (ldapserver)
Thank you for the reviews and comments, Nathan and Rich.
Checked in into HEAD.
The check-ins I did yesterday broke the build on HP-UX and Solaris. Fix is attached. Since this is in the "install" makefile, I've just checked in. Sorry about it. --noriko Fixed the HP-UX and Solaris build failure introduced by the previous check-in. CVS: ---------------------------------------------------------------------- CVS: Enter Log. Lines beginning with `CVS:' are removed automatically CVS: CVS: Committing in . CVS: CVS: Modified Files: CVS: Makefile CVS: ---------------------------------------------------------------------- Checking in Makefile; /cvs/dirsec/ldapserver/ldap/cm/Makefile,v <-- Makefile new revision: 1.66; previous revision: 1.65 done Created attachment 140041 [details]
cvs diffs (in files for perl task scripts)
Files:
template-bak2db.pl.in
template-cl-dump.pl.in
template-db2bak.pl.in
template-db2index.pl.in
template-db2ldif.pl.in
template-ldif2db.pl.in
template-ns-accountstatus.pl.in
template-ns-activate.pl.in
template-ns-inactivate.pl.in
template-ns-newpwpolicy.pl.in
template-repl-monitor-cgi.pl.in
template-verify-db.pl.in
Changes:
replaced ' (single quote) with " (double quote) for setting ENV variables.
(In reply to comment #28) > Created an attachment (id=140041) [edit] > cvs diffs (in files for perl task scripts) Problems to solve: On HP-UX, the variables are not set and the clu acceptance test showed the poor result. > Files: > template-bak2db.pl.in > template-cl-dump.pl.in > template-db2bak.pl.in > template-db2index.pl.in > template-db2ldif.pl.in > template-ldif2db.pl.in > template-ns-accountstatus.pl.in > template-ns-activate.pl.in > template-ns-inactivate.pl.in > template-ns-newpwpolicy.pl.in > template-repl-monitor-cgi.pl.in > template-verify-db.pl.in > > Changes: > replaced ' (single quote) with " (double quote) for setting ENV variables. Created attachment 140067 [details]
cvs commit message
Reviewed by Rich. (Thank you!!!)
Checked in into HEAD.
Note: to support TET in the transition period to autotoolized build, I checked
in *.pl files, as well. They are going to be removed from CVS when the
transition is completed.
RHEL4 64 is broken because the bindir is hardcoded to /usr/lib/mozldap6. We should either get bindir from pkg-config or just use $libdir/mozldap6. *** mozldap.m4.~1.4.~ 2006-10-30 16:32:41.000000000 -0700 --- mozldap.m4 2006-11-21 19:09:32.000000000 -0700 *************** *** 80,85 **** --- 80,86 ---- ldapsdk_inc=`$PKG_CONFIG --cflags-only-I mozldap6` ldapsdk_lib=`$PKG_CONFIG --libs-only-L mozldap6` ldapsdk_libdir=`$PKG_CONFIG --libs-only-L mozldap6 | sed -e s/-L// | sed -e s/\ *$//` + ldapsdk_bindir=`$PKG_CONFIG --variable=bindir mozldap6` AC_MSG_RESULT([using system mozldap6]) else AC_MSG_ERROR([LDAPSDK not found, specify with --with-ldapsdk[-inc|-lib].]) *************** *** 90,96 **** AC_MSG_ERROR([LDAPSDK not found, specify with --with-ldapsdk[-inc|-lib].]) fi dnl default path for the ldap c sdk tools (see [210947] for more details) ! ldapsdk_bindir=/usr/lib/mozldap6 dnl make sure the ldap sdk version is 6 or greater - we do not support dnl the old 5.x or prior versions - the ldap server code expects the new --- 91,99 ---- AC_MSG_ERROR([LDAPSDK not found, specify with --with-ldapsdk[-inc|-lib].]) fi dnl default path for the ldap c sdk tools (see [210947] for more details) ! if test -z "$ldapsdk_bindir" ; then ! ldapsdk_bindir=$libdir/mozldap6 ! fi dnl make sure the ldap sdk version is 6 or greater - we do not support dnl the old 5.x or prior versions - the ldap server code expects the new (In reply to comment #31) > RHEL4 64 is broken because the bindir is hardcoded to /usr/lib/mozldap6. We > should either get bindir from pkg-config or just use $libdir/mozldap6. > > *** mozldap.m4.~1.4.~ 2006-10-30 16:32:41.000000000 -0700 > --- mozldap.m4 2006-11-21 19:09:32.000000000 -0700 > *************** > *** 80,85 **** > --- 80,86 ---- Might be a little safer to have this '-o -z "$ldifsdk_bindir"'? # last resort if test -z "$ldapsdk_inc" -o -z "$ldapsdk_lib" -o -z "$ldapsdk_libdir" -o -z "$ldifsdk_bindir"; then > ldapsdk_inc=`$PKG_CONFIG --cflags-only-I mozldap6` > ldapsdk_lib=`$PKG_CONFIG --libs-only-L mozldap6` > ldapsdk_libdir=`$PKG_CONFIG --libs-only-L mozldap6 | sed -e s/-L// | sed > -e s/\ *$//` > + ldapsdk_bindir=`$PKG_CONFIG --variable=bindir mozldap6` > AC_MSG_RESULT([using system mozldap6]) > else > AC_MSG_ERROR([LDAPSDK not found, specify with --with-ldapsdk[-inc|-lib].]) The rest looks good. Thank you for fixing this bug. ( hardcoded paths are so evil... ): Fix Description: RHEL4 64 is not able to find ldapsearch because the ldapsdk_bindir is hardcoded to /usr/lib/mozldap6. We should get ldapsdk_bindir from pkg-config or just simply use $libdir/mozldap6. Added -o -z "$ldapsdk_bindir" check suggested by nhosoi Reviewed by: nhosoi (Thanks!) Checking in ldapserver/configure; /cvs/dirsec/ldapserver/configure,v <-- configure new revision: 1.15; previous revision: 1.14 done Checking in ldapserver/m4/mozldap.m4; /cvs/dirsec/ldapserver/m4/mozldap.m4,v <-- mozldap.m4 new revision: 1.5; previous revision: 1.4 done |