Re: patch to fix client only builds

I'm trying to do client only builds on a bunch of legacy platforms and
noticed that the include path is messed up...if keywords.o is not
already built, it fails to build be because src/backend/parser but not
src/backend is in the include path. (keywords.c includes
parser/gram.h).

The following fixes it. Probably not the right thing exactly but it works:

Index: Makefile
===================================================================
RCS file: /projects/cvsroot/pgsql/src/backend/parser/Makefile,v
retrieving revision 1.48
diff -r1.48 Makefile
13c13
< override CPPFLAGS := -I$(srcdir) $(CPPFLAGS)
---
> override CPPFLAGS := -I$(subdir) -I.. $(CPPFLAGS)

This would be a nice backpatch to 8.3 (and possibly earlier, I didn't check).

merlin

"Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
> I'm trying to do client only builds on a bunch of legacy platforms and
> noticed that the include path is messed up...if keywords.o is not
> already built, it fails to build be because src/backend/parser but not
> src/backend is in the include path. (keywords.c includes
> parser/gram.h).

Hmm, but nobody should be including gram.h directly out of
backend/parser anyway. They should be getting it via the symlink in
src/include/parser. I think the real problem must be that that symlink
isn't being created during a client-only build?

regards, tom lane

On Thu, Nov 6, 2008 at 11:09 AM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
>> I'm trying to do client only builds on a bunch of legacy platforms and
>> noticed that the include path is messed up...if keywords.o is not
>> already built, it fails to build be because src/backend/parser but not
>> src/backend is in the include path. (keywords.c includes
>> parser/gram.h).
>
> Hmm, but nobody should be including gram.h directly out of
> backend/parser anyway. They should be getting it via the symlink in
> src/include/parser. I think the real problem must be that that symlink
> isn't being created during a client-only build?

ah, correct. the symlink is setup in src/backend/Makefile

psql Makefile pulls in submake-backend to grab keywords.o.

submake-backend:
$(MAKE) -C $(top_builddir)/src/backend/parser keywords.o

so psql build is trying to build directly in backend which breaks (so
is pg_dump, the offender is a backend dependency in pg_dump from
symlinked in dumputils.c). This looks caused by a change to clean up
some quoting issues:

revision 1.36
date: 2007/06/18 21:40:58; author: tgl; state: Exp; lines: +18 -5
Arrange for quote_identifier() and pg_dump to not quote keywords that are
unreserved according to the grammar. The list of unreserved words has gotten
extensive enough that the unnecessary quoting is becoming a bit of an eyesore.
To do this, add knowledge of the keyword category to keywords.c's table.
(Someday we might be able to generate keywords.c's table and the keyword lists
in gram.y from a common source.)
<snip>

IMO, the client only build should be fixed, so we can:
*) put the makefile hack I proposed in
*) implement the keywords.c change suggested above
*) set up backend/parser symlink from different/additional place.

merlin

"Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
> This looks caused by a change to clean up
> some quoting issues:

No, that patch is unrelated --- it didn't modify the inclusion situation
at all.

regards, tom lane

On Thu, Nov 6, 2008 at 12:26 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
>> This looks caused by a change to clean up
>> some quoting issues:
>
> No, that patch is unrelated --- it didn't modify the inclusion situation
> at all.

oop....right again....cvs annotate claims psql Makefile was modified
1.56 (dec-05)? It's not completely clear why. It doesn't really
matter...the dependency is clearly there.

merlin

"Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
> IMO, the client only build should be fixed, so we can:
> *) put the makefile hack I proposed in
> *) implement the keywords.c change suggested above
> *) set up backend/parser symlink from different/additional place.

The last of these seems the correct fix. A minimal change would be
something like

submake-backend:
+ $(MAKE) -C $(top_builddir)/src/backend $(top_builddir)/src/include/parser/gram.h
$(MAKE) -C $(top_builddir)/src/backend/parser keywords.o

(in at least three different Makefiles: pg_dump, psql, scripts). But I
can't help feeling that some Makefile-refactoring seems called for here.
Peter, what do you think?

regards, tom lane

