Brian, See below ...
paul Brian Utterback wrote: > I'm upgrading NTP from the currently included version 3 to version 4, > specifically version 4.2.5p161. There are two steps to this, since the > packages currently reside in the ON consolidation, and afterwards they > will reside in the SFW consolidation. > > This is the second round of review for the SFW consolidation. The new > webrev incorporates the changes requested on the first round. This is > the first round for the removal of the existing NTP code from the ON > consolidation. > > The webrevs are at: > http://cr.opensolaris.org/~blu/ntpv4/sfwnv-webrev/ See below ... > and > http://cr.opensolaris.org/~blu/ntpv4/onnv-webrev/ The ONNV bit looks okay to me === Start of Comments ==== 1. usr/src/cmd/ntpd/METADATA This needs updating to conform to the heads-up of last week, as in ... "http://wikis.sun.com/display/SFWNotes/METADATA"; Line ... 8 SUPPORT: Full is that the correct support classification? 2. tarball ntp-dev-4.2.5p161.tar.gz This is not in the webrev? 3. version ntp-dev-4.2.5p161 The source pkg's website says there is version 'R.C.: 4.2.4p7-RC7 2009/05/12', so should you be using that now? 4. usr/src/cmd/ntpd/install-sfw Some of the directories you are '${INSTALL} -d'ing, are already in the 'Targetdirs' file so you don't need to install then here. And the other directories created here could probably also be put in 'Targetdirs'. You could have used the '_install' macro rather than '${INSTALL}' for installing files. Change as ... Roland Mainz wrote: > use /usr/bin/ksh93 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) 5. usr/src/cmd/ntpd/Makefile.sfw & usr/src/cmd/ntpd/install-sfw Line ... 69 (cd $(VER); env - $(CCSMAKE) all $(TARGET_ENV)) && \ install-sfw $(VER) is the 'make install' installing different stuff to 'install-sfw' ? 6. Tops of files (various files) Cosmetic: Change so that they conform to those in ... "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; as per "http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines"; 7. patches I haven't looked at these - I assume they are okay 8. usr/src/cmd/ntpd/Solaris/ntp Why '/sbin/sh' rather than '/usr/sbin/sh' ? 9. man pages Note, I haven't read their content 10. usr/src/cmd/ntpd/Solaris/ntp-keygen.1m & usr/src/cmd/ntpd/Solaris/ntpq.1m & usr/src/cmd/ntpd/Solaris/ntptime.1m & usr/src/cmd/ntpd/Solaris/ntptrace.1m Does this need the 'Interface Stability' stuff added? And the where to find source added ? ... "Source for xxx is available on http://opensolaris.org 11. usr/src/cmd/ntpd/Solaris/ntp.conf.4 & usr/src/cmd/ntpd/Solaris/ntprc.4 Does this need the sunman stability stuff added? 12. usr/src/cmd/ntpd/Solaris/ntp.xml The Copyright lines should be after the 'CDDL HEADER END' header 13. usr/src/pkgdefs/SUNWntpr/Makefile You have your own pkg copyright files so you don't need the 'copyright' in DATAFILES= 14. usr/src/pkgdefs/SUNWntpr/depend This looks like the default 'depend' file so you can delete it as SUNWntpr/Makefile says use the default one. 15. usr/src/pkgdefs/SUNWntpr/pkginfo.tmpl & usr/src/pkgdefs/SUNWntpu/pkginfo.tmpl Add source pkg version at the end of the DESC= line, eg ... DESC=".................. (x.x.x)" 16. usr/src/pkgdefs/SUNWntpu/copyright & usr/src/pkgdefs/SUNWntpr/copyright Does this need to include the lines 19 to 55 ? Should it also include a copy of the 'Modified BSD' licence (or are lines 5-15 that) 17. usr/src/pkgdefs/SUNWntpu/depend Does this really have no other dependencies. What about SUNWntpr and others. If not it should use the default 'depend' 18. usr/src/cmd/ntpd/Solaris/ntprc.4 Have you delivered this in a SUNW pkg, I can't see where? === End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
