Paul, First thanks for reviewing. Paul, I have incorporated all you comments into the code along with Jim's. I have posted a new webrev and my comments are inline.
-Rich Paul Cunningham wrote: > 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 Done > > 2. usr/src/lib/Makefile > Add it alphabetically Done > > 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)) Done > > 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 - I've changed this but want to have fresh source bit for the 64 bit compile and configure step. Thus the creation and deletion of the tmp directory. I could not find a way with tar/gtar to uncompress into a directory name differing from what been included into the tar archive. If there is a different way of doing this please let me know. > > 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. Removed > > 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. Done > > 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. Done > > 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. Added line to copyright file indicating location of electronic copy of license. > > 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 Done changed to 555 > > END
