Thread: [PATCH] Pass COPT and PROFILE to CXXFLAGS as well
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. This causes postgresql-hll to still store the full build path in the .so file built: https://tests.reproducible-builds.org/debian/rb-pkg/unstable/amd64/diffoscope-results/postgresql-hll.html 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
Attachment
Hi, On 2018-11-13 11:40:05 +0100, Christoph Berg wrote: > 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? Secondary question: Why aren't you using CFLAGS/CXXFLAGS for this? Greetings, Andres Freund
Andres Freund <andres@anarazel.de> writes:
> On 2018-11-13 11:40:05 +0100, Christoph Berg wrote:
>> 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?
> Secondary question: Why aren't you using CFLAGS/CXXFLAGS for this?
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.
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.
regards, tom lane
Hi, On 2018-11-13 17:27:17 -0500, Tom Lane wrote: > Andres Freund <andres@anarazel.de> writes: > > On 2018-11-13 11:40:05 +0100, Christoph Berg wrote: > >> 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? > > > Secondary question: Why aren't you using CFLAGS/CXXFLAGS for this? > > 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? > 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. Greetings, Andres Freund
(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
On 21/11/2018 14:28, Christoph Berg wrote: > 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 Note that pgxs supports PG_CPPFLAGS for adding custom pieces to CPPFLAGS in a safe way. Maybe we should add an equivalent for CFLAGS? It's just that it hasn't been needed so far. -- Peter Eisentraut http://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Fri, Jan 04, 2019 at 11:41:15AM +0100, Peter Eisentraut wrote: > Note that pgxs supports PG_CPPFLAGS for adding custom pieces to CPPFLAGS > in a safe way. Maybe we should add an equivalent for CFLAGS? It's just > that it hasn't been needed so far. +1. Wouldn't it make sense to also have PG_LDFLAGS? In some stuff I maintain, I have been abusing of PG_CPPFLAGS to pass some flags, which is not really right. We also have contrib/passwordcheck/Makefile abuse of it to set -DUSE_CRACKLIB.. -- Michael
Attachment
Re: Michael Paquier 2019-01-04 <20190104133305.GG2067@paquier.xyz> > On Fri, Jan 04, 2019 at 11:41:15AM +0100, Peter Eisentraut wrote: > > Note that pgxs supports PG_CPPFLAGS for adding custom pieces to CPPFLAGS > > in a safe way. Maybe we should add an equivalent for CFLAGS? It's just > > that it hasn't been needed so far. > > +1. Wouldn't it make sense to also have PG_LDFLAGS? In some stuff I > maintain, I have been abusing of PG_CPPFLAGS to pass some flags, which > is not really right. We also have contrib/passwordcheck/Makefile > abuse of it to set -DUSE_CRACKLIB.. I'll buy whatever version that works. I'm using CFLAGS at the moment, but could easily switch to other variables. Note that the missing bit here is the CXX part, adding PG_CFLAGS alone wouldn't help. Updated patch attached. 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
Attachment
Re: To Michael Paquier 2019-01-07 <20190107091734.GA1582@msg.credativ.de> > Updated patch attached. Here's another revision that doesn't add an extra CXXOPT variable but just extends CXXFLAGS whenever COPT or PROFILE are set, which seems more usable. It also updates the documentation. 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
Attachment
On Mon, Jan 07, 2019 at 10:32:20AM +0100, Christoph Berg wrote: > Here's another revision that doesn't add an extra CXXOPT variable but > just extends CXXFLAGS whenever COPT or PROFILE are set, which seems > more usable. > > It also updates the documentation. The documentation is not fully updated. It still lacks the description for each new field. -- Michael
Attachment
Hi, On 2019-01-07 10:32:20 +0100, Christoph Berg wrote: > Re: To Michael Paquier 2019-01-07 <20190107091734.GA1582@msg.credativ.de> > > Updated patch attached. > > Here's another revision that doesn't add an extra CXXOPT variable but > just extends CXXFLAGS whenever COPT or PROFILE are set, which seems > more usable. Why does that seem more usable? How's that supposed to be used for flags that aren't valid for both languages? Greetings, Andres Freund
Re: Andres Freund 2019-01-08 <20190108011837.n4mx7dadvojv2x55@alap3.anarazel.de> > > Here's another revision that doesn't add an extra CXXOPT variable but > > just extends CXXFLAGS whenever COPT or PROFILE are set, which seems > > more usable. > > Why does that seem more usable? How's that supposed to be used for flags > that aren't valid for both languages? The existing COPT and PROFILE are already catch-all type flags that add to CFLAGS and LDFLAGS. Possibly we should leave those alone and only add PG_CXXFLAGS and PG_LDFLAGS? 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
On Tue, Jan 08, 2019 at 09:09:56AM +0100, Christoph Berg wrote: > Re: Andres Freund 2019-01-08 <20190108011837.n4mx7dadvojv2x55@alap3.anarazel.de> >>> Here's another revision that doesn't add an extra CXXOPT variable but >>> just extends CXXFLAGS whenever COPT or PROFILE are set, which seems >>> more usable. >> >> Why does that seem more usable? How's that supposed to be used for flags >> that aren't valid for both languages? > > The existing COPT and PROFILE are already catch-all type flags that > add to CFLAGS and LDFLAGS. Possibly we should leave those alone and > only add PG_CXXFLAGS and PG_LDFLAGS? Personally I see pgxs as something completely different than what COPT and PROFILE are as we are talking about two different facilities: one which is part of the core installation, and the other which can be used for extension modules, so having PG_CFLAGS, PG_CXXFLAGS and PG_LDFLAGS, but leaving CXXFLAGS out of COPT and PROFILE looks like the better long-term move in terms of pluggability. My 2c. -- Michael
Attachment
On Thu, Jan 17, 2019 at 02:55:39PM +0900, Michael Paquier wrote: > Personally I see pgxs as something completely different than what COPT > and PROFILE are as we are talking about two different facilities: one > which is part of the core installation, and the other which can be > used for extension modules, so having PG_CFLAGS, PG_CXXFLAGS and > PG_LDFLAGS, but leaving CXXFLAGS out of COPT and PROFILE looks like > the better long-term move in terms of pluggability. My 2c. It's been a couple of days since this message, and while my opinion has not really changed, there are many other opinions. So perhaps we could reduce the proposal to a strict minimum and find an agreement for the options that we think are absolutely worth adding? Even if we cannot agree on what COPT of PROFILE should do more, perhaps we could still agree with only a portion of the flags we think are worth it? -- Michael
Attachment
On 2019-01-22 15:26:21 +0900, Michael Paquier wrote: > On Thu, Jan 17, 2019 at 02:55:39PM +0900, Michael Paquier wrote: > > Personally I see pgxs as something completely different than what COPT > > and PROFILE are as we are talking about two different facilities: one > > which is part of the core installation, and the other which can be > > used for extension modules, so having PG_CFLAGS, PG_CXXFLAGS and > > PG_LDFLAGS, but leaving CXXFLAGS out of COPT and PROFILE looks like > > the better long-term move in terms of pluggability. My 2c. > > It's been a couple of days since this message, and while my opinion > has not really changed, there are many other opinions. So perhaps we > could reduce the proposal to a strict minimum and find an agreement > for the options that we think are absolutely worth adding? Even if we > cannot agree on what COPT of PROFILE should do more, perhaps we could > still agree with only a portion of the flags we think are worth it? I think its plain wrong to add COPT to CXXFLAGS. Re PROFILE I'm on the fence. I personally think the pgxs stuff is a bit separate, and I'm doubtful we ought to backpatch that. I'm basically planning to apply https://www.postgresql.org/message-id/20190107091734.GA1582%40msg.credativ.de to 11-, minus the PGXS stuff. If we want that, we ought to apply it to master only IMO. Greetings, Andres Freund
On 2019-Jan-22, Andres Freund wrote: > I think its plain wrong to add COPT to CXXFLAGS. Re PROFILE I'm on the > fence. I personally think the pgxs stuff is a bit separate, and I'm > doubtful we ought to backpatch that. I'm basically planning to apply > https://www.postgresql.org/message-id/20190107091734.GA1582%40msg.credativ.de > to 11-, minus the PGXS stuff. If we want that, we ought to apply it to > master only IMO. I don't understand why you don't want to backpatch the PGXS bits. Is there something working today that would be broken by it? I think you're worried about places that invoke makefiles with PG_CXXFLAGS set and expecting the value not to be propagated. Is that a scenario we need to worry about? The patch neglects to update extend.sgml with the new pgxs variable, though. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
Hi, On 2019-01-22 17:10:58 -0300, Alvaro Herrera wrote: > On 2019-Jan-22, Andres Freund wrote: > > > I think its plain wrong to add COPT to CXXFLAGS. Re PROFILE I'm on the > > fence. I personally think the pgxs stuff is a bit separate, and I'm > > doubtful we ought to backpatch that. I'm basically planning to apply > > https://www.postgresql.org/message-id/20190107091734.GA1582%40msg.credativ.de > > to 11-, minus the PGXS stuff. If we want that, we ought to apply it to > > master only IMO. > > I don't understand why you don't want to backpatch the PGXS bits. Largely because I think it's an independent patch from the CXXOPT need from Christopher / Debian packaging. It's a larger patch, that needs more docs etc. If whoever applies that wants to backpatch it - I'm not going to protest, I just wouldn't myself, unless somebody pipes up that it'd help them. Greetings, Andres Freund
Hello On 2019-Jan-22, Andres Freund wrote: > On 2019-01-22 17:10:58 -0300, Alvaro Herrera wrote: > > I don't understand why you don't want to backpatch the PGXS bits. > > Largely because I think it's an independent patch from the CXXOPT need > from Christopher / Debian packaging. It's a larger patch, that needs > more docs etc. If whoever applies that wants to backpatch it - I'm not > going to protest, I just wouldn't myself, unless somebody pipes up that > it'd help them. Ah, I see. No arguments against. -- Álvaro Herrera https://www.2ndQuadrant.com/ PostgreSQL Development, 24x7 Support, Remote DBA, Training & Services
On Tue, Jan 22, 2019 at 06:11:23PM -0300, Alvaro Herrera wrote: > On 2019-Jan-22, Andres Freund wrote: >> Largely because I think it's an independent patch from the CXXOPT need >> from Christopher / Debian packaging. It's a larger patch, that needs >> more docs etc. If whoever applies that wants to backpatch it - I'm not >> going to protest, I just wouldn't myself, unless somebody pipes up that >> it'd help them. > > Ah, I see. No arguments against. Thanks Andres and Alvaro for the input. No issues with the way of Andres proposes. The new PGXS flags would be I think useful to make sure that things for CFLAGS and LDFLAGS get propagated without having to hijack the original flags, so I can handle that part. Which one would be wanted though? - PG_CXXFLAGS - PG_LDFLAGS - PG_CFLAGS I'd see value in all of them, still everybody has likely a different opinion, so I would not mind discarding the ones are not thought as that much useful. New PGXS infrastructure usually finds only its way on HEAD, so I'd rather not back-patch that part. No issues with the back-patch portion for CXXOPT from me as that helps Debian. Thanks, -- Michael
Attachment
Re: Michael Paquier 2019-01-23 <20190123004722.GE3873@paquier.xyz> > >> Largely because I think it's an independent patch from the CXXOPT need > >> from Christopher / Debian packaging. It's a larger patch, that needs > >> more docs etc. If whoever applies that wants to backpatch it - I'm not > >> going to protest, I just wouldn't myself, unless somebody pipes up that > >> it'd help them. > > > > Ah, I see. No arguments against. Fwiw I'm not attached to using COPT and friends, I just happend to pick these because that was one way that worked. With what I've learned now, PG_*FLAGS is the better approach. > The new PGXS flags would be I think useful to make sure > that things for CFLAGS and LDFLAGS get propagated without having to > hijack the original flags, so I can handle that part. Which one would > be wanted though? > - PG_CXXFLAGS > - PG_LDFLAGS > - PG_CFLAGS > > I'd see value in all of them, still everybody has likely a different > opinion, so I would not mind discarding the ones are not thought as > that much useful. New PGXS infrastructure usually finds only its way > on HEAD, so I'd rather not back-patch that part. No issues with the > back-patch portion for CXXOPT from me as that helps Debian. The attached patch adds these three. Re backpatching, I would at least need them in PG11 because that's what is going to be released with Debian buster. An official backpatch to all supported versions would be nice, but I could also sneak in that change into the Debian packages without breaking anything. 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
Attachment
On Tue, Jan 29, 2019 at 04:18:46PM +0100, Christoph Berg wrote: > Re backpatching, I would at least need them in PG11 because that's > what is going to be released with Debian buster. An official backpatch > to all supported versions would be nice, but I could also sneak in > that change into the Debian packages without breaking anything. The change is non-invasive, so if that helps to make packaging easier and more consistent as a whole, then I could do the back-patch. Except if somebody has an objection? -- Michael
Attachment
Hi, On 2019-01-29 16:18:46 +0100, Christoph Berg wrote: > Re: Michael Paquier 2019-01-23 <20190123004722.GE3873@paquier.xyz> > > >> Largely because I think it's an independent patch from the CXXOPT need > > >> from Christopher / Debian packaging. It's a larger patch, that needs > > >> more docs etc. If whoever applies that wants to backpatch it - I'm not > > >> going to protest, I just wouldn't myself, unless somebody pipes up that > > >> it'd help them. > > > > > > Ah, I see. No arguments against. > > Fwiw I'm not attached to using COPT and friends, I just happend to > pick these because that was one way that worked. With what I've > learned now, PG_*FLAGS is the better approach. I'm confused - that doesn't allow to inject flags to all in-core built files? So how does that fix your problem fully? Say e.g. llvmjit_inline.cpp won't get the flag, and thus not be reproducible? Greetings, Andres Freund
Re: Andres Freund 2019-01-30 <20190130015127.hciz36lpmu7prrx2@alap3.anarazel.de> > I'm confused - that doesn't allow to inject flags to all in-core built > files? So how does that fix your problem fully? Say > e.g. llvmjit_inline.cpp won't get the flag, and thus not be > reproducible? The original itch being scratched here is extensions written in C++, which need -ffile-prefix-map (or -fdebug-prefix-map) injected into CXXFLAGS on top of the flags encoded in pgxs.mk. I originally picked COPT because that works for injecting flags into pgxs.mk as well, but PG_*FLAGS is better. For the server build itself, it shouldn't be necessary to inject flags because they are passed directly to ./configure. Admittedly I haven't looked at reproducibility of PG11 yet as clang is still missing the prefix-map feature, but https://reviews.llvm.org/D49466 is making good progress. Do we still want some CXXOPT flag for the server build? I can write a patch, but someone else would need to do the bikeshedding how to name it, and which of the existing knobs would set CXXFLAGS along. I don't think I need that feature. 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
On Wed, Jan 30, 2019 at 02:41:01PM +0100, Christoph Berg wrote: > Do we still want some CXXOPT flag for the server build? I can write a > patch, but someone else would need to do the bikeshedding how to name > it, and which of the existing knobs would set CXXFLAGS along. I don't > think I need that feature. If we don't directly need it, let's not add it now but let's revisit the need if it proves necessary. I propose to just commit the last patch you sent, and back-patch to ease integration with existing extensions. Any objections? -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> On Wed, Jan 30, 2019 at 02:41:01PM +0100, Christoph Berg wrote:
>> Do we still want some CXXOPT flag for the server build? I can write a
>> patch, but someone else would need to do the bikeshedding how to name
>> it, and which of the existing knobs would set CXXFLAGS along. I don't
>> think I need that feature.
> If we don't directly need it, let's not add it now but let's revisit
> the need if it proves necessary.
+1
> I propose to just commit the last
> patch you sent, and back-patch to ease integration with existing
> extensions. Any objections?
A thought here:
ifdef PG_CPPFLAGS
override CPPFLAGS := $(PG_CPPFLAGS) $(CPPFLAGS)
endif
+ifdef PG_CFLAGS
+override CFLAGS := $(PG_CFLAGS) $(CFLAGS)
+endif
+ifdef PG_CXXFLAGS
+override CXXFLAGS := $(PG_CXXFLAGS) $(CXXFLAGS)
+endif
+ifdef PG_LDFLAGS
+override LDFLAGS := $(PG_LDFLAGS) $(LDFLAGS)
+endif
This looks a bit copy-and-paste-y to me, in particular no thought
has been taken for the order of flags. We found in configure that
it's better to add user-specified CFLAGS at the *end*, even though
injecting user-specified CPPFLAGS at the beginning is the right
thing. This is because in, eg, "-O2 -O0" the last flag wins.
Presumably the same goes for CXXFLAGS. I think it's right to
put user LDFLAGS first, though. (The argument for CPPFLAGS and
LDFLAGS is that you want the user's -I and -L flags to go first.)
regards, tom lane
On Wed, Jan 30, 2019 at 08:18:31PM -0500, Tom Lane wrote: > This looks a bit copy-and-paste-y to me, in particular no thought > has been taken for the order of flags. We found in configure that > it's better to add user-specified CFLAGS at the *end*, even though > injecting user-specified CPPFLAGS at the beginning is the right > thing. This is because in, eg, "-O2 -O0" the last flag wins. > Presumably the same goes for CXXFLAGS. I think it's right to > put user LDFLAGS first, though. (The argument for CPPFLAGS and > LDFLAGS is that you want the user's -I and -L flags to go first.) Ah yes, good point about CFLAGS and LDFLAGS. It would be better to add a comment about that and document the difference, aka "prepend" or "append" the flag values. CXXFLAGS applies to compiler options like -g -O2 which you would like to enforce for the C++ compiler, so it seems to me that like CFLAGS the custom values should be added at the end and not at the beginning, no? -- Michael
Attachment
On Thu, Jan 31, 2019 at 11:19:11AM +0900, Michael Paquier wrote: > Ah yes, good point about CFLAGS and LDFLAGS. It would be better to > add a comment about that and document the difference, aka "prepend" or > "append" the flag values. > > CXXFLAGS applies to compiler options like -g -O2 which you would like > to enforce for the C++ compiler, so it seems to me that like CFLAGS > the custom values should be added at the end and not at the beginning, > no? Attached is a patch doing that. Thoughts? -- Michael
Attachment
Michael Paquier <michael@paquier.xyz> writes:
> Attached is a patch doing that. Thoughts?
WFM.
regards, tom lane
On Thu, Jan 31, 2019 at 10:51:11PM -0500, Tom Lane wrote: > Michael Paquier <michael@paquier.xyz> writes: >> Attached is a patch doing that. Thoughts? > > WFM. Thanks, pushed. -- Michael
Attachment
Re: Michael Paquier 2019-02-03 <20190203090737.GA18892@paquier.xyz> > >> Attached is a patch doing that. Thoughts? > > > > WFM. > > Thanks, pushed. Thanks. It makes much more sense that way round. 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