Hi Jim, and thanks for all the comments! On 09/05 10.41, James Gates wrote: > 1) usr/src/cmd/Makefile > > You've made the libpqxx build dependent on the postgres 8.3 build > (because libpqxx is being delivered in the 8.3 packages). But you've > placed the source in it's own directory "postgres/libpqxx". Wouldn't it > make more sense for libpqxx to be in the postgres/postgresql-8.3 > directory? That's how libpq is built & delivered.
Ah, but libpq is an integral part of the PostgreSQL product, libpqxx is developed independently. But I agree, it could also make sense to put it inside the postgresql-8.3 directory. > But maybe, you've decided to release libpqxx with multiple versions in > future (e.g. 8.3 & 8.4) and thought this is the best this way? Have you > thought about libpqxx with multiple versions of postgres? And how is > this reflected in your directory structure & Makefile? Have thought about it but it's not reflected, figured this could wait until we actually build for 8.4. > 2) Your webrev doesn't show libpqxx-2.6.9.tar.gz as a new file. Is the > webrev correct? Are there other missing files? Deliberately left out due to its size, there is nothing interesing for the reviewer to examine. There are no other missing files. > 3) usr/src/cmd/postgres/libpqxx/Makefile.sfw > > This doesn't look like it could easily support building libpqxx for > multiple versions of postgres. It looks more like the libpqxx targets > should be part of each usr/src/cmd/postgres/postgresql-?.?/Makefile.sfw > file. This is especially true given that the protofix in > postgres/postgresql-8.3/Makefile.sfw will fail when run before libpqxx > is built (which is always the case with nightly). Yes this is a problem. OK, I will put libpqxx under postgresql-8.3. Build of this cannot be done until PostgreSQL itself has been *installed* but I can handle that through proper make rules. Will keep it in a separate subdirectory. Will also inherit some variable settings so that when it's time for 8.4, we don't have to duplicate libpqxx but can use a softlink. > 4) usr/src/cmd/postgres/libpqxx/install-sfw > > You shouldn't replace "64" with the with actual directory name. Why is > this necessary? All references to 64-bit directories should be with the > name "64" (because then it's platform independent). The symbolic links > of 64->amd64 & 64->sparcv9 does the work of finding the right file. I just looked at some installed files on a Nevada machine and saw the canonical paths with "sparcv9". But I have no reason to beleive it's really necessary. > 5) usr/src/pkgdefs/SUNWpostgr-83-libs/prototype_com > > Includes file usr/postgres/8.3/lib/libpqxx.la. According to > http://filext.com/file-extension/la, this is a file that's used by the > GNU libtool during linking. So I assume the file is necessary if people > are using libpqxx.so with the GNU compiler/linker. Is that anticipated? > Do we even have a GNU libtool on Solaris? Again, I see quite a few of these files with libraries under /usr/sfw/lib so I didn't want to assume they were unneccesary. I figure it's safer to include a small file which may not be needed, than to risk complaints because someone did need it. [Also, the build for the regression test suite that comes with libpqxx use this file so if it isn't installed with the package, I'd have to do some hack to be able to run the regression tests. So there is a case of a system that does need this file.] > You have included usr/postgres/8.3/bin/pqxx-config in the -libs package. > But pg_config is already in the -devel package. It would make more sense > (and would be more consistent) to put pqxx-config (and it's libpqxx.pc > files) in the -devel package. Ah, I didn't know pg_config was in -devel, I will move pqxx-config. > 6) libpqxx.so is linked with the C++ libraries: > > libCstd.so.1 => /usr/lib/libCstd.so.1 > libCrun.so.1 => /usr/lib/libCrun.so.1 > > These are part of the SUNWlibC package, so you need to update the depend > files of the postgres packages you are changing. Will do. - Bjorn
