Date: Wed, 21 Nov 2018 14:28:01 +0100
From: Christoph Berg <christoph.berg@credativ.de>
To: Andres Freund <andres@anarazel.de>
Cc: Tom Lane <tgl@sss.pgh.pa.us>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
Subject: Re: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
Message-ID: <20181121132801.GH15795@msg.credativ.de>
Mail-Followup-To: Christoph Berg <christoph.berg@credativ.de>,
	Andres Freund <andres@anarazel.de>, Tom Lane <tgl@sss.pgh.pa.us>,
	PostgreSQL Hackers <pgsql-hackers@postgresql.org>
References: <20181113104005.GA32154@msg.credativ.de>
 <20181113215716.qsdhsg54dmg6kvdl@alap3.anarazel.de>
 <22230.1542148037@sss.pgh.pa.us>
 <20181113223330.2ql7tg33hhh6husf@alap3.anarazel.de>
MIME-Version: 1.0
Content-Type: text/plain; charset=iso-8859-1
Content-Disposition: inline
Content-Transfer-Encoding: 8bit
In-Reply-To: <20181113223330.2ql7tg33hhh6husf@alap3.anarazel.de>
User-Agent: Mutt/1.10.1 (2018-07-13)
Precedence: bulk

(Sorry for the delayed response here.)

Re: Andres Freund 2018-11-13 <20181113223330.2ql7tg33hhh6husf@alap3.anarazel.de>
> > >> While working on making extension modules built reproducibly, I
> > >> noticed that extra flags passed via COPT (notably -ffile-prefix-map)
> > >> do not get added to CXXFLAGS.
> > 
> > > PROFILE I can see, but COPT I'm less sure. The name suggests it's about
> > > C not C++. How about adding CXXOPT?

Any of that would work, I was adding it to both because both were the
same so far.

> > > Secondary question:  Why aren't you using CFLAGS/CXXFLAGS for this?

The context here is that we want to use the *FLAGS from pg_config for
compiling PG extension packages, but add additional *FLAGS from the
extension build environment. Merging the pg_config CFLAGS with the
environment CFLAGS seemed hard/weird/error-prone, so what we are doing
now is https://salsa.debian.org/postgresql/postgresql-common/blob/master/pg_buildext#L53-55

    if [ "${CFLAGS:-}" ]; then
        export COPT="$CFLAGS"
    fi

Which works nicely.

> > COPT (and PROFILE) are handy for injecting additional flags manually;
> > they're even documented for that (cf installation.sgml, around line
> > 1550).
> 
> > I agree that CXXOPT would be a better idea than COPT for this.
> 
> Yea, I agree that we want CXX equivalent of the feature, especially for
> -Werror.  I'm just not sure that's the best fit for the debian build
> issue Christoph ran into. I guess the goal is to *not* include the
> -ffile-prefix-map in the CFLAGS for extensions, unless those are also
> built via debian machinery?

-ffile-prefix-map (or actually still -fdebug-prefix-map, -ffile-p-m is
WIP) is the reason for the extra flags, yes. We can't use pg_config's
version here because the build path for PG is not the build path for
the extension module.

> > Not sure about PROFILE.  But we could inject the latter into both
> > flag sets and then document that if that's not what you want, use
> > COPT.
> 
> That seems reasonable to me too.

Updated patch:

diff --git a/src/Makefile.global.in b/src/Makefile.global.in
index 956fd274cd..42bdf9f75c 100644
--- a/src/Makefile.global.in
+++ b/src/Makefile.global.in
@@ -674,8 +674,14 @@ ifdef COPT
    LDFLAGS += $(COPT)
 endif
 
+ifdef CXXOPT
+   CXXFLAGS += $(CXXOPT)
+   LDFLAGS += $(CXXOPT)
+endif
+
 ifdef PROFILE
    CFLAGS += $(PROFILE)
+   CXXFLAGS += $(PROFILE)
    LDFLAGS += $(PROFILE)
 endif
 

I didn't patch the documentation yet because I wasn't sure if the
LDFLAGS handling with CXXOPT makes sense.

Christoph
-- 
Senior Berater, Tel.: +49 2166 9901 187
credativ GmbH, HRB Mönchengladbach 12080, USt-ID-Nummer: DE204566209
Trompeterallee 108, 41189 Mönchengladbach
Geschäftsführung: Dr. Michael Meskes, Jörg Folz, Sascha Heuer
Unser Umgang mit personenbezogenen Daten unterliegt
folgenden Bestimmungen: https://www.credativ.de/datenschutz