patch to fix client only builds

Started by Merlin Moncureabout 17 years ago12 messages

mmoncure@gmail.com

about 17 years ago

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

Tom Lane

tgl@sss.pgh.pa.us

about 17 years ago

In reply to: Merlin Moncure (#1)

Re: patch to fix client only builds

"Merlin Moncure" <mmoncure@gmail.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

Merlin Moncure

mmoncure@gmail.com

about 17 years ago

In reply to: Tom Lane (#2)

Re: patch to fix client only builds

On Thu, Nov 6, 2008 at 11:09 AM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Merlin Moncure" <mmoncure@gmail.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

Tom Lane

tgl@sss.pgh.pa.us

about 17 years ago

In reply to: Merlin Moncure (#3)

Re: patch to fix client only builds

"Merlin Moncure" <mmoncure@gmail.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

Merlin Moncure

mmoncure@gmail.com

about 17 years ago

In reply to: Tom Lane (#4)

Re: patch to fix client only builds

On Thu, Nov 6, 2008 at 12:26 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Merlin Moncure" <mmoncure@gmail.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

Tom Lane

tgl@sss.pgh.pa.us

about 17 years ago

In reply to: Merlin Moncure (#3)

Re: patch to fix client only builds

"Merlin Moncure" <mmoncure@gmail.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

Merlin Moncure

mmoncure@gmail.com

about 17 years ago

In reply to: Tom Lane (#6)

Re: patch to fix client only builds

On Thu, Nov 6, 2008 at 12:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Merlin Moncure" <mmoncure@gmail.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

Tom Lane

tgl@sss.pgh.pa.us

about 17 years ago

In reply to: Merlin Moncure (#7)

Re: patch to fix client only builds

"Merlin Moncure" <mmoncure@gmail.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

Peter Eisentraut

peter_e@gmx.net

about 17 years ago

In reply to: Tom Lane (#8)

Re: patch to fix client only builds

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.

#10

Bruce Momjian

bruce@momjian.us

about 17 years ago

In reply to: Merlin Moncure (#7)

Re: patch to fix client only builds

Did we address this?

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

Merlin Moncure wrote:

On Thu, Nov 6, 2008 at 12:45 PM, Tom Lane <tgl@sss.pgh.pa.us> wrote:

"Merlin Moncure" <mmoncure@gmail.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@postgresql.org)
To make changes to your subscription:
http://www.postgresql.org/mailpref/pgsql-hackers

--
Bruce Momjian <bruce@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#11

Bruce Momjian

bruce@momjian.us

almost 17 years ago

In reply to: Tom Lane (#8)

Re: patch to fix client only builds

Tom Lane wrote:

"Merlin Moncure" <mmoncure@gmail.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@momjian.us> http://momjian.us
EnterpriseDB http://enterprisedb.com

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

#12

Merlin Moncure

mmoncure@gmail.com

almost 17 years ago

In reply to: Bruce Momjian (#11)

Re: patch to fix client only builds

On Wed, Feb 4, 2009 at 1:22 PM, Bruce Momjian <bruce@momjian.us> wrote:

Tom Lane wrote:

"Merlin Moncure" <mmoncure@gmail.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