Hi Stefan, See below for some comments (hope it makes sense) ....
Paul Stefan Teleman wrote: > 6752217 Upgrade nmap to 4.75 > 6755981 Upgrade PCRE to 7.8 > > http://cr.opensolaris.org/~steleman/6752217-6755981/ > === Start of Comments ==== 1. usr/src/cmd/nmap/METADATA & usr/src/lib/pcre/METADATA Update as per guidelines at ... http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines 2. usr/src/cmd/nmap/Makefile.sfw Do you need the following lines ? ... 116 DESTDIR=$(DESTDIR) \ 134 CONFIGURE_OPTIONS="$(CONFIGURE_OPTIONS)" \ Why is this not being installed directory into the normal ws proto area ? ... 3 DESTDIR = $(SRC)/cmd/nmap/proto Are there really core files to delete ... 97 @find . -name core -exec rm -f {} \; if there are shouldn't you find out why? Could have used ... INSTALL = $(GINSTALL) -c instead of ... 34 INSTALL = /usr/bin/ginstall -c Lines ... 32 PREFIX = $(ROOT)/usr 68 PKG_CONFIG_PATH_32 = $(PREFIX)/usr/lib/pkgconfig: ... the use of PREFIX doesn't look right on 68 - usr/usr ! Could have used ... PREFIX = $(ROOT)/$(CFGPREFIX) instead of .. 32 PREFIX = $(ROOT)/usr You could extract the VER= info from the METADATA, something like ... VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) TARBALL =$(VER).tar.bz2 Maybe change ... 69 CONFIGURE_OPTIONS = \ 70 --prefix=$(CFGPREFIX) \ 71 --disable-shared \ 72 --enable-static \ 73 --disable-libtool-lock \ 74 --localstatedir=/var \ etc. to ... CONFIGURE_OPTIONS += --disable-shared CONFIGURE_OPTIONS += --enable-static etc. using its default initial state (from Makefile.master). Isn't there a bit of duplication in these lines .. 114 INSTALL="$(INSTALL)" \ 116 DESTDIR=$(DESTDIR) \ 117 $(GMAKE) "DESTDIR=$(DESTDIR)" "INSTALL=$(INSTALL)" install ) ie. do you need lines 114 & 116? 3. usr/src/cmd/nmap/install-nmap You could have passed the VERS= info into the script from Makefile.sfw using an environment var Why have you got the 'write permission' set on the files? remove it. Roland Mainz wrote: > .... and add a > $ set -o errexit # at the beginning and replace > ". ${SRC}/tools/install.subr" with > "source ${SRC}/tools/install.subr" (the idea is to catch > failures in the script and abort it at that point, > right now the script will just continue) 4. usr/src/lib/pcre/Makefile.sfw You could extract the VER= info from the METADATA, something like ... VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) TARBALL =$(VER).tar.bz2 Could use ... PREFIX = $(CFGPREFIX) instead of .. 35 PREFIX = /usr Why not ... all: all32 all64 lint32 lint64 instead of ... 63 all: real-all 128 real-all: all32 all64 lint32 lint64 Change the './configure .....' to use .. './configure $(CONFIGURE_OPTIONS)' appropriately. 5. usr/src/lib/pcre/install-sfw & usr/src/lib/pcre/install-sfw-64 Roland Mainz wrote: > use /usr/bin/ksh93 or /usr/bin/bash for install-sfw* and > add a $ set -o errexit # at the beginning and replace > ". ${SRC}/tools/install.subr" with > "source ${SRC}/tools/install.subr" (the idea is to > catch failures in the script and abort it at that point, > right now the script will just continue) Why have you got the 'write permission' set on various files? remove it. You could have passed the VERS= info into the script from Makefile.sfw using an environment var 6. usr/src/pkgdefs/SUNWnmap/pkginfo.tmpl Copyright year needs changing 7. usr/src/pkgdefs/SUNWpcre/prototype_com Why have you got the 'write permission' set on various files? remove it. === End of Comments ====== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
