Amanda, See comments below ..
Paul Amanda Waite wrote: > > Can I get a code review for the integration of the FastCGI application > libraries into SFW. The ARC case for this is PSARC/2009/014, OSR is > 11091 Webrev is at http://cr.opensolaris.org/~tekgrrl/FastCGI/ > > > There are no C++ libraries in this integration, as per the ARC case they > will be integrated when the Apache C++ stdlib is integrated and > available to build against. I'll raise a separate CR for that work as > part of this integration. There are still CXX related variables used in > the build just to make sure that the C++ libraries build ok and don't > break the SFW build. > > I've run make check_deps and there really are no other dependencies. 1. usr/src/lib/fastcgi/METADATA You might want to include the blank COMMENTS: line ... "http://wikis.sun.com/display/SFWNotes/Package+writing+guidelines"; so it conforms. 2. usr/src/lib/fastcgi/Makefile.sfw You could replace PREFIX with CFGPREFIX from Makefile.master You might want to do .. Roland Mainz wrote: > use either $(SHELL) or /usr/bin/bash for "configure" > calls (so we know which one is used and "configure" > doesn't pick one itself). Combine lines ... 139 -rm -rf $(VER) 140 -rm -rf $(VER64) 141 -rm -rf $(DESTDIR) into one ... -rm -rf $(VER) $(VER64) $(DESTDIR) Lines ... 88 $(CCSMAKE) install 94 $(CCSMAKE) install why are these not installing into the ws proto area? Why not use the same name for TMPDIR and DESTDIR (its confusing) throughout ... (and into install-sfw*) Is this used ? ... 76 LIBRARY=libfcgi.a Could this lint stuff ... 75 # lint stuff 76 LIBRARY=libfcgi.a 77 LINTOUT= lint.out 78 LINTFLAGS= -I./include -I. $(CCBITS32) $(LARGEFILES) 79 LINTFLAGS64= -I./include -I. $(CCBITS64) $(LARGEFILES) be moved down to go with the lint rules? 3. usr/src/lib/fastcgi/install-sfw Do these need the write permission bit set ? ... 117 _install N ../llib-lfcgi ${LIBDIR}/llib-lfcgi 0644 118 _install N llib-lfcgi.ln ${LIBDIR}/llib-lfcgi.ln 0644 ) and in .. usr/src/lib/fastcgi/install-sfw-64 and in .. usr/src/pkgdefs/SUNWfcgi/prototype_com usr/src/pkgdefs/SUNWfcgi/prototype_i386 usr/src/pkgdefs/SUNWfcgi/prototype_sparc 4. pkg copyright files (SUNW*/copyright) You probably need to add the source owner copyright lines at the top at the top of this, ie. those extracted from the src code files in the unpacked tarballs. 5. usr/src/pkgdefs/SUNWfcgi-doc/depend Does the doc package really depend on SUNWfcgi, ie. to install and look at the doc do I have to have SUNWfcgi installed also? I'm guess, no. 6. SUNWfcgi-doc Is there a reason why the doc has been split out from the main package? -- ---------------------------------------------------------------------- Paul Cunningham Software Engineer Tadpole Business Unit
