Hi all, Thanks for your reviews. I have tried to update changes to old location (http://cr.opensolaris.org/~steven84/bonnieplus) but I got permission denied. So, I have create a new URL for new changes, could you check on that?
New changes: http://cr.opensolaris.org/~steven84/bonnie++/ As far as I go, I fixed most the problems. I have some questions from two reviews (combined): --------START----------- * bonnie++ should be installed in usr/sbin instead of usr/bin * zcav is missing in prototype_com and also add manpage We put bonnie++ into usr/benchmarks and create a symbolic link in usr/bin. Please advise exactly which location bonnie++ will be in. Zcav is included into bonnie++ as a separate program. If we port bonnie++, do we have to port zcav also? usr/src/cmd/bonnieplus/install-sfw > You may want to apply 'Roland Mainz' comments, on other people's install-* > scripts to this. You can find those comments in the sfwnv-discuss archives. >usr/src/cmd/bonnieplus/Makefile.sfwYou may want to use "env - ..." rather than >"env ..."so its doesn't pick-up any random env variable. I have read some of examples in SUNW(packages), most of them using "env \", so I'm confused about using that or "env - \" in makefile.sfw . Could you give me an example when we use without "-", env will pick up a random thing? >usr/src/pkgdefs/SUNWbonnieplus/depend Where I could get the dependency checker script? Do you have example of a package that uses DATAFILES=depend. I keep the my depend file for now. FIXED BINDIR= is not used so you could delete FIXED Create webrev with the CR#6741736 FIXED usr/benchmarks needs to added as part of usr/src/Targetdirs FIXED in METADATA mention URL of bonnie++ FIXED in usr/src/pkgdefs/SUNWbonnieplus/prototype_com add symlinks for bon_csv2txt bon_csv2html FIXED in usr/src/pkgdefs/SUNWbonnieplus/prototype_sparc add '#' between copyright and make sure ident string is correct FIXED remove usr/src/pkgdefs/SUNWbonnieplus/pkginfo file FIXED indent and format problem in files -----------END--------------------- On Wed, Sep 10, 2008 at 12:21 AM, Paul Cunningham <paul.cunningham at tadpole.com> wrote: > David, > > See below for my comments, some of them are probably duplicates of Phadhap's > comments but I have put them here anyway ... > > Paul > > David.Fan at sun.com wrote: >> >> Hello, >> >> We are porting Bonnie++ file system benchmark suite to OpenSolaris. Please >> take some time to review the webrev at: >> >> Webrev: >> http://cr.opensolaris.org/~steven84/bonnieplus/ >> >> RFE: >> 6741736 P3 bonnie++ 1.03c disk benchmark tool to be included >> into SFW consolidation >> >> Bonnie++ home page: >> http://www.coker.com.au/bonnie++ > > === Start of Comments === > > 1. usr/src/cmd/bonnieplus/METADATA > Add line for where the source code came from. 'URL: ...' > > 2. usr/src/cmd/bonnieplus/install-sfw > You may want to apply 'Roland Mainz' comments, on other > people's install-* scripts to this. You can find those > comments in the sfwnv-discuss archives. > > BINDIR= is not used so you could delete > > 3. usr/src/cmd/bonnieplus/Makefile.sfw > You may want to use "env - ..." rather than "env ..." > so its doesn't pick-up any random env variable. > > 4. usr/src/pkgdefs/SUNWbonnieplus/Makefile > Add line space after '#ident ...' line > > 5. usr/src/pkgdefs/SUNWbonnieplus/depend > This looks like the default 'depend' file; if you have > no other dependencies you should remove this file and > add 'DATAFILES= depend' to your SUNWbonnieplus/Makefile. > But have you checked you have no other dependencies with > the dependency checker script? > > 6. usr/src/pkgdefs/SUNWbonnieplus/pkginfo > Remove this files as it is generated from the pkginfo.tmpl > file. > > 7. usr/src/pkgdefs/SUNWbonnieplus/prototype_sparc > Add line space before '#ident ...' line > > 8. usr/src/pkgdefs/SUNWbonnieplus/prototype_i38 > & usr/src/pkgdefs/SUNWbonnieplus/prototype_sparc > '#ident ...' line says ... > # ident "@(#)pkginfo.tmpl ... > > === End of Comments ===== > > -- > ---------------------------------------------------------------------- > Paul Cunningham > Software Engineer > Tadpole Computer Products >
