Hi Stefan, How are you?
See below for my comments .... Paul Stefan Teleman wrote: > Webrev is: http://cr.opensolaris.org/~steleman/6744471/ === Start of Comments === 1. usr/src/lib/curl/METADATA Add fields for ... SUPPORT: ???? BUGTRAQ: solaris/???/??? 2. usr/src/lib/curl/Makefile.sfw You should probably change this to use the predefined --prefix value from Makefile.master. Or better still use the CONFIGURE_OPTIONS method, eg ... CONFIGURE_OPTIONS += .... CONFIGURE_OPTIONS += .... ... ./configure $(CONFIGURE_OPTIONS) You also might want to apply Roland Mainz comment .. > Please use either $(SHELL) or /usr/bin/bash for > "configure" calls (so we know which one is used > and "configure" doesn't pick one itself). You put the 'clean:' rm's as a single 'rm'. Do you need the 'real-all:' rule, it could be just .. all: all32 all64 lint32 lint64 ? The 'test:' rule - where are 'test32 test64' - just remove them maybe. VER=, as you now have a METADATA file you could now extract the info from that I think; something like .. VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) TARBALL =$(VER).tar.bz2 3. usr/src/lib/curl/install-sfw & usr/src/lib/curl/install-sfw-64 Copyright date is wrong. You might want to apply 'Roland Mainz' comments ... - use /usr/bin/ksh93 or /usr/bin/bash for install-* 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). There are a few files with the write permission bit set - does it need this set? 4. usr/src/lib/pcre/Makefile.sfw As you are changing this ... Use the predefined value for '--prefix=..' from Makefile.master; or even better still use the stuff from CONFIGURE_OPTIONS, eg ... ./configure $(CONFIGURE_OPTIONS) see other recent integrations for examples, eg cups. You might want to apply 'Roland Mainz' comments ... - Use "env - ..." and not "env ..." in the Makefiles to make sure "configure" & "make" only see the env variables they should really get(and not pick-up any random env variable) - Use either $(SHELL) or /usr/bin/bash for "configure" calls (so we know which one is used and "configure" doesn't pick one itself). You could extract the version info from the METADATA file as per 'Christopher Mi' comment .. - Use the method define in Makefile.master since you have a standard METADATA file, eg. .. VER =$(COMPONENT_NAME:sh)-$(COMPONENT_VERSION:sh) TARBALL =$(VER).tar.bz2 5. source tarballs I don't see your new and old source tarball in your webrev. === End of Comments ===== -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Computer Products
