Hi Srirama, Thanks for your review, please see my replies below.
Srirama Sharma wrote: > Hi Chris, > > Below are few comments: > > 1. In usr/src/cmd/pen/Makefile.sfw, do you really require to add > "--with-ssl=$(CFGSFW)" into CONFIGURE OPTIONS ? ssl has now been > moved to '/usr', so i guess this may not be required if your build > machine has similar setup compared to the public build machines. Yes, thank you. Now I use --with-ssl=$(CFGPREFIX) and I add the dependency on SUNWopensslr. > > 2. In usr/src/cmd/pen/Solaris/pen-wrapper, > - In line 1, replace "#!/sbin/sh" with "#!/usr/bin/ksh93" > - Change > 36 . /lib/svc/share/smf_include.sh > > to > 36 source /lib/svc/share/smf_include.sh > - At the last line, you could probably replace "exit 0" with "exit > $SMF_EXIT_OK" I only accept the last one. Because I found most of the files in /lib/svc/method use '/sbin/sh' and '.' Maybe this file should not conform to rules of the sfw gate. > > 3. In usr/src/cmd/pen/Solaris/pen.xml, > - In line 34, shouldn't it be SUNWpenr instead of SUNWpen as shown > below ? Please check. > 34 <service_bundle type='manifest' name='*SUNWpenr*:pen'> > - In the stop exec_method, you could exec '/lib/svc/method/pen > stop' instead of ':kill' Done. > > 4. Make sure you resolve the conflicts in usr/src/pkgdefs/Makefile so > that you don't revert back the recent integration changes. Done. > > 5. In usr/src/pkgdefs/SUNWpen/Makefile, you need to remove "DATAFILES > = depend" entry as you have product specific depend file checked in. Done. > > 6. In usr/src/pkgdefs/SUNWpen/depend, > - In the Makefile.sfw it appears that you are configuring pen to > use ssl support. But the depend file doesn't have an entry of > "SUNWopensslr".Please add the same. Done. > - Please check if there are any missing dependencies by running > the check-deps.pl script or by doing "make check_deps" in > usr/src/pkgdefs/SUNWpen I get an error message error: SFW_PKGDB must be set using "make check_deps" in usr/src/pkgdefs/SUNWpen. Any ideas? > > > 7. In usr/src/pkgdefs/SUNWpenr/depend, you have set the dependency on > SUNWpen but it is generally be other way round. SUNWpen should have a > dependency on the root package SUNWpenr. Please see below examples for > reference: > > http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWcupsu/depend > > http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWcupsr/depend > In this case you can use the default depend file for the root > package by specifying "DATAFILES=depend" in > usr/src/pkgdefs/SUNWpenr/Makefile. > Done. The revised webrev is at, http://cr.opensolaris.org/~mishuang/pen/ Thanks, Chris > > Thanks, > Srirama > > Christopher Mi said the following on Friday 27 February 2009 07:39 AM: >> Hi all, >> >> Please help to review pen, a load balancer for "simple" tcp based >> protocols. >> >> The home page is at, >> http://siag.nu/pen/ >> >> The webrev is at, >> http://cr.opensolaris.org/~mishuang/pen/ >> >> Any comments are appreciated. >> >> Thanks, >> Chris >> _______________________________________________ >> sfwnv-discuss mailing list >> sfwnv-discuss at opensolaris.org >> http://mail.opensolaris.org/mailman/listinfo/sfwnv-discuss
