Hi Paul, Mayuresh and all, what started as a little change to fix a specific requirement has now evolved into a general overhaul of the SFW postgres Make- and install files.
I did this because Paul suggested to use a METADATA file, and looking at other Makefiles in the sfw consolidation, I realized that the postgres build environment would profit from a general overhaul to improve clarity and to reduce the amount of duplicate information. The webrev is at http://cr.opensolaris.org/~nigoroll/postgres_overhaul_and_defect3298_missing_pgxs_files-rd3/index.html I suggest to do the following: * I will comment on all changes I made below, please let me know what you think and review them. * If necessary, additional bugs could be opened for the various changes (e.g. "postgres makefiles need overhaul" or "64bit bin and lib should be ${ARCH64}") * So far I have neither tested the changes on sparcv9, nor have I built packages and installed them (there should be an easy way to build packages which I don't know - how?). Would anyone help with these? * After we have agreed on what should be integrated and after changes due to reviews have been made, we should have a final review phase. I will first reply to Pauls and Mayureshs comments and then give a general overview of all changes. Paul Cunningham wrote: > Normally the developer would checkin their changes into their local ws > before generating the webrev - thus they get changed. But I'm not sure > how you are working outside the SWAN so you may not be able to do that. Sun internal teamware repositories are not accessible from outside SWAN and I don't see any sense in using another SCM just for myself, so I use zfs snapshots as a rudimentary replacement for an SCM. #ident tags will get updated when a sponsor checks in the changes, so I don't understand why I should change them just for the webrev. > Roland Mainz wrote: > > - 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) done > > - use either $(SHELL) or /usr/bin/bash for "configure" > > calls (so we know which one is used and "configure" > > doesn't pick one itself) done > > Use the method define in Makefile.master > > since you have a standard METADATA file. done, see below for more. And I still think we should have Metadata files in Makefile format. Mayuresh Nirhali wrote: > 1. SFW consolidation also includes version 8.2 of postgres and the bug > 3298 also applies to that version. > I think it makes sense to fix 8.2 as well with the fix for this bug. Let > me know what you think. Absolutely. I would very much like to get this done and integrated for 8.3 fist and would then implement the same or similar changes for 8.2 as well. At this point, maintaining two sets of almost identical changes would just mean more work. > 2. I see following files in the 32 bit pgxs directory, please check if > they should be present in 64 bit directories as well. > d none usr/postgres/8.3/lib/pgxs/src/test 755 root bin > d none usr/postgres/8.3/lib/pgxs/src/test/regress 755 root bin > f none usr/postgres/8.3/lib/pgxs/src/test/regress/pg_regress 555 root bin done > 3. Another very minor comment, you should include following line in both > platform specific proto files. > d none usr/postgres/8.3/lib/amd64/pgxs 755 root bin oops, done Comments on the other changes / overhaul: === SYSCONFDIR === (diff from Makefile.global) -sysconfdir := /usr/postgres/8.3/etc +sysconfdir := /etc/pgsql/8.3 I don't think any config directory should be under /usr (will break with diskless clients for instance, see filesystem(5)), so I changed the path and introduced the new package SUNWpostgr-83-client-root, containing only the sample pgsql rc-file. I could come up with arguments for calling the directory both /etc/postgres and /etc/pgsql. pgsql is what postgres.org calls it, postgres is the name of the package. IMHO, we should stick to the names the original authors have chosen, so I think pgsql is better, but please let me know what you think. === CFLAGS === (diff from Makefile.global) CC = /opt/SunStudioExpress/bin/cc -Xa GCC = -CFLAGS = -xc99=none -xCC +CFLAGS = -xO3 -xarch=386 -xchip=pentium -xspace -Xa -xildoff -xc99=all -xc99=none -xCC In the oritinal Makefile.sfw, the default cflags were used everywhere (also for the 64bit version), BUT for the target $(VER)/config.status Thus it does look like an error that the (sensible) default CFLAGS are not used, but I might not be aware of a good reason for choosing the minimal CFLAGS. === bindir, libdir === Originally, bindir and libdir were chosen as .../64. My understanding is that those directories should be ../$(MACH64), either amd64 or sparcv9, which is what I've made them to be now. These directories existed already and .../64 was only a symlink, so the change is more making the build environment more consistent with the path name scheme. === Comments on changed files === * Makefile.master - added some path definitions which seemed to be missing - cleaned up CFGSFW definitions - added mussing definitions to ROOTSFW* (plus one fix) * usr/src/cmd/postgres/postgresql-8.3/Makefile.sfw - general cleanup, use macros for all environment settings and configure args * usr/src/cmd/postgres/postgresql-8.3/install-sfw * usr/src/cmd/postgres/postgresql-8.3/install-sfw-64 * usr/src/cmd/postgres/postgresql-8.3/libpqxx/install-sfw IMHO, all of the install scripts should really fail upon error, so I have changed their invocation to $(SH) -e. Made script ignore all uncritical errors. Fixed bogus error output for recreating symlinks by using resymlink function * usr/src/cmd/postgres/postgresql-8.3/libpqxx/Makefile.sfw - general cleanup
