Обсуждение: pgsql: Move gramparse.h to src/backend/parser
Move gramparse.h to src/backend/parser This header is semi-private, being used only in files related to raw parsing, so move to the backend directory where those files live. This allows removal of Makefile rules that symlink gram.h to src/include/parser, since gramparse.h can now include gram.h from within the same directory. This has the side-effect of no longer installing gram.h and gramparse.h, but there doesn't seem to be a good reason to continue doing so. Per suggestion from Andres Freund and Peter Eisentraut Discussion: https://www.postgresql.org/message-id/20220904181759.px6uosll6zbxcum5%40awork3.anarazel.de Branch ------ master Details ------- https://git.postgresql.org/pg/commitdiff/ecaf7c5df54f7fa9df2fdc7225d2bb4e283f0081 Modified Files -------------- src/backend/Makefile | 7 +------ src/backend/parser/gram.y | 2 +- src/{include => backend}/parser/gramparse.h | 4 ++-- src/backend/parser/parser.c | 2 +- src/backend/parser/scan.l | 2 +- src/include/Makefile | 4 ++-- src/include/parser/.gitignore | 1 - src/tools/msvc/Install.pm | 4 ---- src/tools/pginclude/cpluspluscheck | 1 - src/tools/pginclude/headerscheck | 1 - 10 files changed, 8 insertions(+), 20 deletions(-)
On Wed, Sep 14, 2022 at 11:02 AM John Naylor <john.naylor@postgresql.org> wrote: > > Move gramparse.h to src/backend/parser Member crake doesn't like this. I thought I tried vpath for this patch, but I'll go confirm now. -- John Naylor EDB: http://www.enterprisedb.com
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.
regards, tom lane
Hi, 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. Greetings, Andres Freund
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
On Thu, Sep 15, 2022 at 4:04 AM Andres Freund <andres@anarazel.de> wrote: > > 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 Since there have been no other ideas in the past few hours, I will push this but it will be a blind attempt since it seems sporadic and doesn't happen to reproduce for me. -- John Naylor EDB: http://www.enterprisedb.com
Hi, On 2022-09-15 10:52:31 +0700, John Naylor wrote: > On Thu, Sep 15, 2022 at 4:04 AM Andres Freund <andres@anarazel.de> wrote: > > > > 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 > > Since there have been no other ideas in the past few hours, I will > push this but it will be a blind attempt since it seems sporadic and > doesn't happen to reproduce for me. WFM. FWIW, you can reproduce it with rm -rf .deps/ gram.c scan.c *.o *.bc && make parser.bc in src/backend/parser. Greetings, Andres Freund
Hi!
On 15.09.2022 06:52, John Naylor wrote:
> On Thu, Sep 15, 2022 at 4:04 AM Andres Freund <andres@anarazel.de> wrote:
>>
>> 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
>
> Since there have been no other ideas in the past few hours, I will
> push this but it will be a blind attempt since it seems sporadic and
> doesn't happen to reproduce for me.
>
My colleague Marina Polyakova <m.polyakova@postgrespro.ru> found
the similar bug on buildfarm [1] for REL_15_STABLE in the llvm build:
-D_GNU_SOURCE -I/usr/include/libxml2 -flto=thin -emit-llvm -c -o segparse.bc segparse.c
segparse.y:177:10: fatal error: 'segscan.c' file not found
177 | #include "segscan.c"
| ^~~~~~~~~~~
1 error generated
Maybe backpatch [2] to all supported versions not just 16+?
With the best regards,
--
Anton A. Melnikov
Postgres Professional: http://www.postgrespro.com
The Russian Postgres Company
[1] https://buildfarm.postgresql.org/cgi-bin/show_log.pl?nm=alligator&dt=2025-06-02%2019:07:06
[2] https://github.com/postgres/postgres/commit/16492df70bb25bc99ca3c340a75ba84ca64171b8
On Wed, Oct 15, 2025 at 2:49 PM Anton A. Melnikov <a.melnikov@postgrespro.ru> wrote: > My colleague Marina Polyakova <m.polyakova@postgrespro.ru> found > the similar bug on buildfarm [1] for REL_15_STABLE in the llvm build: > > -D_GNU_SOURCE -I/usr/include/libxml2 -flto=thin -emit-llvm -c -o segparse.bc segparse.c > segparse.y:177:10: fatal error: 'segscan.c' file not found > 177 | #include "segscan.c" > | ^~~~~~~~~~~ > 1 error generated > > Maybe backpatch [2] to all supported versions not just 16+? That only changed src/backend/common.mk, so that's not going to affect contrib. I looked, and contrib/contrib-global.mk doesn't have any extra *.bc file handling to begin with (as expected). Also, that fix was in response to a specific change in dependencies, so I don't see how it's directly applicable to earlier branches. Maybe there is something to be done here, but with such a sporadic failure, I'm not sure what. -- John Naylor Amazon Web Services
John Naylor <johncnaylorls@gmail.com> writes:
> On Wed, Oct 15, 2025 at 2:49 PM Anton A. Melnikov
> <a.melnikov@postgrespro.ru> wrote:
>> Maybe backpatch [2] to all supported versions not just 16+?
> That only changed src/backend/common.mk, so that's not going to affect
> contrib. I looked, and contrib/contrib-global.mk doesn't have any
> extra *.bc file handling to begin with (as expected). Also, that fix
> was in response to a specific change in dependencies, so I don't see
> how it's directly applicable to earlier branches. Maybe there is
> something to be done here, but with such a sporadic failure, I'm not
> sure what.
Yeah. One build failure in three years does not sound to me like
something to panic about. It sounds more like a local problem.
Also, I note that alligator is self-described as running a
"gcc experimental (nightly build)" compiler, so temporary build
glitches on it are hardly unexpected.
It seems like there's an increasing number of buildfarm animals whose
aims are less "test Postgres" than "test platform X by building
Postgres on it". alligator is not the only experimental-tool-chain
animal, and fruitcrow (GNU Hurd) is another example we've been seeing
failures from lately. I don't want to tell those folk to go away,
but maybe we should have some kind of annotation about "platform not
believed stable" to remind us not to put huge amounts of effort into
transient failures on those animals.
regards, tom lane