| Message ID | 20190212193350.25239-1-uwe@kleine-koenig.org (mailing list archive) |
|---|---|
| State | Superseded, archived |
| Headers | show |
| Series | honor CFLAGS from environment | expand |
On 12/02/2019 19:33, Uwe Kleine-König wrote: > Debian build scripts pass CFLAGS in the environment. To honor these only set > CFLAGS to "-O2 -g" if CFLAGS is unset. The warnings in the following line > are added unconditionally. This makes sparse builds reproducible. > > Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> > > --- a/Makefile > +++ b/Makefile > @@ -6,7 +6,7 @@ > > > CC = gcc-8 > -CFLAGS = -O2 -g > +CFLAGS ?= -O2 -g > CFLAGS += -Wall -Wwrite-strings > LD = $(CC) > LDFLAGS += -Wl,--as-needed > What version of Makefile is this? It does not apply to the 'master' branch. For example 'CC = gcc-8' and 'LDFLAGS += -Wl,--as-needed' don't appear master:Makefile. Otherwise, this does indeed make setting CFLAGS in the environment easier and doesn't make setting it on the command-line any worse, so ... ATB, Ramsay Jones
Hello, On 2/12/19 9:11 PM, Ramsay Jones wrote: > On 12/02/2019 19:33, Uwe Kleine-König wrote: >> Debian build scripts pass CFLAGS in the environment. To honor these only set >> CFLAGS to "-O2 -g" if CFLAGS is unset. The warnings in the following line >> are added unconditionally. This makes sparse builds reproducible. >> >> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> >> >> --- a/Makefile >> +++ b/Makefile >> @@ -6,7 +6,7 @@ >> >> >> CC = gcc-8 >> -CFLAGS = -O2 -g >> +CFLAGS ?= -O2 -g >> CFLAGS += -Wall -Wwrite-strings >> LD = $(CC) >> LDFLAGS += -Wl,--as-needed >> > > What version of Makefile is this? It does not apply to > the 'master' branch. > > For example 'CC = gcc-8' and 'LDFLAGS += -Wl,--as-needed' > don't appear master:Makefile. Argh, I forgot that I had some patches applied (from the Debian package). > Otherwise, this does indeed make setting CFLAGS in the > environment easier and doesn't make setting it on the > command-line any worse, so ... Is this an Ack? Best regards Uwe
On 12/02/2019 20:12, Uwe Kleine-König wrote: > Hello, > > On 2/12/19 9:11 PM, Ramsay Jones wrote: >> On 12/02/2019 19:33, Uwe Kleine-König wrote: >>> Debian build scripts pass CFLAGS in the environment. To honor these only set >>> CFLAGS to "-O2 -g" if CFLAGS is unset. The warnings in the following line >>> are added unconditionally. This makes sparse builds reproducible. >>> >>> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> >>> >>> --- a/Makefile >>> +++ b/Makefile >>> @@ -6,7 +6,7 @@ >>> >>> >>> CC = gcc-8 >>> -CFLAGS = -O2 -g >>> +CFLAGS ?= -O2 -g >>> CFLAGS += -Wall -Wwrite-strings >>> LD = $(CC) >>> LDFLAGS += -Wl,--as-needed >>> >> >> What version of Makefile is this? It does not apply to >> the 'master' branch. >> >> For example 'CC = gcc-8' and 'LDFLAGS += -Wl,--as-needed' >> don't appear master:Makefile. > > Argh, I forgot that I had some patches applied (from the Debian package). > >> Otherwise, this does indeed make setting CFLAGS in the >> environment easier and doesn't make setting it on the >> command-line any worse, so ... > > Is this an Ack? No. Just a mild encouragement to re-roll the patch on a clean base. Also, "The warnings in the following line are added unconditionally." in the commit message is far from clear. (when CFLAGS is set from the environment, the += append happens without problem, but you seem to be saying that these additions will be added from the environment at the same time?) Do you also set any other variables in the environment (eg. LDFLAGS)? ATB, Ramsay Jones
Hello, On 2/12/19 9:36 PM, Ramsay Jones wrote: > On 12/02/2019 20:12, Uwe Kleine-König wrote: >> On 2/12/19 9:11 PM, Ramsay Jones wrote: >>> On 12/02/2019 19:33, Uwe Kleine-König wrote: >>>> Debian build scripts pass CFLAGS in the environment. To honor these only set >>>> CFLAGS to "-O2 -g" if CFLAGS is unset. The warnings in the following line >>>> are added unconditionally. This makes sparse builds reproducible. >>>> >>>> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> >>>> >>>> --- a/Makefile >>>> +++ b/Makefile >>>> @@ -6,7 +6,7 @@ >>>> >>>> >>>> CC = gcc-8 >>>> -CFLAGS = -O2 -g >>>> +CFLAGS ?= -O2 -g >>>> CFLAGS += -Wall -Wwrite-strings >>>> LD = $(CC) >>>> LDFLAGS += -Wl,--as-needed >>>> >>> >>> What version of Makefile is this? It does not apply to >>> the 'master' branch. >>> >>> For example 'CC = gcc-8' and 'LDFLAGS += -Wl,--as-needed' >>> don't appear master:Makefile. >> >> Argh, I forgot that I had some patches applied (from the Debian package). >> >>> Otherwise, this does indeed make setting CFLAGS in the >>> environment easier and doesn't make setting it on the >>> command-line any worse, so ... >> >> Is this an Ack? > > No. Just a mild encouragement to re-roll the patch on > a clean base. > > Also, "The warnings in the following line are added > unconditionally." in the commit message is far from > clear. (when CFLAGS is set from the environment, the > += append happens without problem, but you seem to > be saying that these additions will be added from the > environment at the same time?) Will fix when respinning the patch. Thanks for your input. > Do you also set any other variables in the environment > (eg. LDFLAGS)? Yes, the packages have the following variables set in their build environment: CFLAGS, CPPFLAGS, CXXFLAGS, FCFLAGS, FFLAGS, GCJFLAGS, LDFLAGS, OBJCFLAGS, OBJCXXFLAGS. Apart from CFLAGS these is no problem. Best regards Uwe
On Tue, Feb 12, 2019 at 09:41:47PM +0100, Uwe Kleine-König wrote: > On 2/12/19 9:36 PM, Ramsay Jones wrote: > > On 12/02/2019 20:12, Uwe Kleine-König wrote: > >> On 2/12/19 9:11 PM, Ramsay Jones wrote: > >>> On 12/02/2019 19:33, Uwe Kleine-König wrote: > >>>> Debian build scripts pass CFLAGS in the environment. To honor these only set > >>>> CFLAGS to "-O2 -g" if CFLAGS is unset. Last year or so, when I changed most of the makefile, I understood that these were set only via the command line. But sure, it's certainly convenient to set them via the env. > >>>> Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org> > >>>> > >>>> --- a/Makefile > >>>> +++ b/Makefile > >>>> @@ -6,7 +6,7 @@ > >>>> > >>>> > >>>> CC = gcc-8 > >>>> -CFLAGS = -O2 -g > >>>> +CFLAGS ?= -O2 -g > >>>> CFLAGS += -Wall -Wwrite-strings > >>>> LD = $(CC) > >>>> LDFLAGS += -Wl,--as-needed > >>>> ... > >>> Otherwise, this does indeed make setting CFLAGS in the > >>> environment easier and doesn't make setting it on the > >>> command-line any worse, so ... Indeed. > >> Is this an Ack? > > > > No. Just a mild encouragement to re-roll the patch on > > a clean base. > > > > Also, "The warnings in the following line are added > > unconditionally." in the commit message is far from > > clear. (when CFLAGS is set from the environment, the > > += append happens without problem, but you seem to > > be saying that these additions will be added from the > > environment at the same time?) It's OK to me if you need to change it to: CFLAGS ?= -O2 -g -Wall -Wwrite-strings because the "-Wall -Wwrite-strings" create reproducibility problems but, if only for my own curiosity, I would like to understand what exactly the problem is. > > Do you also set any other variables in the environment > > (eg. LDFLAGS)? > > Yes, the packages have the following variables set in their build > environment: CFLAGS, CPPFLAGS, CXXFLAGS, FCFLAGS, FFLAGS, GCJFLAGS, > LDFLAGS, OBJCFLAGS, OBJCXXFLAGS. Apart from CFLAGS these is no problem. Yes but for me CC, LD, AR, PKG_CONFIG, ... MANDIR have potentially exactly the same problem (except probably CHECKER_FLAGS that could be considered as internal). but I have no problem to init all of them with ?= like it was asked for PREFIX. Best regards, -- Luc
Hello Luc, On Tue, Feb 12, 2019 at 11:37:05PM +0100, Luc Van Oostenryck wrote: > On Tue, Feb 12, 2019 at 09:41:47PM +0100, Uwe Kleine-König wrote: > > On 2/12/19 9:36 PM, Ramsay Jones wrote: > > > On 12/02/2019 20:12, Uwe Kleine-König wrote: > > >> On 2/12/19 9:11 PM, Ramsay Jones wrote: > > >>> On 12/02/2019 19:33, Uwe Kleine-König wrote: > > >>>> Debian build scripts pass CFLAGS in the environment. To honor these only set > > >>>> CFLAGS to "-O2 -g" if CFLAGS is unset. > > > Also, "The warnings in the following line are added > > > unconditionally." in the commit message is far from > > > clear. (when CFLAGS is set from the environment, the > > > += append happens without problem, but you seem to > > > be saying that these additions will be added from the > > > environment at the same time?) > > It's OK to me if you need to change it to: > CFLAGS ?= -O2 -g -Wall -Wwrite-strings > because the "-Wall -Wwrite-strings" create reproducibility > problems but, if only for my own curiosity, I would like > to understand what exactly the problem is. No, it's not that "-Wall -Wwrite-strings" creates problems with reproducibility. The problem is that CFLAGS from the environment are ignored and there we have (among others) -fdebug-prefix-map=$(pwd)=. and without that the build path leaks into the debug info. So your build artifacts include the string "/home/luc/gitsources/sparse" while mine have "/home/uwe/debpkg/sparse" and the Debian buildds have something with "/home/buildd" IIRC. Also note that with my patch the following is passed to the compiler: $WHATEVERCFLAGSWASSETTOINTHEENVIRONMENT -Wall -Wwrite-strings I don't care much if "-Wall -Wwrite-strings" is added to the value that was provided via the environment or not. Probably it would be consequent to not add it. > > > Do you also set any other variables in the environment > > > (eg. LDFLAGS)? > > > > Yes, the packages have the following variables set in their build > > environment: CFLAGS, CPPFLAGS, CXXFLAGS, FCFLAGS, FFLAGS, GCJFLAGS, > > LDFLAGS, OBJCFLAGS, OBJCXXFLAGS. Apart from CFLAGS these is no problem. > > Yes but for me CC, LD, AR, PKG_CONFIG, ... MANDIR have potentially > exactly the same problem (except probably CHECKER_FLAGS that > could be considered as internal). but I have no problem to init > all of them with ?= like it was asked for PREFIX. That would be just fine. Best regards Uwe
Hi, On Wed, Feb 13, 2019 at 08:39:45AM +0100, Uwe Kleine-König wrote: > On Tue, Feb 12, 2019 at 11:37:05PM +0100, Luc Van Oostenryck wrote: > > On Tue, Feb 12, 2019 at 09:41:47PM +0100, Uwe Kleine-König wrote: > > > On 2/12/19 9:36 PM, Ramsay Jones wrote: > > > > On 12/02/2019 20:12, Uwe Kleine-König wrote: > > > >> On 2/12/19 9:11 PM, Ramsay Jones wrote: > > > >>> On 12/02/2019 19:33, Uwe Kleine-König wrote: > > > >>>> Debian build scripts pass CFLAGS in the environment. To honor these only set > > > >>>> CFLAGS to "-O2 -g" if CFLAGS is unset. > > > > Also, "The warnings in the following line are added > > > > unconditionally." in the commit message is far from > > > > clear. (when CFLAGS is set from the environment, the > > > > += append happens without problem, but you seem to > > > > be saying that these additions will be added from the > > > > environment at the same time?) > > > > It's OK to me if you need to change it to: > > CFLAGS ?= -O2 -g -Wall -Wwrite-strings > > because the "-Wall -Wwrite-strings" create reproducibility > > problems but, if only for my own curiosity, I would like > > to understand what exactly the problem is. > > No, it's not that "-Wall -Wwrite-strings" creates problems with > reproducibility. The problem is that CFLAGS from the environment are > ignored and there we have (among others) > > -fdebug-prefix-map=$(pwd)=. > > and without that the build path leaks into the debug info. OK, I see. The reproducibility problem is just one aspect of the environment being ignored. > Also note that with my patch the following is passed to the compiler: > > $WHATEVERCFLAGSWASSETTOINTHEENVIRONMENT -Wall -Wwrite-strings > > I don't care much if "-Wall -Wwrite-strings" is added to the value that > was provided via the environment or not. Probably it would be consequent > to not add it. OK. > > > > Do you also set any other variables in the environment > > > > (eg. LDFLAGS)? > > > > > > Yes, the packages have the following variables set in their build > > > environment: CFLAGS, CPPFLAGS, CXXFLAGS, FCFLAGS, FFLAGS, GCJFLAGS, > > > LDFLAGS, OBJCFLAGS, OBJCXXFLAGS. Apart from CFLAGS these is no problem. > > > > Yes but for me CC, LD, AR, PKG_CONFIG, ... MANDIR have potentially > > exactly the same problem (except probably CHECKER_FLAGS that > > could be considered as internal). but I have no problem to init > > all of them with ?= like it was asked for PREFIX. > > That would be just fine. I'll do so then. Thanks for the explanation, -- Luc
--- a/Makefile +++ b/Makefile @@ -6,7 +6,7 @@ CC = gcc-8 -CFLAGS = -O2 -g +CFLAGS ?= -O2 -g CFLAGS += -Wall -Wwrite-strings LD = $(CC) LDFLAGS += -Wl,--as-needed
Debian build scripts pass CFLAGS in the environment. To honor these only set CFLAGS to "-O2 -g" if CFLAGS is unset. The warnings in the following line are added unconditionally. This makes sparse builds reproducible. Signed-off-by: Uwe Kleine-König <uwe@kleine-koenig.org>