Bug 210947

Summary: Processed: parameterizing the hardcoded paths (phase 3. installed binaries, change log, setup)
Product: [Retired] 389 Reporter: Noriko Hosoi <nhosoi>
Component: UnknownAssignee: Noriko Hosoi <nhosoi>
Status: CLOSED CURRENTRELEASE QA Contact: Viktor Ashirov <vashirov>
Severity: medium Docs Contact:
Priority: medium    
Version: 1.0.2CC: 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 Flags
ldapserver files to be changed
none
cvs diffs (ldapserver)
none
cvs diffs (ldapserver -- revised)
none
cvs commit message (ldapserver)
none
cvs diffs (in files for perl task scripts)
none
cvs commit message none

Description Noriko Hosoi 2006-10-16 17:27:33 UTC
Description of problem:
Phase 3 of FHS support.
This phase should cover all the leftovers.

Comment 1 Noriko Hosoi 2006-10-23 22:38:04 UTC
Created attachment 139176 [details]
ldapserver files to be changed

Comment 2 Noriko Hosoi 2006-10-23 23:02:08 UTC
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);

Comment 6 Rich Megginson 2006-10-24 02:50:45 UTC
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.

Comment 7 Noriko Hosoi 2006-10-24 06:27:00 UTC
(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!

Comment 8 Noriko Hosoi 2006-10-24 18:06:09 UTC
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

Comment 9 Rich Megginson 2006-10-24 18:10:17 UTC
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.

Comment 10 Noriko Hosoi 2006-10-24 18:20:39 UTC
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?

Comment 11 Nathan Kinder 2006-10-24 18:23:16 UTC
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.

Comment 12 Rich Megginson 2006-10-24 18:45:26 UTC
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.

Comment 13 Nathan Kinder 2006-10-24 18:57:06 UTC
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.

Comment 14 Noriko Hosoi 2006-10-24 19:00:45 UTC
(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!!

Comment 15 Noriko Hosoi 2006-10-24 20:58:52 UTC
(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...  


Comment 16 Rich Megginson 2006-10-24 21:25:40 UTC
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

Comment 17 Noriko Hosoi 2006-10-24 21:28:29 UTC
All right!  I'll add them to m4 files in the m4 directory.  Is it okay?

Comment 18 Nathan Kinder 2006-10-24 21:36:56 UTC
I think putting them in the m4 files is best.

Comment 19 Rich Megginson 2006-10-24 21:38:27 UTC
> 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.

Comment 20 Nathan Kinder 2006-10-24 22:16:19 UTC
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.

Comment 21 Noriko Hosoi 2006-10-24 22:23:33 UTC
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");



Comment 22 Nathan Kinder 2006-10-24 22:26:11 UTC
Yep, those changes look good.


Comment 23 Noriko Hosoi 2006-10-24 23:48:41 UTC
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#.

Comment 24 Nathan Kinder 2006-10-24 23:57:38 UTC
The new set of diffs looks good.

Comment 25 Noriko Hosoi 2006-10-25 00:17:15 UTC
Created attachment 139291 [details]
cvs commit message (ldapserver)

Thank you for the reviews and comments, Nathan and Rich.

Checked in into HEAD.

Comment 27 Noriko Hosoi 2006-10-25 17:07:52 UTC
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

Comment 28 Noriko Hosoi 2006-11-01 21:23:58 UTC
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.

Comment 29 Noriko Hosoi 2006-11-01 21:25:28 UTC
(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.



Comment 30 Noriko Hosoi 2006-11-02 02:25:18 UTC
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.

Comment 31 Rich Megginson 2006-11-22 02:09:07 UTC
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


Comment 32 Noriko Hosoi 2006-11-22 02:49:13 UTC
(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... ):

Comment 33 Rich Megginson 2006-11-22 03:01:17 UTC
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