Steven,
After very quick recheck, this looks okay to me now.
Paul
Steven Le wrote:
> On 09/25/08 03:15, Paul Cunningham wrote:
>> Steven,
>>
>> This looks okay to me now on a quick skip through; you need to resync
>> usr/src/cmd/Makefile with the gate though (so it doesn't look as
>> though you are try to change other stuff).
>>
>> Paul
>>
>> Steven Le wrote:
>>> On 09/23/08 03:03, Paul Cunningham wrote:
>>>> Steven,
>>>>
>>>> I took another quick look - you might also want to change the
>>>> following ...
>>>>
>>>> 1. usr/src/cmd/bonnieplus/METADATA
>>>> Add the BUGTRAQ and SUPPORT info, eg. ...
>>>> SUPPORT: ????
>>>> BUGTRAQ: solaris/utility/????
>>>>
>>>> 2. usr/src/cmd/bonnieplus/install-sfw
>>>> (I forget if you responded to this ..)
>>>> You might want to apply these comments ..
>>>>
>>>> Roland Mainz wrote:
>>>> > - Please use /usr/bin/ksh93 or /usr/bin/bash for
>>>> > install-sfw* and add a $ set -o errexit # at the
>>>> > beginning and
>>>> > replace ". ${SRC}/tools/install.subr" with
>>>> > "source ${SRC}/tools/install.subr" (the idea is to
>>>> > catch failures in the script and abort it at that
>>>> > point, right now the script will just continue)
>>>>
>>>> 3. usr/src/cmd/bonnieplus/Makefile.sfw
>>>> (again I forget if you responded to this ..)
>>>> You might want to apply these comments ...
>>>>
>>>> Roland Mainz wrote:
>>>> > - use "env - ..." and not "env ..." in the Makefiles
>>>> > to make sure "configure" & "make" only see the env
>>>> > variables they should really get (and not pick-up
>>>> > any random env variable)
>>>> > - use either $(SHELL) or /usr/bin/bash for "configure"
>>>> > calls (so we know which one is used and "configure"
>>>> > doesn't pick one itself)
>>>>
>>>> Christopher Mi wrote:
>>>> > Use the method define in Makefile.master
>>>> > since you have a standard METADATA file.
>>>> >
>>>> > VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)
>>>> > TARBALL =$(VER).tar.bz2
>>>>
>>>> Does it really core dump during the 'all:' rule, if not
>>>> remove the 'find ... core ...'. If it does, why does it?
>>>>
>>>> 4. depend
>>>> Did you check you have no other dependencies?
>>>>
>>>>
>>>> Everything else looks okay to me
>>>>
>>>> Paul
>>>>
>>>>
>>>>
>>>> Steven Le wrote:
>>>>> Hi all,
>>>>> I have fixed all the problems that you suggested me to do. The
>>>>> up2date files are at:
>>>>> http://cr.opensolaris.org/~steven84/bonnieplus/
>>>>>
>>>>> Please email if there is anything
>>>>> Note: We need your final approval in order to put back, please help
>>>>> me as soon as you can.
>>>>>
>>>>> 1st review: 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
>>>>> * bonnie++ should be installed in usr/sbin instead of usr/bin FIXED.
>>>>> NOTE: our sponsors request us to put into usr/benchmarks
>>>>> * zcav is missing in prototype_com and also add manpage 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
>>>>>
>>>>>
>>>>> 2nd review: FIXED
>>>>> === 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 =====
>>>>>
>>>>
>>> Hi,
>>> Thanks for your review. I have changed files as you have recommended.
>>> Could you help me to review again to see as if I need anything? We
>>> are trying to put back bonnie++ next Tuesday.
>>> http://cr.opensolaris.org/~steven84/bonnieplus/
>>>
>>> Note: C-team needs your final approval in order to complete the review.
>>>
>>
> Hi
> I have bringover and reslove the conflict.
> http://cr.opensolaris.org/~steven84/bonnieplus/ (sync Makefile with the
> gate )
--
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products
General Dynamics Itronix Europe Ltd.
