Hi Paul:
Thanks for your advice. I've updated the webrev according to your comment: http://cr.opensolaris.org/~fan.li/aalib/ Thanks ~Li Fan Paul Cunningham wrote: > Li Fan, > > See below ... > > Li Fan wrote: >> Thanks very much for your review. Please see my replies. >> >> Paul Cunningham wrote: >>> See below for my comments .... > >>> Li Fan wrote: >>>> >>>> re-generate webrev: >>>> http://cr.opensolaris.org/~fan.li/aalib > ... cut ... > >>> 3. usr/src/lib/aalib/METADATA > ... cut .. >>> Shouldn't the URL: point to ... >>> "http://aa-project.sourceforge.net/aalib/"; >> This is old website.They have moved to >> http://sourceforge.net/projects/aa-project > > okay :-) > > ... cut .. > >>> 6. usr/src/lib/aalib/install-sfw >>> & usr/src/lib/aalib/install-sfw-64 >>> Remove the write permission bit from files installed >>> into /usr >> AA-lib source code default to give write permission to binaries. >> Fedora aa-lib package also do this. So I think keep as it is would be >> the best solution. > > The solaris sfw practice is not to install new files into /usr with > the write permission bits set - as is done by all recent integrations > into the sfwnv gate. So I think you should remove them! Removed the write permission! Thanks. > >>> 8. usr/src/pkgdefs/SUNWaalib/copyright >>> Add the Sun disclaimer and the pkg src owner copyright >>> lines as in ... >>> "http://src.opensolaris.org/source/xref/sfw/usr/src/pkgdefs/SUNWmeld/copyright"; >>> >>> >> Added the Sun disclaimer. Thanks. >> I don't think it's necessary to put author info in copyright, since >> the GPLv2 license file in aalib source code does not include it, and >> lots of packages in sfw doesn't have it in copyright file. > > It's normal practice now to include the package owner copyright lines > (those that don't are historic and probably wrong), ie. those > extracted from the unpackaged source files. Added the author info in copyright. > >>> 10. usr/src/pkgdefs/SUNWaalib/prototype_com >>> & usr/src/pkgdefs/SUNWaalib/prototype_i386 >>> & usr/src/pkgdefs/SUNWaalib/prototype_sparc >>> Remove the write permission bit from files installed >>> into /usr (also see (6)) >> Same as 6 > see above comment Done > > additional comments ... > > 1. usr/src/lib/aalib/Makefile.sfw > Use the predefined prefix value from Makefile.master, > ie. CFGPREFIX. > Your prefix as defined shouldn't have the ROOT bit > otherwise it tells configure that at runtime it's > installed and run from the build workspace's proto area > (when it should be /usr/...). > So you could just change (lines) ... > 34 PREFIX=${ROOT}/usr > 35 CONFIGURE_OPTIONS += --prefix=$(PREFIX) > --infodir=$(PREFIX)/share/info \ > 36 --mandir=$(PREFIX)/share/man > to ... > CONFIGURE_OPTIONS += --infodir=$(CFGINFO) okay :-) > > Paul
