On Wed, Feb 27, 2008 at 01:40:44AM -0600, Norm Jacobs wrote: > > I took a look at your webrev and have some questions/comments... > > Does SUNWsqlite need to declare any dependencies outside of the common > depend file in the gate?
No, it doesn't. > usr/src/lib/Makefile > > Are you missing a dependency for "sqlite: tcl"? Yes, I am. > usr/src/lib/sqlite3/Makefile.sfw > > You use the same configure options for your 32 and 64 bit > config.status targets. You might want to put them in a common > variable (CONFIGURE_OPTIONS) and call "./configure $(CONFIGURE_OPTIONS)" Yeah. At one point I was using different options and trying to install the 32-bit build straight into $ROOT while the 64-bit build into $ROOT/tmp/... When I changed this I obviated the need for different configure options (other than CFLAGS). > You shouldn't need -R/usr/lib and -L/usr/lib or -R/usr/lib/64 and > -L/usr/lib/64, but you should probalby have -L$(ROOT)/usr/lib or > -L$(ROOT)/usr/lib/64 depending on if you are building the 32 or 64 > bit target. Right. > Be sure that you are linking against the TCL in the gate instead of > build system so we don't end up with a mismatch if/when TCL upgrades > at some future date. Hmmm, good point. > You appear to be staging your install and cherry picking the bits > from your staged area. You might consider setting DESTDIR=$(ROOT) > and INSTALL to $(SRC)/tools /install-proto and installing your bits I tried this, but it gets complicated by the fact that I want the 32-bit sqlite3 executable but not the 64-bit one. It's *much* simpler to install into DESTDIR=$(ROOT)/tmp/sqlite3-$(BITS) and then let install-sfw do the cherry picking. > directly into the proto area. It is bound to be slightly more > involved than simply "cd $(VER) ; make ... install" particularly > because libtool installs it's own turds that you should probably not > deliver, but it will most likely save some copying of data and less > likely to miss something. Exactly. I'm _not_ going to do this. I tried it and it sucked. > usr/src/lib/sqlite3/install-sfw > > The check_elf_soname() and fix_elf_runpath() functions *should* not check_elf_soname() is there because SQLite3 uses 8.8.6 as a version number in libtool command lines (which is where libsqlite3.so.0.8.6 comes from) and I want to make sure that this never accidentally ends up resulting in a different SONAME than the current one (libsqlite3.so.0). It's a safety feature, for the next fellow who comes along to update SQLite in SFW to some new version. fix_elf_runpath() is needed because libtool puts extraneous paths in libtclsqlite3.so's RUNPATH. > be necessary. The result of "cd $(VER) ; make install" should be > setting the SONAME and RUNPATH correctly. If not, you should > probably be patching the sqlite3 build environment and working with > the sqlite community to get fixes back in upstream. The same can > probably be said for mapfile support. I don't know that I can get libtool to not put build paths into libtclsqlite3.so's RUNPATH. I want to keep changes to the SQLite3 build system to an absolute minimum without unnecessarily complicating our SFW build. Once I decided that I wanted a SONAME check as a safety feature then adding fix_elf_runpath() was quite straightforward and not an unnecessary complication. > usr/src/pkgdefs/SUNWsqlite3/Makefile > > Personally, I would rather not see you generating prototype_com on > the fly via cat/find/echo I'll keep it this way. We definitely should list all the executables, shared objects, header and such files in the prototype files, but I think we are probably much less interested in doing this for the doc files -- those are much more likely to change from SQLite release to release. Consider this a favor to the next person to update SQLite3 in SFW. > usr/src/pkgdefs/SUNWsqlite3tcl/prototype_* > usr/src/pkgdefs/SUNWsqlite3/prototype_* > > You might want to sort these based on path Why? > You might want to run Capser's fix-copyright across this workspace. The > following files need copyright updates: > > usr/src/tools/install.subr > usr/src/lib/Makefile > usr/src/pkgdefs/Makefile Ah, yes, I'll fix those. Thanks! Nico --
