Hi Paul, Thanks a lot for the comments. Would you please see my embedded reply. I have updated the webrev according to your comments except the License issue of prototype_com. Webrev: http://cr.opensolaris.org/~rokh/libconfuse/.
Thanks, -Rokh Paul Cunningham wrote: > Rokh, > > This mainly looks good to me, see below for a few comments ... > > Paul > > Rokh Wang wrote: >> I am porting "libconfuse". libconfuse is a configuration file parser >> library . >> More information can be found at http://www.nongnu.org/confuse. >> Webrev is at "http://cr.opensolaris.org/~rokh/libconfuse/";. > > 1. usr/src/pkgdefs/SUNWlibconfuse/prototype_com > & usr/src/pkgdefs/SUNWlibconfuse/prototype_i386 > & usr/src/pkgdefs/SUNWlibconfuse/prototype_sparc > Remove the write permission bit from files installed in /usr > I have changed all the files with 644/755 permissions to 444/555. > 2. usr/src/pkgdefs/SUNWlibconfuse/prototype_i386 > & usr/src/pkgdefs/SUNWlibconfuse/prototype_sparc > I don't think the symbolic link ("s ...") lines need the > permissions/owner/group stuff on them, see ... > "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWcupsu/prototype_i386"; > > > I have changed prototype_i386/prototype_sparc/prototype_com according your comments. > 3. usr/src/pkgdefs/SUNWlibconfuse/prototype_com > You might want to add that it is the 'ISC License (ISCL)' some > where at the top, eg ... > 1 Copyright (c) 2002,2003,2007 Martin .... > > ISC License (ISCL) > 2 > 3 Permission to use, copy, modify, .... > I just wonder why I need to add the "ISC License" at the top of the prototype_com. Since there is CDDL header already there and this file is created by me not from the source of libconfuse. > 4. usr/src/lib/libconfuse/Makefile.sfw > Would it be better to explicitly define the 'configure --prefix=...' > value? You could also use the predefined CONFIGURE_OPTIONS > stuff from Makefile.master, eg. something like ... > CONFIGURE_OPTIONS += --enable-shared > CONFIGURE_OPTIONS += --disable-nls > ... > ... > ./configure $(CONFIGURE_OPTIONS)) > > Put the following all on the same line ... > 80 -rm -rf $(VER) \ > 81 $(VER64) > eg .... > 80 -rm -rf $(VER) $(VER64) > I have changed the Makefile.sfw according your comments.
