Feedback-ID: id4a34324:Fastmail
Date: Wed, 14 Sep 2022 14:04:27 -0700
From: Andres Freund <andres@anarazel.de>
To: Tom Lane <tgl@sss.pgh.pa.us>
Cc: John Naylor <john.naylor@postgresql.org>,
	pgsql-committers@lists.postgresql.org
Subject: Re: pgsql: Move gramparse.h to src/backend/parser
Message-ID: <20220914210427.y26tkagmxo5wwbvp@awork3.anarazel.de>
References: <E1oYJbR-000FbR-LP@gemulon.postgresql.org>
 <2862807.1663184226@sss.pgh.pa.us>
 <20220914205715.xbmuqox5htobzggy@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline
In-Reply-To: <20220914205715.xbmuqox5htobzggy@awork3.anarazel.de>
Archived-At: 
 <https://www.postgresql.org/message-id/20220914210427.y26tkagmxo5wwbvp%40awork3.anarazel.de>
Precedence: bulk

Hi,

On 2022-09-14 13:57:15 -0700, Andres Freund wrote:
> On 2022-09-14 15:37:06 -0400, Tom Lane wrote:
> > John Naylor <john.naylor@postgresql.org> writes:
> > > Move gramparse.h to src/backend/parser
> >
> > The cfbot is unhappy since this commit; some but not all tests fail with
> >
> > [09:33:13.793] In file included from scan.c:39:
> > [09:33:13.793] ./gramparse.h:29:10: fatal error: 'gram.h' file not found
> > [09:33:13.794] #include "gram.h"
> > [09:33:13.794]          ^~~~~~~~
> > [09:33:13.839] In file included from parser.c:25:
> > [09:33:13.839] ./gramparse.h:29:10: fatal error: 'gram.h' file not found
> > [09:33:13.839] #include "gram.h"
> > [09:33:13.839]          ^~~~~~~~
> >
> > What I think is happening is that it was a mistake to remove
> > parser/gram.h from the dependencies of backend/Makefile's
> > generated-headers target: that allows builds to proceed before
> > gram.h has necessarily been created.  The fact that it works
> > at all for anybody says that there's another dependency path
> > somewhere that causes bison to get run ... but, seemingly,
> > that doesn't always happen soon enough in a parallel build.
>
> But why doesn't the below take care of it?
>
> > # Force these dependencies to be known even without dependency info built:
> > gram.o scan.o parser.o: gram.h
>
> The only file including gram.h is gramparse.h, which in turn is only included
> by parser.c, scan.l, gram.y. So this should suffice.

Ah, I see: The problem is compilation of .c -> .bc files, not -> .o, so it
only happens with llvm enabled. So far that was just taken care of by the
generated-headers dependency, but it's more granular now...

Since the bison aspect is quite slow, it'd probably be nicer to not include it
in generated-headers?

The most general solution I can see would be

diff --git i/src/backend/common.mk w/src/backend/common.mk
index fa96a82b1a0..61861f5c7eb 100644
--- i/src/backend/common.mk
+++ w/src/backend/common.mk
@@ -23,6 +23,7 @@ objfiles.txt: Makefile $(SUBDIROBJS) $(OBJS)

 ifeq ($(with_llvm), yes)
 objfiles.txt: $(patsubst %.o,%.bc, $(OBJS))
+$(patsubst %.o,%.bc, $(OBJS)): $(OBJS)
 endif

 # make function to expand objfiles.txt contents


Greetings,

Andres Freund