Hi Paul
Please have a look again, I had rebuilt it successfully.
Thanks
- Alex
On Jun 8, 2009, at 9:18 PM, Paul Cunningham wrote:
> I'll take another look when you have fixed all the other things
> (previous email replies) ; but please rebuilt it before you send out
> a new webrev.
>
> paul
>
> alex zhang wrote:
>> Hi Paul,
>> Thanks a lot for quick reply. A latest webrev had uploaded at the
>> same link:
>> http://cr.opensolaris.org/~alz/bittorrent/
>> My answers are added below, waiting for your advice.
>> Thanks
>> - Alex
>> On Jun 8, 2009, at 5:50 PM, Paul Cunningham wrote:
>>> 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";
>> both cmd/rtorrent/METADATA and lib/libtorrent/METADATA had updated.
>>>
>>> 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/
>>> "
>> All files had checked and updated
>>>
>>> 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' ?
>> in line 52, I set CC=$(CCC), so it isn't conflict with each other.
>> in configure option, I told him to use sun CC instead of g++
>>>
>>> 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
>> removed
>>>
>>> Lines ...
>>> 67 autoheader; \
>>> 68 libtoolize --automake --copy --force; \
>>> 69 automake-1.10; \
>>> 70 autoconf; \
>>> do you need to run these ?
>> since one patch file change the "configure.ac" and "Makefile.am", I
>> think it is safer
>> 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
>> change
>> VERSDIR=$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh)?
>>>
>>> 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
>> same as question 3.
>>> Line ..
>>> 69 (cd $(VER); env DESTDIR=$(DESTDIR)
>>> change 'env ' to 'env - '
>> changed
>>>
>>> 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 ?
>> see answer 3
>>>
>>> Line ...
>>> 112 cd ..)
>>> I don't think this is needed.
>> done.
>>> 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)
>> previous I use "source", but it always report "can not find the
>> command source"
>> I don't know if it is cause by Makefile "$(SH) ./install-sfw"
>>>
>>> 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
>> I am not very clear, just remove them? Please help me have a look
>> at the latest webrev if it is ok.
>>>
>>> 8. libtorrent.3 and install-sfw
>>> I don't think this has the sunman stability stuff in it when
>>> installed ?
>> I just write according to libsigsegv/sigsegv.3, since I had
>> provided a man page, need I provide
>> sunman-stability file?
>>>
>>> 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
>>>
>>> "
>> added
>>>
>>> 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)"
>> done
>
>
> --
> ----------------------------------------------------------------------
> Paul Cunningham
> Software Engineer
> Tadpole Business Unit
