Feedback-ID: id4a34324:Fastmail
Date: Sat, 20 Aug 2022 12:45:50 -0700
From: Andres Freund <andres@anarazel.de>
To: Peter Eisentraut <peter.eisentraut@enterprisedb.com>,
	Tom Lane <tgl@sss.pgh.pa.us>, Noah Misch <noah@leadboat.com>
Cc: pgsql-hackers@postgresql.org, samay sharma <smilingsamay@gmail.com>,
	Nazir Bilal Yavuz <byavuz81@gmail.com>
Subject: Inconsistencies around defining FRONTEND
Message-ID: <20220820194550.725755r6fj2ro3rx@awork3.anarazel.de>
MIME-Version: 1.0
Content-Type: multipart/mixed; boundary="6cdqp334ngfj3pun"
Content-Disposition: inline
Archived-At: 
 <https://www.postgresql.org/message-id/20220820194550.725755r6fj2ro3rx%40awork3.anarazel.de>
Precedence: bulk


--6cdqp334ngfj3pun
Content-Type: text/plain; charset=us-ascii
Content-Disposition: inline

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

--6cdqp334ngfj3pun
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0001-Don-t-define-FRONTEND-for-initdb.patch"

From 96b9580229956c95279b4546f52d781408a3b15b Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 20 Aug 2022 12:13:42 -0700
Subject: [PATCH v1 1/3] Don't define FRONTEND for initdb

No headers requiring FRONTED to be defined are included as of af1a949109d.

Since this is the last user of (frontend|contrib)_defines in Mkvcbuild.pm,
remove that. For commit the file should be perltidy'ed, but as that causes
more changes I thought it'd be easer like this for review.
---
 src/bin/initdb/Makefile     |  2 +-
 src/tools/msvc/Mkvcbuild.pm | 14 ++------------
 2 files changed, 3 insertions(+), 13 deletions(-)

diff --git a/src/bin/initdb/Makefile b/src/bin/initdb/Makefile
index b0dd13dfbdf..6737938c3f8 100644
--- a/src/bin/initdb/Makefile
+++ b/src/bin/initdb/Makefile
@@ -16,7 +16,7 @@ subdir = src/bin/initdb
 top_builddir = ../../..
 include $(top_builddir)/src/Makefile.global
 
-override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) -I$(top_srcdir)/src/timezone $(CPPFLAGS)
+override CPPFLAGS := -I$(libpq_srcdir) -I$(top_srcdir)/src/timezone $(CPPFLAGS)
 
 # Note: it's important that we link to encnames.o from libpgcommon, not
 # from libpq, else we have risks of version skew if we run with a libpq
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index ee963d85f30..ac863418442 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -35,7 +35,6 @@ my $libpq;
 my @unlink_on_exit;
 
 # Set of variables for modules in contrib/ and src/test/modules/
-my $contrib_defines        = {};
 my @contrib_uselibpq       = ();
 my @contrib_uselibpgport   = ();
 my @contrib_uselibpgcommon = ();
@@ -53,7 +52,6 @@ my @contrib_excludes       = (
 	'unsafe_tests');
 
 # Set of variables for frontend modules
