Cunningham, Paul - UK wrote: Hi Paul,
> Colin, > > A few more comments ... > > 1. You need to change the SUNW package name(s) in the METADATA file. > Done. > 2. SUNWlogrotater/Makefile - does this need the default 'depend' added: > DATAFILES = depend ? > I actually removed the line because all this package do is install a file into /etc. There are no code or anything. I ran the dependency checker script and it ran clean on this package. > 3. SUNWlogrotater/pkginfo.tmpl - you need to add '(root)' to the NAME= > and DESC= lines I think (see other root pkgs for examples). > Some did while others did not. I added the info. > 4. SUNWlogrotateu/depend - does this need to depend on SUNWlogrotater > also. What about the other pkgs as in default depend. Have you run the > dependency checker script on this pkg? > Yes, I ran it again and it is clean for both packages. > Paul > Thanks for you review Paul. Since the above changes are rather small, I will run a full bringover and build and post the webrev again for 1 last look see. I will run the dependency script again. You mean this one right: /home/mike_s/run_checkdeps_one Colin > > >> -----Original Message----- >> From: Colin.Ngam at Sun.COM [mailto:Colin.Ngam at Sun.COM] >> Sent: 03 July 2008 14:00 >> To: Cunningham, Paul - UK >> Cc: sfwnv-discuss at opensolaris.org >> Subject: Re: [sfwnv-discuss] Request for code review - >> logrotate utility >> >> Cunningham, Paul - UK wrote: >> >> Hi, >> >> Here's another go at code review: >> >> http://cr.opensolaris.org/~cn162287/logrotate/webrev >> >> Changes has been made as per the last 2 reviews. There are >> now 2 packages: >> SUNWlogrotater and SUNWlogrotateu >> >> Thanks. >> >> colin >> >> >> >> >>>> what is a root package? >>>> >>>> >>> I Believe (someone correct me if I am wrong please) - It's just >>> another package that contains all the stuff for the package >>> >> that must >> >>> be install on the local machine, eg. the /etc stuff, >>> >> whereas the stuff >> >>> in the base package can be installed & mounted from a >>> >> remote server. >> >>> So you just need to create a second package with the /etc >>> >> stuff in it >> >>> - these root packages normally have an 'r' at the end of their name >>> and '(root)' on the pkginfo DESC line - take a look in the >>> >> gate at other root packages. >> >>> See below for other comments ... >>> >>> Paul >>> >>> >>> >>>> -----Original Message----- >>>> From: sfwnv-discuss-bounces at opensolaris.org >>>> [mailto:sfwnv-discuss-bounces at opensolaris.org] On Behalf Of Colin >>>> Ngam >>>> Sent: 25 June 2008 22:20 >>>> To: Cunningham, Paul - UK >>>> Cc: sfwnv-discuss at opensolaris.org; solaris-qe-pkgproj at Sun.COM >>>> Subject: Re: [sfwnv-discuss] Request for code review - logrotate >>>> utility >>>> >>>> Cunningham, Paul - UK wrote: >>>> >>>> >>>>> Colin, >>>>> >>>>> See below for my comments from my quick skip through ... >>>>> >>>>> Paul >>>>> >>>>> >>>>> >>>>> >>>>>> -----Original Message----- >>>>>> From: sfwnv-discuss-bounces at opensolaris.org >>>>>> [mailto:sfwnv-discuss-bounces at opensolaris.org] On Behalf >>>>>> >> Of Colin >> >>>>>> Ngam >>>>>> Sent: 23 June 2008 14:45 >>>>>> To: sfwnv-discuss at opensolaris.org; solaris-qe-pkgproj at Sun.COM >>>>>> Cc: Colin Ngam >>>>>> Subject: [sfwnv-discuss] Request for code review - >>>>>> >>>>>> >>>> logrotate utility >>>> >>>> >>>>>> Hi, >>>>>> >>>>>> This is a request for code review. Please see >>>>>> http://cr.opensolaris.org/~cn162287/logrotate/webrev >>>>>> >>>>>> And provide any comments or changes I need to make. >>>>>> >>>>>> >>>>>> >>>>> === Start of Comments === >>>>> >>>>> 1. usr/src/cmd/Makefile >>>>> & usr/src/pkgdefs/Makefile >>>>> This needs resyncing with gate otherwise it looks as >>>>> though you are trying to delete stuff. >>>>> >>>>> >>>>> >>>> okay >>>> >>>> >>> >>> >>>>> 2. usr/src/pkgdefs/SUNWlogrotate/prototype_com >>>>> Shouldn't the 'etc/...' stuff be in a separate >>>>> root package? >>>>> >>>>> >>>>> >>>> Can someone help me out here? I am installing a sample >>>> >> configuration >> >>>> file into /etc/logrotate.conf .. what is a root package? >>>> >>>> PKG="SUNWlogrotate" >>>> NAME="Logrotate - rotates and compresses log file" >>>> ARCH="ISA" >>>> VERSION="SFWVERS,REV=0.0.0" >>>> SUNW_PRODNAME="SunOS" >>>> SUNW_PRODVERS="RELEASE/VERSION" >>>> SUNW_PKGTYPE="usr" >>>> MAXINST="1000" >>>> CATEGORY="system" >>>> DESC="Logrotate - rotates and compresses log file (3.7.1)" >>>> VENDOR="Sun Microsystems, Inc." >>>> HOTLINE="Please contact your local service provider" >>>> EMAIL="" >>>> CLASSES="none" >>>> BASEDIR=/ >>>> SUNW_PKGVERS="1.0" >>>> SUNW_PKG_ALLZONES="false" >>>> SUNW_PKG_HOLLOW="false" >>>> SUNW_PKG_THISZONE="false" >>>> >>>> The prototype_com file looks like: >>>> >>>> d none usr 0755 root sys >>>> d none usr/sbin 0755 root bin >>>> f none usr/sbin/logrotate 0555 root bin d none usr/share 0755 root >>>> sys d none usr/share/man 0755 root bin d none >>>> >> usr/share/man/man1 0755 >> >>>> root bin f none usr/share/man/man1/logrotate.1 0444 root bin >>>> >>>> >>>> >>>>> 3. usr/src/pkgdefs/SUNWlogrotate/depend >>>>> I can't see this file link - permissions wrong? >>>>> >>>>> >>>>> >>>> Corrected >>>> >>>> >>> I still can't see this - have you update your webrev? >>> >>> >>> >>>>> 4. usr/src/cmd/logrotate/METADATA >>>>> I don't think its normal to have the sccs ident >>>>> stuff in a METADATA file. I don't know if it >>>>> matters though either. >>>>> >>>>> >>>>> >>>> Tools complain .. so I added it to shut it up. >>>> >>>> >>> I wonder why no one else has not had in complain? - unless >>> >> they ignore >> >>> it. >>> >>> >>> >>>>> 5. usr/src/cmd/logrotate/Makefile.sfw >>>>> Does this source package have a 'configure' file? if not >>>>> you may want to add comment to say so and that the 'touch' >>>>> is just there to aid make >>>>> >>>>> >>>>> >>>> No configure file. >>>> >>>> >>> I still think then that you should add a comment into the >>> >> file to say >> >>> what the '$(VER)/configure' is used for if there is not one >>> >> in the src >> >>> pkg. >>> >>> >>> >>>>> 6. usr/src/cmd/logrotate/install-sfw >>>>> The 'INFODIR=' line is not used so delete it >>>>> >>>>> >>>>> >>>> Okay. >>>> >>>> >>> >>> >>>>> === End of Comments ===== >>>>> >>>>> >>
