dependencies for generated header files

Started by Robert Haasover 16 years ago18 messages
#1Robert Haas
robertmhaas@gmail.com
1 attachment(s)

Hi,

I think that our dependencies for generated header files (gram.h,
fmgroids.h, probes.h) are not as good as they could be. What we do
right now is make src/backend/Makefile rebuild these before recursing
through its subdirectories. This works OK for a top-level make, but
if you run make further down in the tree (like under
src/backend/commands) it won't necessarily rebuild everything that it
should.

The attached patch moves some of this logic from src/backend/Makefile
to src/Makefile.global.in. That way, if you --enable-depend and then
do something like "touch src/include/catalog/pg_proc.h" and then "cd
src/backend/commands; make vacuum.o", it rebuilds fmgroids.h and then
recompiles vacuum.c. Under HEAD, it just tells you that vacuum.o is
up to date.

I have tested this on vpath and non-vpath builds, with and without
--enable-depend.

...Robert

Attachments:

header_depend-v1.patchtext/x-diff; charset=US-ASCII; name=header_depend-v1.patchDownload
*** a/src/Makefile.global.in
--- b/src/Makefile.global.in
***************
*** 517,522 **** $(top_builddir)/src/interfaces/ecpg/include/ecpg_config.h: $(top_builddir)/src/i
--- 517,551 ----
  $(top_builddir)/config.status: $(top_srcdir)/configure
  	cd $(top_builddir) && ./config.status --recheck
  
+ # Generated header files need dependencies here to ensure that everything
+ # which depends on them gets rebuilt when necessary.  Note that it's important
+ # we match the dependencies shown in the subdirectory makefiles!
+ $(top_srcdir)/src/backend/parser/gram.h: $(top_srcdir)/src/backend/parser/gram.y
+ 	$(MAKE) -C $(top_builddir)/src/backend/parser gram.h
+ 
+ $(top_builddir)/src/backend/utils/fmgroids.h: $(top_srcdir)/src/backend/utils/Gen_fmgrtab.sh $(top_srcdir)/src/include/catalog/pg_proc.h
+ 	$(MAKE) -C $(top_builddir)/src/backend/utils fmgroids.h
+ 
+ $(top_builddir)/src/backend/utils/probes.h: $(top_srcdir)/src/backend/utils/probes.d
+ 	$(MAKE) -C $(top_builddir)/src/backend/utils probes.h
+ 
+ # Make symlinks for these headers in the include directory. That way
+ # we can cut down on the -I options. Also, a symlink is automatically
+ # up to date when we update the base file.
+ 
+ $(top_builddir)/src/include/parser/gram.h: $(top_srcdir)/src/backend/parser/gram.h
+ 	prereqdir=`cd $(dir $<) >/dev/null && pwd` && \
+ 	  cd $(dir $@) && rm -f $(notdir $@) && \
+ 	  $(LN_S) "$$prereqdir/$(notdir $<)" .
+ 
+ $(top_builddir)/src/include/utils/fmgroids.h: $(top_builddir)/src/backend/utils/fmgroids.h
+ 	cd $(dir $@) && rm -f $(notdir $@) && \
+ 	    $(LN_S) ../../../src/backend/utils/fmgroids.h .
+ 
+ $(top_builddir)/src/include/utils/probes.h: $(top_builddir)/src/backend/utils/probes.h
+ 	cd $(dir $@) && rm -f $(notdir $@) && \
+ 	    $(LN_S) ../../../src/backend/utils/probes.h .
+ 
  endif # not PGXS
  
  