-my $frontend_defines = { 'initdb' => 'FRONTEND' };
 my @frontend_uselibpq =
   ('pg_amcheck', 'pg_ctl', 'pg_upgrade', 'pgbench', 'psql', 'initdb');
 my @frontend_uselibpgport = (
@@ -1126,7 +1124,7 @@ sub AdjustContribProj
 {
 	my $proj = shift;
 	AdjustModule(
-		$proj,                    $contrib_defines,
+		$proj,
 		\@contrib_uselibpq,       \@contrib_uselibpgport,
 		\@contrib_uselibpgcommon, $contrib_extralibs,
 		$contrib_extrasource,     $contrib_extraincludes);
@@ -1137,7 +1135,7 @@ sub AdjustFrontendProj
 {
 	my $proj = shift;
 	AdjustModule(
-		$proj,                     $frontend_defines,
+		$proj,
 		\@frontend_uselibpq,       \@frontend_uselibpgport,
 		\@frontend_uselibpgcommon, $frontend_extralibs,
 		$frontend_extrasource,     $frontend_extraincludes);
@@ -1147,7 +1145,6 @@ sub AdjustFrontendProj
 sub AdjustModule
 {
 	my $proj                  = shift;
-	my $module_defines        = shift;
 	my $module_uselibpq       = shift;
 	my $module_uselibpgport   = shift;
 	my $module_uselibpgcommon = shift;
@@ -1156,13 +1153,6 @@ sub AdjustModule
 	my $module_extraincludes  = shift;
 	my $n                     = $proj->{name};
 
-	if ($module_defines->{$n})
-	{
-		foreach my $d ($module_defines->{$n})
-		{
-			$proj->AddDefine($d);
-		}
-	}
 	if (grep { /^$n$/ } @{$module_uselibpq})
 	{
 		$proj->AddIncludeDir('src\interfaces\libpq');
-- 
2.37.0.3.g30cc8d0f14


--6cdqp334ngfj3pun
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0002-Don-t-define-FRONTEND-for-ecpg-libraries.patch"

From f7a2ad801701a7eac8c1747de1a763558702ac2e Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 20 Aug 2022 12:16:47 -0700
Subject: [PATCH v1 2/3] Don't define FRONTEND for ecpg libraries

Looks like that's not needed after 7143b3e8213
---
 src/interfaces/ecpg/compatlib/Makefile  | 2 +-
 src/interfaces/ecpg/ecpglib/Makefile    | 2 +-
 src/interfaces/ecpg/pgtypeslib/Makefile | 2 +-
 src/tools/msvc/Mkvcbuild.pm             | 3 ---
 4 files changed, 3 insertions(+), 6 deletions(-)

diff --git a/src/interfaces/ecpg/compatlib/Makefile b/src/interfaces/ecpg/compatlib/Makefile
index 867ad55539a..62b8a8c12cc 100644
--- a/src/interfaces/ecpg/compatlib/Makefile
+++ b/src/interfaces/ecpg/compatlib/Makefile
@@ -19,7 +19,7 @@ SO_MAJOR_VERSION= 3
 SO_MINOR_VERSION= $(MAJORVERSION)
 
 override CPPFLAGS := -I../include -I$(top_srcdir)/src/interfaces/ecpg/include \
-	-I$(libpq_srcdir) -DFRONTEND $(CPPFLAGS)
+	-I$(libpq_srcdir) $(CPPFLAGS)
 override CFLAGS += $(PTHREAD_CFLAGS)
 
 SHLIB_LINK_INTERNAL = -L../ecpglib -lecpg -L../pgtypeslib -lpgtypes $(libpq_pgport_shlib)
diff --git a/src/interfaces/ecpg/ecpglib/Makefile b/src/interfaces/ecpg/ecpglib/Makefile
index b00229f3fec..844fc6eb6ec 100644
--- a/src/interfaces/ecpg/ecpglib/Makefile
+++ b/src/interfaces/ecpg/ecpglib/Makefile
@@ -19,7 +19,7 @@ SO_MAJOR_VERSION= 6
 SO_MINOR_VERSION= $(MAJORVERSION)
 
 override CPPFLAGS := -I../include -I$(top_srcdir)/src/interfaces/ecpg/include \
-	-I$(libpq_srcdir) -I$(top_builddir)/src/port -DFRONTEND $(CPPFLAGS)
+	-I$(libpq_srcdir) -I$(top_builddir)/src/port $(CPPFLAGS)
 override CFLAGS += $(PTHREAD_CFLAGS)
 
 OBJS = \
diff --git a/src/interfaces/ecpg/pgtypeslib/Makefile b/src/interfaces/ecpg/pgtypeslib/Makefile
index 763787f22da..13ef992ee72 100644
--- a/src/interfaces/ecpg/pgtypeslib/Makefile
+++ b/src/interfaces/ecpg/pgtypeslib/Makefile
@@ -19,7 +19,7 @@ SO_MAJOR_VERSION= 3
 SO_MINOR_VERSION= $(MAJORVERSION)
 
 override CPPFLAGS := -I../include -I$(top_srcdir)/src/interfaces/ecpg/include \
-	-DFRONTEND $(CPPFLAGS)
+	$(CPPFLAGS)
 override CFLAGS += $(PTHREAD_CFLAGS)
 
 SHLIB_LINK_INTERNAL = -lpgcommon_shlib -lpgport_shlib
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index ac863418442..3a2dbdf267f 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -315,14 +315,12 @@ sub mkvcbuild
 	my $pgtypes = $solution->AddProject(
 		'libpgtypes', 'dll',
 		'interfaces', 'src/interfaces/ecpg/pgtypeslib');
-	$pgtypes->AddDefine('FRONTEND');
 	$pgtypes->AddReference($libpgcommon, $libpgport);
 	$pgtypes->UseDef('src/interfaces/ecpg/pgtypeslib/pgtypeslib.def');
 	$pgtypes->AddIncludeDir('src/interfaces/ecpg/include');
 
 	my $libecpg = $solution->AddProject('libecpg', 'dll', 'interfaces',
 		'src/interfaces/ecpg/ecpglib');
-	$libecpg->AddDefine('FRONTEND');
 	$libecpg->AddIncludeDir('src/interfaces/ecpg/include');
 	$libecpg->AddIncludeDir('src/interfaces/libpq');
 	$libecpg->AddIncludeDir('src/port');
@@ -333,7 +331,6 @@ sub mkvcbuild
 	my $libecpgcompat = $solution->AddProject(
 		'libecpg_compat', 'dll',
 		'interfaces',     'src/interfaces/ecpg/compatlib');
-	$libecpgcompat->AddDefine('FRONTEND');
 	$libecpgcompat->AddIncludeDir('src/interfaces/ecpg/include');
 	$libecpgcompat->AddIncludeDir('src/interfaces/libpq');
 	$libecpgcompat->UseDef('src/interfaces/ecpg/compatlib/compatlib.def');
-- 
2.37.0.3.g30cc8d0f14


--6cdqp334ngfj3pun
Content-Type: text/x-diff; charset=us-ascii
Content-Disposition: attachment;
	filename="v1-0003-Don-t-define-FRONTEND-for-libpq.patch"

From 7c975742d1c7a991ed6fe404dad49f19e4256716 Mon Sep 17 00:00:00 2001
From: Andres Freund <andres@anarazel.de>
Date: Sat, 20 Aug 2022 12:21:53 -0700
Subject: [PATCH v1 3/3] Don't define FRONTEND for libpq

Looks like that's not needed after 7143b3e8213
---
 src/interfaces/libpq/Makefile | 2 +-
 src/tools/msvc/Mkvcbuild.pm   | 1 -
 2 files changed, 1 insertion(+), 2 deletions(-)

diff --git a/src/interfaces/libpq/Makefile b/src/interfaces/libpq/Makefile
index 79c574eeb8b..1d31b256fc9 100644
--- a/src/interfaces/libpq/Makefile
+++ b/src/interfaces/libpq/Makefile
@@ -22,7 +22,7 @@ NAME= pq
 SO_MAJOR_VERSION= 5
 SO_MINOR_VERSION= $(MAJORVERSION)
 
-override CPPFLAGS :=  -DFRONTEND -I$(srcdir) $(CPPFLAGS) -I$(top_builddir)/src/port -I$(top_srcdir)/src/port
+override CPPFLAGS :=  -I$(srcdir) $(CPPFLAGS) -I$(top_builddir)/src/port -I$(top_srcdir)/src/port
 ifneq ($(PORTNAME), win32)
 override CFLAGS += $(PTHREAD_CFLAGS)
 endif
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index 3a2dbdf267f..5c4c0a98f13 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -263,7 +263,6 @@ sub mkvcbuild
 
 	$libpq = $solution->AddProject('libpq', 'dll', 'interfaces',
 		'src/interfaces/libpq');
-	$libpq->AddDefine('FRONTEND');
 	$libpq->AddIncludeDir('src/port');
 	$libpq->AddLibrary('secur32.lib');
 	$libpq->AddLibrary('ws2_32.lib');
-- 
2.37.0.3.g30cc8d0f14


--6cdqp334ngfj3pun--