On Thu, Nov 6, 2008 at 12:45 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> "Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
>> IMO, the client only build should be fixed, so we can:
>> *) put the makefile hack I proposed in
>> *) implement the keywords.c change suggested above
>> *) set up backend/parser symlink from different/additional place.
>
> The last of these seems the correct fix. A minimal change would be
> something like
>
> submake-backend:
> + $(MAKE) -C $(top_builddir)/src/backend $(top_builddir)/src/include/parser/gram.h
> $(MAKE) -C $(top_builddir)/src/backend/parser keywords.o
>

Noticed another issue with 'client only' type installs in the
Makefile. Shouldn't make install on libpq install postgres_ext.h in
addition to the interface headers?

install: all installdirs install-lib
$(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
$(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)'
$(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
$(INSTALL_DATA) $(srcdir)/pg_service.conf.sample
'$(DESTDIR)$(datadir)/pg_service.conf.sample'

merlin

"Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
> Noticed another issue with 'client only' type installs in the
> Makefile. Shouldn't make install on libpq install postgres_ext.h in
> addition to the interface headers?

I don't think libpq's Makefile has any business doing that.

Maybe we should bite the bullet and provide some kind of top-level make
targets for client-only build/install.

regards, tom lane

Tom Lane wrote:
> Maybe we should bite the bullet and provide some kind of top-level make
> targets for client-only build/install.

That would probably work best.

Did we address this?

---------------------------------------------------------------------------

Merlin Moncure wrote:
> On Thu, Nov 6, 2008 at 12:45 PM, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us> wrote:
> > "Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
> >> IMO, the client only build should be fixed, so we can:
> >> *) put the makefile hack I proposed in
> >> *) implement the keywords.c change suggested above
> >> *) set up backend/parser symlink from different/additional place.
> >
> > The last of these seems the correct fix. A minimal change would be
> > something like
> >
> > submake-backend:
> > + $(MAKE) -C $(top_builddir)/src/backend $(top_builddir)/src/include/parser/gram.h
> > $(MAKE) -C $(top_builddir)/src/backend/parser keywords.o
> >
>
> Noticed another issue with 'client only' type installs in the
> Makefile. Shouldn't make install on libpq install postgres_ext.h in
> addition to the interface headers?
>
> install: all installdirs install-lib
> $(INSTALL_DATA) $(srcdir)/libpq-fe.h '$(DESTDIR)$(includedir)'
> $(INSTALL_DATA) $(srcdir)/libpq-events.h '$(DESTDIR)$(includedir)'
> $(INSTALL_DATA) $(srcdir)/libpq-int.h '$(DESTDIR)$(includedir_internal)'
> $(INSTALL_DATA) $(srcdir)/pqexpbuffer.h '$(DESTDIR)$(includedir_internal)'
> $(INSTALL_DATA) $(srcdir)/pg_service.conf.sample
> '$(DESTDIR)$(datadir)/pg_service.conf.sample'
>
> merlin
>
> --
> Sent via pgsql-hackers mailing list (pgsql-hackers(at)postgresql(dot)org)
> To make changes to your subscription:
> http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

Tom Lane wrote:
> "Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
> > Noticed another issue with 'client only' type installs in the
> > Makefile. Shouldn't make install on libpq install postgres_ext.h in
> > addition to the interface headers?
>
> I don't think libpq's Makefile has any business doing that.
>
> Maybe we should bite the bullet and provide some kind of top-level make
> targets for client-only build/install.

Is someone going to handle this?

--
Bruce Momjian <bruce(at)momjian(dot)us> http://momjian.us
EnterpriseDB http://enterprisedb.com

+ If your life is a hard drive, Christ can be your backup. +

On Wed, Feb 4, 2009 at 1:22 PM, Bruce Momjian <bruce(at)momjian(dot)us> wrote:
> Tom Lane wrote:
>> "Merlin Moncure" <mmoncure(at)gmail(dot)com> writes:
>> > Noticed another issue with 'client only' type installs in the
>> > Makefile. Shouldn't make install on libpq install postgres_ext.h in
>> > addition to the interface headers?
>>
>> I don't think libpq's Makefile has any business doing that.
>>
>> Maybe we should bite the bullet and provide some kind of top-level make
>> targets for client-only build/install.
>
> Is someone going to handle this?

I'll take a look. I'm not very good with makefiles...

merlin