Robin, See comments below ...
Paul Robin Guo wrote: > > I just put a code review for tcpdump package porting, > would you or somebody else have time to take a look on it? > > The webrev is at http://cr.opensolaris.org/~robinguo/tcpdump/ === Start of Comments ==== 1. CDDL HEADER and top of files Please updated as per those in ... "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; 2. usr/src/cmd/tcpdump/METADATA Add missing fields as in ... http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines 3. usr/src/cmd/tcpdump/Makefile.sfw Extract the VER= info from the METADATA, see recent integrations for examples. 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) Change the following so that it uses the predefined --prefix= value from Makefile.master, and also use the CONFIGURE_OPTIONS method of setting and using configure options ... 54 ./configure --prefix=/usr \ 55 --enable-ipv6=yes ) So something like ... CONFIGURE_OPTIONS += --enable-ipv6=yes etc .... .... ./configure $(CONFIGURE_OPTIONS) Do you really need the line ? ... 41 @find . -name core -exec rm -f {} \; if not remove it Add line after '29 VER= ...' ... TARBALL=$(VER).tar.gz then replace throughout ... $(VER).tar.gz with ... $(TARBALL) 3. Portions Copyright in various files Remove the 'Portions Copyright' lines, eg ... # /* Portions Copyright 2007 ShivaKumar GN */ 4. usr/src/cmd/tcpdump/install-sfw Change as .. Roland Mainz wrote: > 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) Pass the PKGVERS= info in from your Makefile.sfw (extracted from METADATA) as an environment var or as an option. 5. usr/src/pkgdefs/SUNWtcpdump/copyright Add the package owner copyright lines (from src files) at the top, see example .. "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; 6. usr/src/pkgdefs/SUNWtcpdump/depend Move Copyright lines to after the CDDL HEADER END header + as in (1) 7. usr/src/pkgdefs/SUNWtcpdump/pkginfo.tmpl Add version number to end the DESC= line, eg. ... DESC="....... (4.0.0)" === End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
