Colin, A few more comments ...
1. You need to change the SUNW package name(s) in the METADATA file. 2. SUNWlogrotater/Makefile - does this need the default 'depend' added: DATAFILES = depend ? 3. SUNWlogrotater/pkginfo.tmpl - you need to add '(root)' to the NAME= and DESC= lines I think (see other root pkgs for examples). 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? Paul > -----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 ===== > >>> > >
