On Sun, Jun 07, 2020 at 10:05:03PM +0300, Alexander Korotkov wrote: > On Sat, Jun 6, 2020 at 8:49 PM Peter Eisentraut > <peter.eisentr...@2ndquadrant.com> wrote: >> Why is fe_archive.c in src/common/ and not in src/fe_utils/? It is not >> "common" to frontend and backend. > > Yep, this seems wrong to me. I can propose following file rename. > > src/common/fe_archive.c => src/fe_utils/archive.c > include/common/fe_archive.h => include/fe_utils/archive.h
Is it technically a problem though? fe_archive.c is listed as a frontend-only file with OBJS_FRONTEND in src/common/Makefile. Anyway, for this one I would not mind to do the move so please see the attached, but I am not completely sure either why src/fe_utils/ had better be chosen than src/common/. >> It actually defines functions that clash with functions in the backend, >> so this seems doubly wrong. > > Let's have frontend version of RestoreArchivedFile() renamed as well. > What about RestoreArchivedFileFrontend()? -1 from me for the renaming, which was intentional to ease grepping with the backend counterpart. We have other cases like that, say palloc(), fsync_fname(), etc. Perhaps we have more things that are frontend-only in src/common/ that could be moved to src/fe_utils/? I am looking at restricted_token.c, fe_memutils.c, logging.c and file_utils.c here, but that would mean breaking a couple of declarations, something never free for plugin developers. -- Michael
From ca75b91498c71b6558bd6befa3ad1510e00cf5fa Mon Sep 17 00:00:00 2001
From: Michael Paquier <mich...@paquier.xyz>
Date: Mon, 8 Jun 2020 15:06:02 +0900
Subject: [PATCH] Move frontend-side archive APIs to src/fe_utils/
---
.../{common/fe_archive.h => fe_utils/archive.h} | 4 ++--
src/common/Makefile | 1 -
src/fe_utils/Makefile | 1 +
src/{common/fe_archive.c => fe_utils/archive.c} | 10 +++-------
src/bin/pg_rewind/parsexlog.c | 2 +-
src/tools/msvc/Mkvcbuild.pm | 8 ++++----
6 files changed, 11 insertions(+), 15 deletions(-)
rename src/include/{common/fe_archive.h => fe_utils/archive.h} (91%)
rename src/{common/fe_archive.c => fe_utils/archive.c} (94%)
diff --git a/src/include/common/fe_archive.h b/src/include/fe_utils/archive.h
similarity index 91%
rename from src/include/common/fe_archive.h
rename to src/include/fe_utils/archive.h
index 495b560d24..a6beaf04ea 100644
--- a/src/include/common/fe_archive.h
+++ b/src/include/fe_utils/archive.h
@@ -1,12 +1,12 @@
/*-------------------------------------------------------------------------
*
- * fe_archive.h
+ * archive.h
* Routines to access WAL archives from frontend
*
* Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
* Portions Copyright (c) 1994, Regents of the University of California
*
- * src/include/common/fe_archive.h
+ * src/include/fe_utils/archive.h
*
*-------------------------------------------------------------------------
*/
diff --git a/src/common/Makefile b/src/common/Makefile
index d0be882cca..16619e4ba8 100644
--- a/src/common/Makefile
+++ b/src/common/Makefile
@@ -89,7 +89,6 @@ endif
# (Mkvcbuild.pm has a copy of this list, too)
OBJS_FRONTEND = \
$(OBJS_COMMON) \
- fe_archive.o \
fe_memutils.o \
file_utils.o \
logging.o \
diff --git a/src/fe_utils/Makefile b/src/fe_utils/Makefile
index 9eb4417690..dd20663604 100644
--- a/src/fe_utils/Makefile
+++ b/src/fe_utils/Makefile
@@ -20,6 +20,7 @@ include $(top_builddir)/src/Makefile.global
override CPPFLAGS := -DFRONTEND -I$(libpq_srcdir) $(CPPFLAGS)
OBJS = \
+ archive.o \
cancel.o \
conditional.o \
mbprint.o \
diff --git a/src/common/fe_archive.c b/src/fe_utils/archive.c
similarity index 94%
rename from src/common/fe_archive.c
rename to src/fe_utils/archive.c
index b0d68870db..c4cb213198 100644
--- a/src/common/fe_archive.c
+++ b/src/fe_utils/archive.c
@@ -1,6 +1,6 @@
/*-------------------------------------------------------------------------
*
- * fe_archive.c
+ * archive.c
* Routines to access WAL archives from frontend
*
* Portions Copyright (c) 1996-2020, PostgreSQL Global Development Group
@@ -8,15 +8,11 @@
*
*
* IDENTIFICATION
- * src/common/fe_archive.c
+ * src/fe_utils/archive.c
*
*-------------------------------------------------------------------------
*/
-#ifndef FRONTEND
-#error "This file is not expected to be compiled for backend code"
-#endif
-
#include "postgres_fe.h"
#include <unistd.h>
@@ -24,8 +20,8 @@
#include "access/xlog_internal.h"
#include "common/archive.h"
-#include "common/fe_archive.h"
#include "common/logging.h"
+#include "fe_utils/archive.h"
/*
diff --git a/src/bin/pg_rewind/parsexlog.c b/src/bin/pg_rewind/parsexlog.c
index d637f5eb77..bc6f976994 100644
--- a/src/bin/pg_rewind/parsexlog.c
+++ b/src/bin/pg_rewind/parsexlog.c
@@ -19,7 +19,7 @@
#include "catalog/pg_control.h"
#include "catalog/storage_xlog.h"
#include "commands/dbcommands_xlog.h"
-#include "common/fe_archive.h"
+#include "fe_utils/archive.h"
#include "filemap.h"
#include "pg_rewind.h"
diff --git a/src/tools/msvc/Mkvcbuild.pm b/src/tools/msvc/Mkvcbuild.pm
index c21c94dc1f..82d7d2c0e8 100644
--- a/src/tools/msvc/Mkvcbuild.pm
+++ b/src/tools/msvc/Mkvcbuild.pm
@@ -138,14 +138,14 @@ sub mkvcbuild
}
our @pgcommonfrontendfiles = (
- @pgcommonallfiles, qw(fe_archive.c fe_memutils.c
- file_utils.c logging.c restricted_token.c));
+ @pgcommonallfiles, qw(fe_memutils.c file_utils.c
+ logging.c restricted_token.c));
our @pgcommonbkndfiles = @pgcommonallfiles;
our @pgfeutilsfiles = qw(
- cancel.c conditional.c mbprint.c print.c psqlscan.l psqlscan.c
- simple_list.c string_utils.c recovery_gen.c);
+ archive.c cancel.c conditional.c mbprint.c print.c psqlscan.l
+ psqlscan.c simple_list.c string_utils.c recovery_gen.c);
$libpgport = $solution->AddProject('libpgport', 'lib', 'misc');
$libpgport->AddDefine('FRONTEND');
--
2.27.0.rc0
signature.asc
Description: PGP signature
