From: Tom Lane <tgl@sss.pgh.pa.us>
To: Alvaro Herrera <alvherre@2ndquadrant.com>
cc: Peter Eisentraut <peter.eisentraut@2ndquadrant.com>,
        Ian Barwick <ian.barwick@2ndquadrant.com>,
        "pgsql-hackers@postgresql.org" <pgsql-hackers@postgresql.org>
Subject: Re: PG12, PGXS and linking pgfeutils
In-reply-to: <27315.1557424032@sss.pgh.pa.us>
References: <20190509173911.GA16616@alvherre.pgsql>
 <27315.1557424032@sss.pgh.pa.us>
Comments: In-reply-to Tom Lane <tgl@sss.pgh.pa.us>
	message dated "Thu, 09 May 2019 13:47:12 -0400"
MIME-Version: 1.0
Content-Type: text/plain; charset="us-ascii"
Content-ID: <9754.1557775997.1@sss.pgh.pa.us>
Date: Mon, 13 May 2019 15:33:17 -0400
Message-ID: <9755.1557775997@sss.pgh.pa.us>
Precedence: bulk

I wrote:
> Alvaro Herrera <alvherre@2ndquadrant.com> writes:
>> I wonder if a better solution isn't to move the file_utils stuff to
>> fe_utils.  Half of it is frontend-specific.  The only one that should be
>> shared to backend seems to be fsync_fname ... but instead of sharing it,
>> we have a second copy in fd.c.

> Hm, if file_utils is the only thing in common/ that uses this, and we
> expect that to remain true, that would fix the issue.  But ...

Thumbing through commit cc8d41511, I see that it already touched
five common/ modules

diff --git a/src/common/controldata_utils.c b/src/common/controldata_utils.c
diff --git a/src/common/file_utils.c b/src/common/file_utils.c
diff --git a/src/common/pgfnames.c b/src/common/pgfnames.c
diff --git a/src/common/restricted_token.c b/src/common/restricted_token.c
diff --git a/src/common/rmtree.c b/src/common/rmtree.c

Several of those have substantial backend components, so moving them
to fe_utils is a nonstarter.  I think moving fe_utils/logging.[hc] to
common/ is definitely the way to get out of this problem.


I started working on a patch to do that, and soon noticed that there
are pre-existing files logging.[hc] in src/bin/pg_rewind/.  This seems
like a Bad Thing, in fact the #includes in pg_rewind/ are already a
little confused due to this.  I think we should either rename those
two pg_rewind files to something else, or rename the generic ones,
perhaps to "fe_logging.[hc]".  The latter could be done nearly
trivially as part of the movement patch, but on cosmetic grounds
I'd be more inclined to do the former instead.  Thoughts?

			regards, tom lane