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! >> 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. >> 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 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) Paul -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
