Michal, See comments below from my quick skip through ...
Paul Michal Bachorik - Sun Microsystems - Prague Czech Republic wrote: > > I am looking for a reviewer for a freeipmi project, already ARC approved > (http://arc.opensolaris.org/caselog/PSARC/2009/245/). The webrev is > accessible at http://cr.opensolaris.org/~jf222792/sfwnv_wr/. === Start of Comments === 1. usr/src/cmd/freeipmi/METADATA Update as per "SFW Package writing guidelines" ... "http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines"; 2. usr/src/pkgdefs/Makefile & usr/src/cmd/Makefile Resync with gate/clone 3. usr/src/pkgdefs/SUNWfreeipmir/depend & SUNWfreeipmir/Makefile Use the default 'depend' rather than this in the root package 4. SUNWfreeipmir/Makefile & SUNWfreeipmiu/Makefile where is these ? 5. File Copyright year Correct this in all relevant files 6. Top of file layout Fix this in all files as per ... "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; 7. usr/src/cmd/freeipmi/Makefile.sfw Extract the VER= info from the METADATA - there are numerous examples in gate now. Lines ... 61 MAKE=$(CCSMAKE) \ 62 DESTDIR="$(ROOT)" \ 63 INSTALL=/usr/bin/ginstall you could put these before the '$(CCSMAKE) install', and for lines 68-70. Do you need to use 'protofix' to fix the permissions on files installed with 'make install' ? (see comment 13) Lines 84-91 and 95-103 ... use the predefined '--prefix=' value from Makefile.master ie, something like ... CONFIGURE_OPTIONS += --with-dont-check-for-root CONFIGURE_OPTIONS += --sysconfdir=/etc etc. .... $(SHELL) ./configure $(CONFIGURE_OPTIONS) 8. usr/src/cmd/freeipmi/install-sfw Change as per ... 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) Why don't you use the '_install' macro rather than ginstall? Man pages: why not edit the man pages files and then install into the proto area with '_install M ..) 9. usr/src/cmd/freeipmi/sunman-stability Where is the stability stuff? Add line for where to get source from on opensolaris.org? 10. usr/src/pkgdefs/SUNWfreeipmir/pkginfo.tmpl DESC= line, put pkg version at end of line, eg. DESC="................... (0.7.7)" 11. usr/src/pkgdefs/SUNWfreeipmir/prototype_com & usr/src/pkgdefs/SUNWfreeipmiu/pkginfo.tmpl Line ... 53 f none etc/ipmi_monitoring_sensors.conf does this file need to be preserved over SUNW pkg update? (may be others) Lines ... ARCH="i386" VERSION="11.11.0,REV=2009.04.03.13.50" are wrong - this looks like the file has been built - get .tmpl file from another package in the gate/clone 12. usr/src/pkgdefs/SUNWfreeipmiu/depend Again this looks like the default 'depend' - have you checked you have no other dependencies with the dependency checker script? 13. usr/src/pkgdefs/SUNWfreeipmiu/prototype_com Do not install files in /usr with the write permision bit set. 14. Amanda's comments === End of Comments === -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit www.gdc4s.com/Tadpole
