Rich, See below for my comments ...
Paul Rich Reinhard wrote: > > I've posted a new webrev with your comments incorporated. Except for > removal of 64 bit binaries. Why do we not deliver these? We have > compiled them. > Webrev: http://cr.opensolaris.org/~richrein/yaz/ >> Rich Reinhard wrote: >>> >>> I am porting the YAZ which is a programmers? toolkit supporting the >>> development of Z39.50/SRW/SRU clients and servers. Would you please >>> help review, any feedback would be much appreciated. >>> >>> Webrev: http://cr.opensolaris.org/~richrein/yaz/ 1. usr/src/lib/libyaz/METADATA The src version on these lines differ ... 2 VERSION: 3.0.46 7 SOURCE_DOWNLOAD: http://ftp.indexdata.dk/pub/yaz/yaz-3.0.45.tar.gz 2. usr/src/lib/Makefile Add it alphabetically 3. usr/src/lib/libyaz/Makefile.sfw Use the configure prefix defined in Makefile.master (CFGPREFIX), or better still changed the configure lines 81-82 and 91-94 to use CONFIGURE_OPTIONS, eg. something like (see examples in gate) ... change lines 81-82 to .. $(SHELL) ./configure $(CONFIGURE_OPTIONS)) and change lines 91-94 to ... CONFIGURE_OPTIONS += --bindir=$(PREFIX)/bin/$(MACH64) CONFIGURE_OPTIONS += --libdir=$(PREFIX)/lib/$(MACH64) .... .... $(SHELL) ./configure $(CONFIGURE_OPTIONS)) Why do you need to do the, mkdir, mv, rm stuff on lines ? .. 97 $(MKDIR) -p tmp; gzip -dc $(VER).tar.gz | (cd tmp; $(TAR) xopf -) 98 $(MV) tmp/$(VER) $(VER); $(RMDIR) tmp couldn't you have just done ... gzip -dc $(VER).tar.gz | $(TAR) xopf - Lines ... 100 $(CHMOD) 755 $(VER)/configure 106 $(CHMOD) 755 $(VER64)/configure do you need to do these, or are they for "just-in-case" ? If they are for "just-in-case" you should probably remove them. 4. usr/src/lib/libyaz/install-sfw & usr/src/lib/libyaz/install-sfw-64 Pass in the VERS= info from Makefile.sfw (see examples in gate) Don't install files with the write permission bit set, eg. change 755 to 555, and 644 to 444, etc. Note, you shouldn't need the protofix invocation in Makefile.sfw if these are correct. 5. usr/src/lib/libyaz/sunman-stability Line .. 51 Source for YAZ is available on http://www.indexdata.dk/yaz.\ I think that should refer to opensolaris.org rather than the community site. 6. usr/src/pkgdefs/SUNWlibyaz/copyright Shouldn't this refer to it being "Revised BSD License" (from http://www.indexdata.dk/licensing/bsd/) somewhere in it. 7. usr/src/pkgdefs/SUNWlibyaz/prototype_i386 & usr/src/pkgdefs/SUNWlibyaz/prototype_sparc & usr/src/pkgdefs/SUNWlibyaz/prototype_com Do not install stuff into /usr with the write permission bit set (755, etc) unless its really required END -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
