Inconsistencies around defining FRONTEND

From: Andres Freund <andres(at)anarazel(dot)de>
To: Peter Eisentraut <peter(dot)eisentraut(at)enterprisedb(dot)com>, Tom Lane <tgl(at)sss(dot)pgh(dot)pa(dot)us>, Noah Misch <noah(at)leadboat(dot)com>
Cc: pgsql-hackers(at)postgresql(dot)org, samay sharma <smilingsamay(at)gmail(dot)com>, Nazir Bilal Yavuz <byavuz81(at)gmail(dot)com>
Subject: Inconsistencies around defining FRONTEND
Date: 2022-08-20 19:45:50
Message-ID: 20220820194550.725755r6fj2ro3rx@awork3.anarazel.de
Views: Raw Message | Whole Thread | Download mbox | Resend email
Thread:
Lists: pgsql-hackers

Hi,

This started at https://postgr.es/m/20220817215317.poeofidf7o7dy6hy%40awork3.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

Attachment Content-Type Size
v1-0001-Don-t-define-FRONTEND-for-initdb.patch text/x-diff 3.1 KB
v1-0002-Don-t-define-FRONTEND-for-ecpg-libraries.patch text/x-diff 3.3 KB
v1-0003-Don-t-define-FRONTEND-for-libpq.patch text/x-diff 1.4 KB

Responses

Browse pgsql-hackers by date

  From Date Subject
Next Message Andres Freund 2022-08-20 20:44:01 Re: Strip -mmacosx-version-min options from plperl build
Previous Message Erik Rijkers 2022-08-20 18:44:49 Re: Schema variables - new implementation for Postgres 15