*** a/src/backend/Makefile
--- b/src/backend/Makefile
***************
*** 113,150 **** $(SUBDIRS:%=%-recursive): $(top_builddir)/src/include/parser/gram.h $(top_buildd
  postgres.o: $(OBJS)
  	$(CC) $(LDREL) $(LDFLAGS) $(call expand_subsys,$^) $(LIBS) -o $@
  
- 
- # The following targets are specified in make commands that appear in
- # the make files in our subdirectories. Note that it's important we
- # match the dependencies shown in the subdirectory makefiles!
- 
- $(srcdir)/parser/gram.h: parser/gram.y
- 	$(MAKE) -C parser gram.h
- 
- utils/fmgroids.h: utils/Gen_fmgrtab.sh $(top_srcdir)/src/include/catalog/pg_proc.h
- 	$(MAKE) -C utils fmgroids.h
- 
- utils/probes.h: utils/probes.d
- 	$(MAKE) -C utils probes.h
- 
- # Make symlinks for these headers in the include directory. That way
- # we can cut down on the -I options. Also, a symlink is automatically
- # up to date when we update the base file.
- 
- $(top_builddir)/src/include/parser/gram.h: $(srcdir)/parser/gram.h
- 	prereqdir=`cd $(dir $<) >/dev/null && pwd` && \
- 	  cd $(dir $@) && rm -f $(notdir $@) && \
- 	  $(LN_S) "$$prereqdir/$(notdir $<)" .
- 
- $(top_builddir)/src/include/utils/fmgroids.h: utils/fmgroids.h
- 	cd $(dir $@) && rm -f $(notdir $@) && \
- 	    $(LN_S) ../../../$(subdir)/utils/fmgroids.h .
- 
- $(top_builddir)/src/include/utils/probes.h: utils/probes.h
- 	cd $(dir $@) && rm -f $(notdir $@) && \
- 	    $(LN_S) ../../../$(subdir)/utils/probes.h .
- 
- 
  ifeq ($(PORTNAME), solaris)
  utils/probes.o: utils/probes.d $(SUBDIROBJS)
  	$(DTRACE) $(DTRACEFLAGS) -C -G -s $(call expand_subsys,$^) -o $@
--- 113,118 ----
#2Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#1)
1 attachment(s)
Re: dependencies for generated header files

On Sun, Jun 28, 2009 at 2:21 PM, Robert Haas<robertmhaas@gmail.com> wrote:

I think that our dependencies for generated header files (gram.h,
fmgroids.h, probes.h) are not as good as they could be.  What we do
right now is make src/backend/Makefile rebuild these before recursing
through its subdirectories.  This works OK for a top-level make, but
if you run make further down in the tree (like under
src/backend/commands) it won't necessarily rebuild everything that it
should.

The attached patch moves some of this logic from src/backend/Makefile
to src/Makefile.global.in.  That way, if you --enable-depend and then
do something like "touch src/include/catalog/pg_proc.h" and then "cd
src/backend/commands; make vacuum.o", it rebuilds fmgroids.h and then
recompiles vacuum.c.  Under HEAD, it just tells you that vacuum.o is
up to date.

I have tested this on vpath and non-vpath builds, with and without
--enable-depend.

Woops. It seems that patch generates some warnings on a vpath build
which I failed to notice. Corrected version that guards against same
is attached.

...Robert

Attachments:

header_depend-v2.patchtext/x-diff; charset=US-ASCII; name=header_depend-v2.patchDownload
*** a/src/Makefile.global.in
--- b/src/Makefile.global.in
***************
*** 517,522 **** $(top_builddir)/src/interfaces/ecpg/include/ecpg_config.h: $(top_builddir)/src/i
--- 517,555 ----
  $(top_builddir)/config.status: $(top_srcdir)/configure
  	cd $(top_builddir) && ./config.status --recheck
  
+ # Generated header files need dependencies here to ensure that everything
+ # which depends on them gets rebuilt when necessary.  Note that it's important
+ # we match the dependencies shown in the subdirectory makefiles!
+ ifneq ($(subdir),src/backend/parser)
+ $(top_srcdir)/src/backend/parser/gram.h: $(top_srcdir)/src/backend/parser/gram.y
+ 	$(MAKE) -C $(top_builddir)/src/backend/parser gram.h
+ endif
+ 
+ ifneq ($(subdir),src/backend/utils)
+ $(top_builddir)/src/backend/utils/fmgroids.h: $(top_srcdir)/src/backend/utils/Gen_fmgrtab.sh $(top_srcdir)/src/include/catalog/pg_proc.h
+ 	$(MAKE) -C $(top_builddir)/src/backend/utils fmgroids.h
+ 
+ $(top_builddir)/src/backend/utils/probes.h: $(top_srcdir)/src/backend/utils/probes.d
+ 	$(MAKE) -C $(top_builddir)/src/backend/utils probes.h
+ endif
+ 
+ # Make symlinks for these headers in the include directory. That way
+ # we can cut down on the -I options. Also, a symlink is automatically
+ # up to date when we update the base file.
+ 
+ $(top_builddir)/src/include/parser/gram.h: $(top_srcdir)/src/backend/parser/gram.h
+ 	prereqdir=`cd $(dir $<) >/dev/null && pwd` && \
+ 	  cd $(dir $@) && rm -f $(notdir $@) && \
+ 	  $(LN_S) "$$prereqdir/$(notdir $<)" .
+ 
+ $(top_builddir)/src/include/utils/fmgroids.h: $(top_builddir)/src/backend/utils/fmgroids.h
+ 	cd $(dir $@) && rm -f $(notdir $@) && \
+ 	    $(LN_S) ../../../src/backend/utils/fmgroids.h .
+ 
+ $(top_builddir)/src/include/utils/probes.h: $(top_builddir)/src/backend/utils/probes.h
+ 	cd $(dir $@) && rm -f $(notdir $@) && \
+ 	    $(LN_S) ../../../src/backend/utils/probes.h .
+ 
  endif # not PGXS
  
  
*** a/src/backend/Makefile
--- b/src/backend/Makefile
***************
*** 113,150 **** $(SUBDIRS:%=%-recursive): $(top_builddir)/src/include/parser/gram.h $(top_buildd
  postgres.o: $(OBJS)
  	$(CC) $(LDREL) $(LDFLAGS) $(call expand_subsys,$^) $(LIBS) -o $@
  
- 
- # The following targets are specified in make commands that appear in
- # the make files in our subdirectories. Note that it's important we
- # match the dependencies shown in the subdirectory makefiles!
- 
- $(srcdir)/parser/gram.h: parser/gram.y
- 	$(MAKE) -C parser gram.h
- 
- utils/fmgroids.h: utils/Gen_fmgrtab.sh $(top_srcdir)/src/include/catalog/pg_proc.h
- 	$(MAKE) -C utils fmgroids.h
- 
- utils/probes.h: utils/probes.d
- 	$(MAKE) -C utils probes.h
- 
- # Make symlinks for these headers in the include directory. That way
- # we can cut down on the -I options. Also, a symlink is automatically
- # up to date when we update the base file.
- 
- $(top_builddir)/src/include/parser/gram.h: $(srcdir)/parser/gram.h
- 	prereqdir=`cd $(dir $<) >/dev/null && pwd` && \
- 	  cd $(dir $@) && rm -f $(notdir $@) && \
- 	  $(LN_S) "$$prereqdir/$(notdir $<)" .
- 
- $(top_builddir)/src/include/utils/fmgroids.h: utils/fmgroids.h
- 	cd $(dir $@) && rm -f $(notdir $@) && \
- 	    $(LN_S) ../../../$(subdir)/utils/fmgroids.h .
- 
- $(top_builddir)/src/include/utils/probes.h: utils/probes.h
- 	cd $(dir $@) && rm -f $(notdir $@) && \
- 	    $(LN_S) ../../../$(subdir)/utils/probes.h .
- 
- 
  ifeq ($(PORTNAME), solaris)
  utils/probes.o: utils/probes.d $(SUBDIROBJS)
  	$(DTRACE) $(DTRACEFLAGS) -C -G -s $(call expand_subsys,$^) -o $@
--- 113,118 ----
#3Alvaro Herrera
alvherre@commandprompt.com
In reply to: Robert Haas (#2)
Re: dependencies for generated header files

Robert Haas escribi�:

Woops. It seems that patch generates some warnings on a vpath build
which I failed to notice. Corrected version that guards against same
is attached.

Interestingly, this patch causes diffstat (at least my version of it) to
attribute the line additions to src/backend/catalog/dependency.c instead
of the new file ...

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#4Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#1)
Re: dependencies for generated header files

On Sunday 28 June 2009 21:21:35 Robert Haas wrote:

I think that our dependencies for generated header files (gram.h,
fmgroids.h, probes.h) are not as good as they could be. What we do
right now is make src/backend/Makefile rebuild these before recursing
through its subdirectories. This works OK for a top-level make, but
if you run make further down in the tree (like under
src/backend/commands) it won't necessarily rebuild everything that it
should.

The attached patch moves some of this logic from src/backend/Makefile
to src/Makefile.global.in.

Have you tried putting them in src/backend/common.mk? That might be a better
place.

Also, modulo that change, do you think it would be OK to apply this patch now
while the other patch about header generation is under discussion? Or would
that create a complete merge disaster if we ended up going with some variant
of the header generation patch?

#5Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#4)
Re: dependencies for generated header files

On Wed, Jul 29, 2009 at 8:43 AM, Peter Eisentraut<peter_e@gmx.net> wrote:

On Sunday 28 June 2009 21:21:35 Robert Haas wrote:

I think that our dependencies for generated header files (gram.h,
fmgroids.h, probes.h) are not as good as they could be.  What we do
right now is make src/backend/Makefile rebuild these before recursing
through its subdirectories.  This works OK for a top-level make, but
if you run make further down in the tree (like under
src/backend/commands) it won't necessarily rebuild everything that it
should.

The attached patch moves some of this logic from src/backend/Makefile
to src/Makefile.global.in.

Have you tried putting them in src/backend/common.mk?  That might be a better
place.

No, I haven't. I can look at that at some point, but I'm a bit backed
up right now.

Also, modulo that change, do you think it would be OK to apply this patch now
while the other patch about header generation is under discussion?  Or would
that create a complete merge disaster if we ended up going with some variant
of the header generation patch?

Well, since this one is only an 83-line patch, I don't think it will
be too bad. Of course if committing this gets you excited about
overhauling the Makefiles and you decide to change a bunch of other
things too, that might be more of an issue...

...Robert

#6Robert Haas
robertmhaas@gmail.com
In reply to: Robert Haas (#5)
Re: dependencies for generated header files

On Wed, Jul 29, 2009 at 9:28 AM, Robert Haas<robertmhaas@gmail.com> wrote:

On Wed, Jul 29, 2009 at 8:43 AM, Peter Eisentraut<peter_e@gmx.net> wrote:

On Sunday 28 June 2009 21:21:35 Robert Haas wrote:

I think that our dependencies for generated header files (gram.h,
fmgroids.h, probes.h) are not as good as they could be.  What we do
right now is make src/backend/Makefile rebuild these before recursing
through its subdirectories.  This works OK for a top-level make, but
if you run make further down in the tree (like under
src/backend/commands) it won't necessarily rebuild everything that it
should.

The attached patch moves some of this logic from src/backend/Makefile
to src/Makefile.global.in.

Have you tried putting them in src/backend/common.mk?  That might be a better
place.

No, I haven't.  I can look at that at some point, but I'm a bit backed
up right now.

[ looks ]

Is it conceivable that there might be a dependency on one of the
generated header files from someplace in src other than src/backend?
If so, it seems like that might be an argument for leaving it as I had
it.

...Robert

#7Alvaro Herrera
alvherre@commandprompt.com
In reply to: Robert Haas (#6)
Re: dependencies for generated header files

Robert Haas escribi�:

Is it conceivable that there might be a dependency on one of the
generated header files from someplace in src other than src/backend?
If so, it seems like that might be an argument for leaving it as I had
it.

Well, plperl.c includes fmgroids.h.

--
Alvaro Herrera http://www.CommandPrompt.com/
The PostgreSQL Company - Command Prompt, Inc.

#8Peter Eisentraut
peter_e@gmx.net
In reply to: Robert Haas (#1)
Re: dependencies for generated header files

On Sunday 28 June 2009 21:21:35 Robert Haas wrote:

I think that our dependencies for generated header files (gram.h,
fmgroids.h, probes.h) are not as good as they could be. What we do
right now is make src/backend/Makefile rebuild these before recursing
through its subdirectories. This works OK for a top-level make, but
if you run make further down in the tree (like under
src/backend/commands) it won't necessarily rebuild everything that it
should.

The attached patch moves some of this logic from src/backend/Makefile
to src/Makefile.global.in. That way, if you --enable-depend and then
do something like "touch src/include/catalog/pg_proc.h" and then "cd
src/backend/commands; make vacuum.o", it rebuilds fmgroids.h and then
recompiles vacuum.c. Under HEAD, it just tells you that vacuum.o is
up to date.

I'm not convinced by this use case. Why do you want to rebuild vacuum.o
anyway? If you change something, then you probably want to rebuild postgres,
and that works fine without this change.

I'm concerned how this change moves random make rules into a global makefile.
This is exacerbated by your next patch, which among other things moves the
entire catalog list to build the various output files into Makefile.global,
far away from its home.

#9Robert Haas
robertmhaas@gmail.com
In reply to: Peter Eisentraut (#8)
Re: dependencies for generated header files

On Tue, Aug 11, 2009 at 4:38 PM, Peter Eisentraut<peter_e@gmx.net> wrote:

On Sunday 28 June 2009 21:21:35 Robert Haas wrote:

I think that our dependencies for generated header files (gram.h,
fmgroids.h, probes.h) are not as good as they could be.  What we do
right now is make src/backend/Makefile rebuild these before recursing
through its subdirectories.  This works OK for a top-level make, but
if you run make further down in the tree (like under
src/backend/commands) it won't necessarily rebuild everything that it
should.

The attached patch moves some of this logic from src/backend/Makefile
to src/Makefile.global.in.  That way, if you --enable-depend and then
do something like "touch src/include/catalog/pg_proc.h" and then "cd
src/backend/commands; make vacuum.o", it rebuilds fmgroids.h and then
recompiles vacuum.c.  Under HEAD, it just tells you that vacuum.o is
up to date.

I'm not convinced by this use case.  Why do you want to rebuild vacuum.o
anyway?  If you change something, then you probably want to rebuild postgres,
and that works fine without this change.

Well, my original motivation for developing this patch was that I
often go into a subdirectory and start hacking on a .c file and then
type make to rebuild just that subdirectory (so it's easier to see
warnings and errors). But sometimes I would edit a relevant header
file and then the rebuild wouldn't happen. That bugged me. Your
mileage may vary.

I'm concerned how this change moves random make rules into a global makefile.
This is exacerbated by your next patch, which among other things moves the
entire catalog list to build the various output files into Makefile.global,
far away from its home.

*shrug* You don't have to accept the patch, but I'm unclear as to how
make from a subdirectory will ever work reliably without something
like this. The problem occurs when .c files have dependencies on
files that are generated by a Makefile in some other subdirectory. So
it's not random stuff: it's the stuff where that particular thing
happens.

...Robert

#10Alvaro Herrera
alvherre@commandprompt.com
In reply to: Robert Haas (#9)
Re: dependencies for generated header files

Robert Haas escribi�:

*shrug* You don't have to accept the patch, but I'm unclear as to how
make from a subdirectory will ever work reliably without something
like this. The problem occurs when .c files have dependencies on
files that are generated by a Makefile in some other subdirectory. So
it's not random stuff: it's the stuff where that particular thing
happens.

But what use would have to build that particular .o file, and not the
whole postgres binary?

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#11Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#9)
Re: dependencies for generated header files

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Aug 11, 2009 at 4:38 PM, Peter Eisentraut<peter_e@gmx.net> wrote:

I'm not convinced by this use case.

Well, my original motivation for developing this patch was that I
often go into a subdirectory and start hacking on a .c file and then
type make to rebuild just that subdirectory (so it's easier to see
warnings and errors). But sometimes I would edit a relevant header
file and then the rebuild wouldn't happen. That bugged me. Your
mileage may vary.

Surely the answer to that is "you should be configuring with --enable-depend".

regards, tom lane

#12Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#11)
Re: dependencies for generated header files

On Tue, Aug 11, 2009 at 9:00 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Aug 11, 2009 at 4:38 PM, Peter Eisentraut<peter_e@gmx.net> wrote:

I'm not convinced by this use case.

Well, my original motivation for developing this patch was that I
often go into a subdirectory and start hacking on a .c file and then
type make to rebuild just that subdirectory (so it's easier to see
warnings and errors).  But sometimes I would edit a relevant header
file and then the rebuild wouldn't happen.  That bugged me.  Your
mileage may vary.

Surely the answer to that is "you should be configuring with --enable-depend".

Uhm, the point is that this is broken even with ---enable-depend.
Please refer to the OP.

...Robert

#13Robert Haas
robertmhaas@gmail.com
In reply to: Alvaro Herrera (#10)
Re: dependencies for generated header files

On Tue, Aug 11, 2009 at 5:56 PM, Alvaro
Herrera<alvherre@commandprompt.com> wrote:

Robert Haas escribió:

*shrug*  You don't have to accept the patch, but I'm unclear as to how
make from a subdirectory will ever work reliably without something
like this.  The problem occurs when .c files have dependencies on
files that are generated by a Makefile in some other subdirectory.  So
it's not random stuff: it's the stuff where that particular thing
happens.

But what use would have to build that particular .o file, and not the
whole postgres binary?

I don't think that I can spell it out any more clearly than I did in
the paragraph immediately preceding the one you have quoted here.

...Robert

#14Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#12)
Re: dependencies for generated header files

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Aug 11, 2009 at 9:00 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:

Surely the answer to that is "you should be configuring with --enable-depend".

Uhm, the point is that this is broken even with ---enable-depend.

Oh, okay, but that's only for *generated* header files. I'm not excited
about that. We've pretty much managed to minimize compile-time
dependencies on generated files. Now of course your other patch wants
to vastly expand the use of generated files, and I can see that we might
have a problem of this sort if we accepted that patch --- but it seems
Peter's not excited about that one either.

regards, tom lane

#15Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#14)
Re: dependencies for generated header files

On Tue, Aug 11, 2009 at 9:19 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

On Tue, Aug 11, 2009 at 9:00 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:

Surely the answer to that is "you should be configuring with --enable-depend".

Uhm, the point is that this is broken even with ---enable-depend.

Oh, okay, but that's only for *generated* header files.  I'm not excited
about that.  We've pretty much managed to minimize compile-time
dependencies on generated files.  Now of course your other patch wants
to vastly expand the use of generated files, and I can see that we might
have a problem of this sort if we accepted that patch --- but it seems
Peter's not excited about that one either.

Given that the anum.h stuff is gone, "vastly" might be an
overstatement. I'm pretty surprised to find out that people don't
like the idea of having dependencies be correct from anywhere in the
tree. Even if I'm the only developer who does partial builds, the
cost seems to me to be next to nil, so I'm not quite sure what anyone
gets out of rejecting this patch. That having been said, it's not
really worth it to me to spend a lot of time arguing about it. So
far, the only coherent argument why this is bad is that it moves some
logic into a shared Makefile rather than a directory-specific
Makefile, which might be confusing to someone trying to maintain the
Makefiles. I don't really buy that because they're already complex
enough that you have to read them all to understand what they are
doing, and nothing in this quite small patch is going to change that
picture very much, but I guess that's just me.

...Robert

#16Alvaro Herrera
alvherre@commandprompt.com
In reply to: Robert Haas (#15)
Re: dependencies for generated header files

Robert Haas escribi�:

Given that the anum.h stuff is gone, "vastly" might be an
overstatement. I'm pretty surprised to find out that people don't
like the idea of having dependencies be correct from anywhere in the
tree. Even if I'm the only developer who does partial builds, the
cost seems to me to be next to nil, so I'm not quite sure what anyone
gets out of rejecting this patch.

I actually kinda like this patch. I tend to do partial builds
frequently.

That having been said, it's not
really worth it to me to spend a lot of time arguing about it. So
far, the only coherent argument why this is bad is that it moves some
logic into a shared Makefile rather than a directory-specific
Makefile, which might be confusing to someone trying to maintain the
Makefiles. I don't really buy that because they're already complex
enough that you have to read them all to understand what they are
doing, and nothing in this quite small patch is going to change that
picture very much, but I guess that's just me.

The action-at-a-distance rules in the shared makefile is a pain, but I
think I'd live with it -- just make sure it is properly documented in
both places.

--
Alvaro Herrera http://www.CommandPrompt.com/
PostgreSQL Replication, Consulting, Custom Development, 24x7 support

#17Tom Lane
tgl@sss.pgh.pa.us
In reply to: Robert Haas (#15)
Re: dependencies for generated header files

Robert Haas <robertmhaas@gmail.com> writes:

Given that the anum.h stuff is gone, "vastly" might be an
overstatement. I'm pretty surprised to find out that people don't
like the idea of having dependencies be correct from anywhere in the
tree. Even if I'm the only developer who does partial builds, the
cost seems to me to be next to nil, so I'm not quite sure what anyone
gets out of rejecting this patch.

It's not that having the dependencies be 100% up to date wouldn't be
nice; it's that there's a limit to how much we're willing to uglify
the Makefiles to have that. The makefiles need maintenance too,
you know, and putting things far away from where they should be is
not any better in the makefiles than it is in C code.

As far as I can tell, if you've used --enable-depend then things will
get updated properly before you can ever attempt to run the code
(ie, install a rebuilt postmaster). The only situation where you'd
actually get an improvement from redoing the dependencies like this
is where lack of an update to a derived file results in a compiler
error/warning. But there aren't many such cases. The only one I can
even think of offhand is lack of an fmgroids.h symbol for a newly-added
function ... but we don't use F_XXX symbols enough to make that a
convincing example. We've intentionally arranged things so that
more-fragile cases like gram.h are not referenced outside their own
directories.

regards, tom lane

#18Robert Haas
robertmhaas@gmail.com
In reply to: Tom Lane (#17)
Re: dependencies for generated header files

On Tue, Aug 11, 2009 at 9:56 PM, Tom Lane<tgl@sss.pgh.pa.us> wrote:

Robert Haas <robertmhaas@gmail.com> writes:

Given that the anum.h stuff is gone, "vastly" might be an
overstatement.  I'm pretty surprised to find out that people don't
like the idea of having dependencies be correct from anywhere in the
tree.  Even if I'm the only developer who does partial builds, the
cost seems to me to be next to nil, so I'm not quite sure what anyone
gets out of rejecting this patch.

It's not that having the dependencies be 100% up to date wouldn't be
nice; it's that there's a limit to how much we're willing to uglify
the Makefiles to have that.  The makefiles need maintenance too,
you know, and putting things far away from where they should be is
not any better in the makefiles than it is in C code.

Well, I certainly agree that making a huge mess to address what is
admittedly a corner case is not a good idea. But I also don't think
this patch is all that messy. However, I guess we're getting to the
point where we need to make a decision one way or the other so that we
can close out this CommitFest.

As far as I can tell, if you've used --enable-depend then things will
get updated properly before you can ever attempt to run the code
(ie, install a rebuilt postmaster).  The only situation where you'd
actually get an improvement from redoing the dependencies like this
is where lack of an update to a derived file results in a compiler
error/warning.  But there aren't many such cases.  The only one I can
even think of offhand is lack of an fmgroids.h symbol for a newly-added
function ... but we don't use F_XXX symbols enough to make that a
convincing example.  We've intentionally arranged things so that
more-fragile cases like gram.h are not referenced outside their own
directories.

Yes, that's definitely the best situation.

...Robert