Thread: Makefile for parser

I've started doing a bit of work on gram.y, and am noticing some new
cruftiness in the Makefile: if I add tokens to gram.y/keywords.c then I
can't just remake in that directory since parse.h is not updated
elsewhere in the tree.

I believe that the Makefile used to reach up and over in the tree to
update parse.h, and I'll guess that this fell victim to a general
cleanup (looks like Tom Lane and Peter E. have been working with the
Makefiles, but I haven't tracked down the details).

Any suggestions for a fixup? In general, I agree that having Makefiles
muck with stuff elsewhere in the tree is a Bad Idea, but it would be
nice if each directory could be built on its own.
                   - Thomas

Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
> I've started doing a bit of work on gram.y, and am noticing some new
> cruftiness in the Makefile: if I add tokens to gram.y/keywords.c then I
> can't just remake in that directory since parse.h is not updated
> elsewhere in the tree.

Uh ... what's your point?  If the changes to parse.h affect anything
else then you ought to be doing a top-level make --- or at the very
least a make in src/backend --- and that will rebuild
include/parser/parse.h.
        regards, tom lane

> > I've started doing a bit of work on gram.y, and am noticing some new
> > cruftiness in the Makefile: if I add tokens to gram.y/keywords.c 
> > then I can't just remake in that directory since parse.h is not 
> > updated elsewhere in the tree.
> Uh ... what's your point?  If the changes to parse.h affect anything
> else then you ought to be doing a top-level make --- or at the very
> least a make in src/backend --- and that will rebuild
> include/parser/parse.h.

Any change to gram.y regenerates the local copy of parse.h and affects
other files *in that local directory* (as well as elsewhere). The
makefile *in that local directory* should be able to make the other
files *in that same directory* at the same time.

That's my point ;)

istm that the local makefile should at least reach up and over to the
other rule for building parse.h (wherever that is), so that parse.h gets
moved to the include/ area. If make is invoked from the top of the tree,
then it is a noop. If make is invoked from backend/parser/, then the
local files get rebuilt correctly.
                    - Tom

Thomas Lockhart <lockhart@alumni.caltech.edu> writes:
>> Uh ... what's your point?

> Any change to gram.y regenerates the local copy of parse.h and affects
> other files *in that local directory* (as well as elsewhere). The
> makefile *in that local directory* should be able to make the other
> files *in that same directory* at the same time.

Oh, right, the files in that directory are going to include parse.h
from the include dir now, instead of ".", aren't they?  I see your
problem.

Probably the rule that installs parse.h into the include tree ought to
be pushed down from backend/Makefile to backend/parser/Makefile (but
backend/Makefile still needs to invoke it during its prebuildheaders
phase).  Maybe likewise for fmgroids.h into backend/utils.

Peter, any thoughts here?
        regards, tom lane

> Oh, right, the files in that directory are going to include parse.h
> from the include dir now, instead of ".", aren't they?  I see your
> problem.

Right.

> Probably the rule that installs parse.h into the include tree ought to
> be pushed down from backend/Makefile to backend/parser/Makefile (but
> backend/Makefile still needs to invoke it during its prebuildheaders
> phase).  Maybe likewise for fmgroids.h into backend/utils.

I've got (simple) patches which do this for parse.h. I can see why this
was pushed up, since it is not very clean to have Makefiles putting
their fingers into other places in the tree. But for this case I don't
see a way out.

Peter E?
                      - Thomas

Tom Lane writes:

> > I've started doing a bit of work on gram.y, and am noticing some new
> > cruftiness in the Makefile: if I add tokens to gram.y/keywords.c then I
> > can't just remake in that directory since parse.h is not updated
> > elsewhere in the tree.
> 
> Uh ... what's your point?  If the changes to parse.h affect anything
> else then you ought to be doing a top-level make --- or at the very
> least a make in src/backend --- and that will rebuild
> include/parser/parse.h.

I'm having a feeling that this will not work too well with parallel
make. Every directory needs to know how to make all the files that it
needs. For the case of parse.h it would not be too difficult to teach the
few places that need it:

src/backend$ find -name '*.c' | xargs fgrep 'parse.h' | fgrep -v './parser/'
./commands/command.c:#include "parser/parse.h"
./commands/comment.c:#include "parser/parse.h"
./nodes/outfuncs.c:#include "parser/parse.h"
./tcop/utility.c:#include "parser/parse.h"

fmgroids.h on the other hand would be trickier. We might need a
backend/Makefile.inc (perhaps as a wrapper around Makefile.global) to do
it right. But I haven't gotten to the backend tree at all yet.


-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden

Thomas Lockhart writes:

> I've got (simple) patches which do this for parse.h.

Please feel free to do whatever helps you right now. I haven't gotten to
the backend tree yet.


-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden

On the topic of make, have you all read "Recursive Make Considered
Harmful" at http://www.tip.net.au/~millerp/rmch/recu-make-cons-harm.html

He makes a good point that most people write makefiles wrong. Certainly
the pgsql makefiles are broken for parallel make.

-- 
Chris Bitmead
mailto:chris@bitmead.com
Peter Eisentraut wrote:
> 
> Tom Lane writes:
> 
> > > I've started doing a bit of work on gram.y, and am noticing some new
> > > cruftiness in the Makefile: if I add tokens to gram.y/keywords.c then I
> > > can't just remake in that directory since parse.h is not updated
> > > elsewhere in the tree.
> >
> > Uh ... what's your point?  If the changes to parse.h affect anything
> > else then you ought to be doing a top-level make --- or at the very
> > least a make in src/backend --- and that will rebuild
> > include/parser/parse.h.
> 
> I'm having a feeling that this will not work too well with parallel
> make. Every directory needs to know how to make all the files that it
> needs. For the case of parse.h it would not be too difficult to teach the
> few places that need it:
> 
> src/backend$ find -name '*.c' | xargs fgrep 'parse.h' | fgrep -v './parser/'
> ./commands/command.c:#include "parser/parse.h"
> ./commands/comment.c:#include "parser/parse.h"
> ./nodes/outfuncs.c:#include "parser/parse.h"
> ./tcop/utility.c:#include "parser/parse.h"
> 
> fmgroids.h on the other hand would be trickier. We might need a
> backend/Makefile.inc (perhaps as a wrapper around Makefile.global) to do
> it right. But I haven't gotten to the backend tree at all yet.
> 
> --
> Peter Eisentraut                  Sernanders väg 10:115
> peter_e@gmx.net                   75262 Uppsala
> http://yi.org/peter-e/            Sweden

Chris Bitmead <chris@bitmead.com> writes:
> On the topic of make, have you all read "Recursive Make Considered
> Harmful" at http://www.tip.net.au/~millerp/rmch/recu-make-cons-harm.html

I read it, I don't believe a word of it.  The whole thing is founded on
a bogus example, to which is added specious reasoning and an assumption
that everyone wants to use GCC as compiler plus a nonstandardly-patched
version of GNU make.  This is not the real world.

The Postgres build setup is certainly far from ideal, but IMHO the only
thing *really* wrong with it is that we're not constructing accurate
dependency lists by default.  I believe Peter E. is planning to
fix that...
        regards, tom lane

Tom Lane wrote:
> 
> Chris Bitmead <chris@bitmead.com> writes:
> > On the topic of make, have you all read "Recursive Make Considered
> > Harmful" at http://www.tip.net.au/~millerp/rmch/recu-make-cons-harm.html
> 
> I read it, I don't believe a word of it.  The whole thing is founded on
> a bogus example, to which is added specious reasoning

?

> and an assumption
> that everyone wants to use GCC as compiler plus a nonstandardly-patched
> version of GNU make.  This is not the real world.

It doesn't depend on using gcc. The GNU make patch referred to was put
into the core GNU make distribution a long time ago.

> The Postgres build setup is certainly far from ideal, but IMHO the only
> thing *really* wrong with it is that we're not constructing accurate
> dependency lists by default.  I believe Peter E. is planning to
> fix that...

It is pretty nice if you use his recommendation to be able to type make
at the top level and be told immediately that everything is up to date
rather than seeing 10 pages of messages scroll past. I think you've
dismissed him a little easily about the problems of properly specifying
dependancies with recursive make.

Tom Lane writes:

> > On the topic of make, have you all read "Recursive Make Considered
> > Harmful" at http://www.tip.net.au/~millerp/rmch/recu-make-cons-harm.html
> 
> I read it, I don't believe a word of it.

Not only do I believe some words of it, I've had essentially the same
thoughts for quite a while. No, certainly there should not be a single
makefile for all of PostgreSQL. I'd certainly like to be able to do `make
-C src/bin/psql install'. But for the backend tree where you only build
one program this certainly would make sense. And note that the author's
example is essentially the same we've been talking about: a parse.h file
with incomplete dependencies.

> The whole thing is founded on a bogus example, to which is added
> specious reasoning and an assumption that everyone wants to use GCC as
> compiler

Well, you would need a compiler that handles -c and -o at the same time,
but you can always use cc -c && mv if that doesn't work. I think GNU make
might even do that by default, but I'd have to check.

> The Postgres build setup is certainly far from ideal, but IMHO the only
> thing *really* wrong with it is that we're not constructing accurate
> dependency lists by default.

Accurate dependencies are one thing, actually knowing how to satisfy these
dependencies is another. If the dependencies say that file X depends on
fmgroids.h but no further information about fmgroids.h is provided then it
will fail if it doesn't exist, or assume on mere existance that it is up
to date. This is again exactly what this guy is talking about.


Concluding, I don't know how well the suggested setup would work. I
haven't tried it, but I certainly will. In any case there's got to be
something better than maintaining 50+ makefiles that all do the same thing
and all do the same thing wrong.


-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden

Chris Bitmead writes:

> Certainly the pgsql makefiles are broken for parallel make.

I think of "broken" for parallel make if it doesn't work at all, which
certainly needs to be fixed. "Unsupportive" of parallel make are things
like this:

DIRS := initdb initlocation ipcclean pg_ctl pg_dump pg_id \       pg_passwd pg_version psql scripts

all:       @for dir in $(DIRS); do $(MAKE) -C $$dir $@ || exit 1; done

because no matter how smart make is, the loop will still execute
sequentially.

But parallel make can co-exist with recursive make, like this:

DIRS := initdb initlocation ipcclean pg_ctl pg_dump pg_id \                              pg_passwd pg_version psql
scripts

all: $(DIRS:%=%-all-recursive)

.PHONY: $(DIRS:%=%-all-recursive)

$(DIRS:%=%-all-recursive):$(MAKE) -C $(subst -all-recursive,,$@) all


Then again, if you want faster builds, use -pipe. I'd like to make that
the default but I haven't found a reliable way to test for it. GCC doesn't
reject invalid switches in a detectable manner.

-- 
Peter Eisentraut                  Sernanders väg 10:115
peter_e@gmx.net                   75262 Uppsala
http://yi.org/peter-e/            Sweden