On Mon, Oct 30, 2017 at 05:53:49AM +0800, Christopher Li wrote: > On Mon, Oct 30, 2017 at 1:28 AM, Luc Van Oostenryck > <luc.vanoostenryck@xxxxxxxxx> wrote: > > The goal of the initial patch, as stated by Jeff, was: > > "The fedora packaging tools want to *override* $CFLAGS when building sparse" > > > > This version doesn't allow to override CFLAGS. > > See what happens with 'make CFLAGS=-O3' for example. > > The original patch proposed by Jeff does not allow usage of > "make CFLAGS=-O3" either. I take a look at your patch, it behave > the same way in this regard. Do you mean I should change the > commit title to *append* CFLAGS instead of override? > > May be Jeff can clarify the intention? > > BTW, I am actually leaning toward just expose some thing like > CLI_CFLAGS to be override by package builders. I am not that > convince override CFLAGS is established stander practices from > the project that I have look at. `make CFLAGS='-arbitrary -flags'` is quite common, and people generally expect it to work, though it isn't *shocking* when it breaks. Though the exact boundary for what should get overridden by that and what should remain seems a bit fuzzy. Roughly speaking, if the application normally wants to build with "-O2 -ffooish-bar -Wall -Wanother -I/some/path" , then `make CFLAGS='-Os -arbitrary -flags'` should build with "-Os -arbitrary -flags -Wall -Wanother -Isome-path`. I think. > On the other hand, it is trivial to avoid using CFLAGS in sparse > Makefile. If Jeff really want CFLAGS instead of CLI_CFLAGS, > it is not a big a deal. I think it's worth not putting anything into CFLAGS that would break the build if missing. For instance, git's Makefile specifically says "# CFLAGS and LDFLAGS are for the users to override from the command line.", and then just has "CFLAGS = -g -O2 -Wall". It then has additional variables like ALL_CFLAGS that include both CFLAGS and other things. -- To unsubscribe from this list: send the line "unsubscribe linux-sparse" in the body of a message to majordomo@xxxxxxxxxxxxxxx More majordomo info at http://vger.kernel.org/majordomo-info.html
