Inconsistencies around defining FRONTEND
Hackorum builds and tests every patch posted to the lists, not only commitfest submissions. This is Hackorum's own CI rather than the PostgreSQL project's, and it is still under testing - please report anything that looks wrong.
You can run a PostgreSQL built from this patch straight from Docker, with no checkout and no build:
docker run --rm -p 5432:5432 ghcr.io/hackorum-dev/postgres-patch:t46450psql -h localhost -U postgresBuilt from patchset v4 (message #4), August 18, 2026 at 03:48 PM.
Every patchset is also pushed to a branch of our PostgreSQL fork, so you can check out the same tree CI built. Without a PostgreSQL checkout:
git clone --branch t46450_4 https://github.com/hackorum-dev/postgres.gitIn a checkout you already have, add the fork once:
git remote add hackorum https://github.com/hackorum-dev/postgres.gitthen, for this patchset and every later one:
git fetch hackorum t46450_4 && git checkout t46450_4Patchset v4 (message #4) is on t46450_4
Hi,
This started at /messages/by-id/20220817215317.poeofidf7o7dy6hy@awork3.anarazel.de
Peter made a good point about -DFRONTED not being defined symmetrically
between meson and autoconf builds, which made me look at where we define
it. And I think we ought to clean this up independ of the meson patch.
On 2022-08-17 14:53:17 -0700, Andres Freund wrote:
On 2022-08-17 15:50:23 +0200, Peter Eisentraut wrote:
- -DFRONTEND is used somewhat differently from the makefiles. For
example, meson sets -DFRONTEND for pg_controldata, but the
makefiles don't. Conversely, the makefiles set -DFRONTEND for
ecpglib, but meson does not. This should be checked again to make
sure it all matches up.Yes, should sync that up.
FWIW, meson does add -DFRONTEND for ecpglib. There were a few places that did
add it twice, I'll push a cleanup of that in a bit.
Yikes, the situation in HEAD is quite the mess.
Several .c files set FRONTEND themselves, so they can include postgres.h,
instead of postgres_fe.h:
$ git grep '#define.*FRONTEND' upstream/master ':^src/include/postgres_fe.h'
upstream/master:src/bin/pg_controldata/pg_controldata.c:#define FRONTEND 1
upstream/master:src/bin/pg_resetwal/pg_resetwal.c:#define FRONTEND 1
upstream/master:src/bin/pg_waldump/compat.c:#define FRONTEND 1
upstream/master:src/bin/pg_waldump/pg_waldump.c:#define FRONTEND 1
upstream/master:src/bin/pg_waldump/rmgrdesc.c:#define FRONTEND 1
Yet, most of them also define FRONTEND in both make and msvc buildsystem.
$ git grep -E "(D|AddDefine\(')FRONTEND" upstream/master src/bin/ src/tools/msvc/
upstream/master:src/bin/initdb/Makefile:override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) -I$(top_srcdir)/src/timezone $(CPPFLAGS)
upstream/master:src/bin/pg_rewind/Makefile:override CPPFLAGS := -I$(libpq_srcdir) -DFRONTEND $(CPPFLAGS)
upstream/master:src/bin/pg_waldump/Makefile:override CPPFLAGS := -DFRONTEND $(CPPFLAGS)
upstream/master:src/tools/msvc/Mkvcbuild.pm: $libpgport->AddDefine('FRONTEND');
upstream/master:src/tools/msvc/Mkvcbuild.pm: $libpgcommon->AddDefine('FRONTEND');
upstream/master:src/tools/msvc/Mkvcbuild.pm: $libpgfeutils->AddDefine('FRONTEND');
upstream/master:src/tools/msvc/Mkvcbuild.pm: $libpq->AddDefine('FRONTEND');
upstream/master:src/tools/msvc/Mkvcbuild.pm: $pgtypes->AddDefine('FRONTEND');
upstream/master:src/tools/msvc/Mkvcbuild.pm: $libecpg->AddDefine('FRONTEND');
upstream/master:src/tools/msvc/Mkvcbuild.pm: $libecpgcompat->AddDefine('FRONTEND');
upstream/master:src/tools/msvc/Mkvcbuild.pm: $pgrewind->AddDefine('FRONTEND');
upstream/master:src/tools/msvc/Mkvcbuild.pm: $pg_waldump->AddDefine('FRONTEND')
That's largely because they also build files from src/backend, which obviously
contain no #define FRONTEND.
The -DFRONTENDs for the various ecpg libraries don't seem necessary
anymore. That looks to be a leftover from 7143b3e8213, before that ecpg had
copies of various pgport libraries.
Same with libpq, also looks to be obsoleted by 7143b3e8213.
I don't think we need FRONTEND in initdb - looks like that stopped being
required with af1a949109d.
Unfortunately, the remaining uses of FRONTEND are required. That's:
- pg_controldata, via #define
- pg_resetwal, via #define
- pg_rewind, via -DFRONTEND, due to xlogreader.c
- pg_waldump, via #define and -DFRONTEND, due to xlogreader.c, xlogstats.c, rmgrdesc/*desc.c
I'm kind of wondering if we should add xlogreader.c, xlogstat.c, *desc to
fe_utils, instead of building them in various places. That'd leave us only
with #define FRONTENDs for cases that do need to include postgres.h
themselves, which seems a lot more palatable.
Greetings,
Andres Freund
Hi,
On 2022-08-20 12:45:50 -0700, Andres Freund wrote:
The -DFRONTENDs for the various ecpg libraries don't seem necessary
anymore. That looks to be a leftover from 7143b3e8213, before that ecpg had
copies of various pgport libraries.Same with libpq, also looks to be obsoleted by 7143b3e8213.
I don't think we need FRONTEND in initdb - looks like that stopped being
required with af1a949109d.
I think the patches for this are fairly obvious, and survived CI without an
issue [1]https://cirrus-ci.com/build/4648937721167872, so the src/tools/msvc bits work too. So I'm planning to push them
fairly soon.
But the remaining "issues" don't have an obvious solutions and not addressed
by these patches:
Unfortunately, the remaining uses of FRONTEND are required. That's:
- pg_controldata, via #define
- pg_resetwal, via #define
- pg_rewind, via -DFRONTEND, due to xlogreader.c
- pg_waldump, via #define and -DFRONTEND, due to xlogreader.c, xlogstats.c, rmgrdesc/*desc.cI'm kind of wondering if we should add xlogreader.c, xlogstat.c, *desc to
fe_utils, instead of building them in various places. That'd leave us only
with #define FRONTENDs for cases that do need to include postgres.h
themselves, which seems a lot more palatable.
Greetings,
Andres Freund
Hi,
On 2022-08-22 08:48:34 -0700, Andres Freund wrote:
On 2022-08-20 12:45:50 -0700, Andres Freund wrote:
The -DFRONTENDs for the various ecpg libraries don't seem necessary
anymore. That looks to be a leftover from 7143b3e8213, before that ecpg had
copies of various pgport libraries.Same with libpq, also looks to be obsoleted by 7143b3e8213.
I don't think we need FRONTEND in initdb - looks like that stopped being
required with af1a949109d.I think the patches for this are fairly obvious, and survived CI without an
issue [1], so the src/tools/msvc bits work too. So I'm planning to push them
fairly soon.
Done.
- Andres
On Sat, Aug 20, 2022 at 3:46 PM Andres Freund <andres@anarazel.de> wrote:
Unfortunately, the remaining uses of FRONTEND are required. That's:
- pg_controldata, via #define
- pg_resetwal, via #define
- pg_rewind, via -DFRONTEND, due to xlogreader.c
- pg_waldump, via #define and -DFRONTEND, due to xlogreader.c, xlogstats.c, rmgrdesc/*desc.c
Actually, I think we could fix these pretty easily too. See attached.
This has been annoying me for a while. I hope we can agree on a way to
clean it up.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
Actually, I think we could fix these pretty easily too. See attached.
Hmm, do these headers still pass headerscheck/cpluspluscheck?
I might quibble a bit with the exact placement of the #ifndef FRONTEND
tests, but overall this looks pretty plausible.
regards, tom lane
On Tue, Aug 23, 2022 at 5:56 PM Tom Lane <tgl@sss.pgh.pa.us> wrote:
Robert Haas <robertmhaas@gmail.com> writes:
Actually, I think we could fix these pretty easily too. See attached.
Hmm, do these headers still pass headerscheck/cpluspluscheck?
I didn't check before sending the patch, but now I ran it locally, and
I did get failures from both, but they all seem to be unrelated.
Mainly, it's sad that I don't have Python.h, but I didn't configure
with python, so whatever.
I might quibble a bit with the exact placement of the #ifndef FRONTEND
tests, but overall this looks pretty plausible.
Yep, that's arguable. In particular, should the redo functions also be
protected by #ifdef FRONTEND?
I'd be more than thrilled if you wanted to adjust this to taste and
apply it, barring objections from others of course.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On 2022-08-23 17:24:30 -0400, Robert Haas wrote:
On Sat, Aug 20, 2022 at 3:46 PM Andres Freund <andres@anarazel.de> wrote:
Unfortunately, the remaining uses of FRONTEND are required. That's:
- pg_controldata, via #define
- pg_resetwal, via #define
- pg_rewind, via -DFRONTEND, due to xlogreader.c
- pg_waldump, via #define and -DFRONTEND, due to xlogreader.c, xlogstats.c, rmgrdesc/*desc.cActually, I think we could fix these pretty easily too.
I just meant that they're not already obsolete ;)
See attached.
Just to make sure I understand - you're just trying to get rid of the #define
frontends, not the -DFRONTENDs passed in from the Makefile? Because afaics we
still need those, correct?
Greetings,
Andres Freund
On Tue, Aug 23, 2022 at 7:24 PM Andres Freund <andres@anarazel.de> wrote:
Just to make sure I understand - you're just trying to get rid of the #define
frontends, not the -DFRONTENDs passed in from the Makefile? Because afaics we
still need those, correct?
Oh, yeah, this only fixes the #define ones. But maybe fixing the other
ones with a similar approach would be possible?
I really don't see why we should tolerate having #define FRONTEND in
more than once place.
--
Robert Haas
EDB: http://www.enterprisedb.com
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Aug 23, 2022 at 7:24 PM Andres Freund <andres@anarazel.de> wrote:
Just to make sure I understand - you're just trying to get rid of the #define
frontends, not the -DFRONTENDs passed in from the Makefile? Because afaics we
still need those, correct?
Oh, yeah, this only fixes the #define ones. But maybe fixing the other
ones with a similar approach would be possible?
I really don't see why we should tolerate having #define FRONTEND in
more than once place.
src/port and src/common really need to do it like that (ie pass in
the -D switch) so that the identical source file can be built
both ways. Maybe we could get rid of -DFRONTEND in other places,
like pg_rewind and pg_waldump.
regards, tom lane
Hi,
On 2022-08-23 19:50:00 -0400, Tom Lane wrote:
Robert Haas <robertmhaas@gmail.com> writes:
On Tue, Aug 23, 2022 at 7:24 PM Andres Freund <andres@anarazel.de> wrote:
Just to make sure I understand - you're just trying to get rid of the #define
frontends, not the -DFRONTENDs passed in from the Makefile? Because afaics we
still need those, correct?Oh, yeah, this only fixes the #define ones. But maybe fixing the other
ones with a similar approach would be possible?I really don't see why we should tolerate having #define FRONTEND in
more than once place.src/port and src/common really need to do it like that (ie pass in
the -D switch) so that the identical source file can be built
both ways. Maybe we could get rid of -DFRONTEND in other places,
like pg_rewind and pg_waldump.
We could, if we make xlogreader.c and the rmgrdesc routines built as part of
src/common. I don't really see how otherwise.
Greetings,
Andres Freund
On Tue, Aug 23, 2022 at 9:55 PM Andres Freund <andres@anarazel.de> wrote:
We could, if we make xlogreader.c and the rmgrdesc routines built as part of
src/common. I don't really see how otherwise.
After a little bit of study, I agree.
It looks to me like -DFRONTEND can be removed from
src/fe_utils/Makefile and probably also src/common/unicode/Makefile
without changing anything else, because the C files in those
directories seem to be frontend-only and they already include
"postgres_fe.h". I think we should go ahead and do that, and also
apply the patch I posted yesterday with whatever bikeshedding seems
appropriate.
It doesn't really seem like we have a plausible alternative to the
current system for src/common or src/port.
pg_rewind and pg_waldump seem to need the xlogreader code moved to
src/common, as Andres proposes. I'm not volunteering to tackle that
right now but I think it might be a good thing to do sometime.
--
Robert Haas
EDB: http://www.enterprisedb.com
Hi,
On 2022-08-24 10:40:01 -0400, Robert Haas wrote:
pg_rewind and pg_waldump seem to need the xlogreader code moved to
src/common, as Andres proposes. I'm not volunteering to tackle that
right now but I think it might be a good thing to do sometime.
The easier way would be to just keep their current method of building, but do
it as part of src/common.
Greetings,
Andres Freund