Alex, See comments below ...
Paul alex zhang wrote: > Thanks Lizhong for review. > A newer version of webrev at http://cr.opensolaris.org/~alz/bittorrent/ > Please have a look at it. >>> >>>> From: alex zhang <Huawei.Zhang at Sun.COM> >>>> Date: April 1, 2009 4:38:05 PM GMT+08:00 >>>> >>>> Please help me have a code review of opensolaris package porting, >>>> the webrev at: >>>> http://cr.opensolaris.org/~alz/libtorrent/, which had sent out a >>>> few days without any response. 1. usr/src/cmd/rtorrent/METADATA & usr/src/lib/libtorrent/METADATA These need updating to the new format, see ... "http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines"; 2. Top of Files Update the tops of your files so that they conform to that in .. "http://src.opensolaris.org/source/xref/onnv/onnv-gate/usr/src/prototypes/"; 3. usr/src/cmd/rtorrent/Makefile.sfw Explicitly set the 'configure --prefix=' value, using $(SHELL) configure $(CONFIGURE_OPTIONS) Lines ... 53 "CXX=$(CC)" \ 57 ./configure CXX=$(CCC) why is CXX= set to $(CC) and why do you then need the CXX=$(CCC) after 'configure' ? You could modernise the makefile and use the inbuild tarball uncompress and patching, see cups/Makefile.sfw Line ... 71 cd .. I don't think you need this Lines ... 67 autoheader; \ 68 libtoolize --automake --copy --force; \ 69 automake-1.10; \ 70 autoconf; \ do you need to run these ? 4. usr/src/cmd/rtorrent/install-sfw & usr/src/lib/libtorrent/install-sfw & usr/src/lib/libtorrent/install-sfw-64 Pass the VERS= ( and/or VERSDIR=) in from the Makefile.sfw 5. usr/src/lib/libtorrent/Makefile.sfw Lines (and other places) .. 49 "CC=$(CCC)" \ 50 "CXX=$(CC)" why is CC equal $(CCC) and CXX equal $(CC) ? Lines ... 83 "CXX=$(CC)" 88 ./configure CXX=$(CCC) do you need the CXX=$(CCC) on the configure line Line .. 69 (cd $(VER); env DESTDIR=$(DESTDIR) change 'env ' to 'env - ' You could modernise the makefile and use the inbuild tarball uncompress and patching, see cups/Makefile.sfw Lines .. 108 autoheader; \ 109 libtoolize --automake --copy --force; \ 110 automake-1.10; \ 111 autoconf; \ do you need to run these ? Line ... 112 cd ..) I don't think this is needed. 6. usr/src/lib/libtorrent/install-sfw & usr/src/lib/libtorrent/install-sfw-64 Apply the following ... 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) 7. usr/src/lib/libtorrent/install-sfw Lines ... 43 # install headers 44 mkdir -p ${INCDIR}/torrent 45 mkdir -p ${INCDIR}/torrent/data 46 mkdir -p ${INCDIR}/torrent/peer add these dirs to the Targetdirs files and remove the 'mkdir's from here 8. libtorrent.3 and install-sfw I don't think this has the sunman stability stuff in it when installed ? 9. patch files Note: I haven't looked as any of these 10. usr/src/pkgdefs/SUNWlibtorrent/copyright & usr/src/pkgdefs/SUNWrtorrent/copyright Add the src owner copyright lines, after the sun disclaimer, extracted from the unpacked tarball files. See example in .. "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; 11. usr/src/pkgdefs/SUNWlibtorrent/pkginfo.tmpl & usr/src/pkgdefs/SUNWrtorrent/pkginfo.tmpl Add src pkg version at end of DESC= line, eg ... DESC="............ (x.x.x)" END -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
