Nils,
Here are a few more comments ...
1. usr/src/cmd/postgres/postgresql-8.3/METADATA
Your need to add the following lines ..
URL: <web link to source site>
SUPPORT: ???
BUGTRAQ: solaris/utility/???
2. usr/src/cmd/postgres/postgresql-8.3/Makefile.sfw
Typo (out -> our) ...
41 # out package name is postgres
Lines 303 + 304, combines these into a single rm line.
3. usr/src/cmd/postgres/postgresql-8.3/install-sfw
& usr/src/cmd/postgres/postgresql-8.3/install-sfw-64
The directories created in here should be in the
Targetdirs file, then they don't need creating here.
4. usr/src/cmd/postgres/postgresql-8.3/libpqxx/Makefile.sfw
Lines 161 + 162, combines these into a single rm line.
5. usr/src/cmd/postgres/postgresql-8.3/libpqxx/Makefile.sfw
Do you need the "| tee make.out" bits?
6. usr/src/pkgdefs/SUNWpostgr-83-client-root/copyright
Is this the correct content?
7. usr/src/pkgdefs/SUNWpostgr-83-client-root/depend
This looks like the default depend file - if you have no
other dependencies then delete this file and add 'depend'
to the DATAFILES= line in
SUNWpostgr-83-client-root/Makefile.
If you keep it you should move the 'Copyright' lines to
after the 'CDDL HEADER END' header.
8. usr/src/pkgdefs/SUNWpostgr-83-client-root/Makefile
Are these 'i.rbac r.rbac' required on the DATAFILES= line?
Paul
Nils Goroll wrote:
> 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
>
--
----------------------------------------------------------------------
Paul Cunningham
Software Engineer
Tadpole Computer Products
