From patchwork Thu Oct 19 10:41:35 2017 Content-Type: text/plain; charset="utf-8" MIME-Version: 1.0 Content-Transfer-Encoding: 7bit X-Patchwork-Submitter: Jeff Layton X-Patchwork-Id: 10016397 Return-Path: Received: from mail.wl.linuxfoundation.org (pdx-wl-mail.web.codeaurora.org [172.30.200.125]) by pdx-korg-patchwork.web.codeaurora.org (Postfix) with ESMTP id C08C760215 for ; Thu, 19 Oct 2017 10:41:41 +0000 (UTC) Received: from mail.wl.linuxfoundation.org (localhost [127.0.0.1]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id C311228CD1 for ; Thu, 19 Oct 2017 10:41:41 +0000 (UTC) Received: by mail.wl.linuxfoundation.org (Postfix, from userid 486) id B7BEC28CD3; Thu, 19 Oct 2017 10:41:41 +0000 (UTC) X-Spam-Checker-Version: SpamAssassin 3.3.1 (2010-03-16) on pdx-wl-mail.web.codeaurora.org X-Spam-Level: X-Spam-Status: No, score=-6.3 required=2.0 tests=BAYES_00,DKIM_SIGNED, RCVD_IN_DNSWL_HI, RCVD_IN_SORBS_SPAM, T_DKIM_INVALID autolearn=ham version=3.3.1 Received: from vger.kernel.org (vger.kernel.org [209.132.180.67]) by mail.wl.linuxfoundation.org (Postfix) with ESMTP id E943128CD1 for ; Thu, 19 Oct 2017 10:41:40 +0000 (UTC) Received: (majordomo@vger.kernel.org) by vger.kernel.org via listexpand id S1751892AbdJSKlk (ORCPT ); Thu, 19 Oct 2017 06:41:40 -0400 Received: from mail-yw0-f195.google.com ([209.85.161.195]:47065 "EHLO mail-yw0-f195.google.com" rhost-flags-OK-OK-OK-OK) by vger.kernel.org with ESMTP id S1751311AbdJSKli (ORCPT ); Thu, 19 Oct 2017 06:41:38 -0400 Received: by mail-yw0-f195.google.com with SMTP id t71so3079629ywc.3 for ; Thu, 19 Oct 2017 03:41:38 -0700 (PDT) DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=poochiereds-net.20150623.gappssmtp.com; s=20150623; h=message-id:subject:from:to:cc:date:in-reply-to:references :mime-version:content-transfer-encoding; bh=mZlOqofayJBcbggJwviYui+l4oitzlCrhuBFhTDlbhg=; b=yUfjyA3b6mLgvQvKPN2A7D7HHwICyTzdivOx2ZPjDeJbz5X11d5vIpY7s6uZuT2feO AKRu3QxcSaSd71ClKyoe/s+DpXfZk3A2X0biBrNJsu1KOGk+G4xkA/bm6iHdvN7kf8q6 OFs6eGqVohb8JVSAVx8/pCI1lyCUdjF6T/K0U/MJv917Q43Wv2lLkCy2sECnBg/L7g1Q 4W5ISiMl0Sb9MeIZYuoyi7rwXNifKpVmnYzOReyff4RiAwVKxBk57WCwM1IgS3TSB7t+ A5onJvFEeXTfURfxlhjVQTYqB5Jw6pHRP4jRYFMyxTNFGBCoxhmUySPsICE09va+Hdaf KXGA== X-Google-DKIM-Signature: v=1; a=rsa-sha256; c=relaxed/relaxed; d=1e100.net; s=20161025; h=x-gm-message-state:message-id:subject:from:to:cc:date:in-reply-to :references:mime-version:content-transfer-encoding; bh=mZlOqofayJBcbggJwviYui+l4oitzlCrhuBFhTDlbhg=; b=No7kqUnZSbsrHizLSqIaIX5/XLlgyKf/5v/fWUP4VDc965Qe+ehog3oLG7kleOfkQL 7qlrhvJv7XYd+JvoMHmiFgCqknWtsAg6t0q0+TD2JG1RVxT2LzHaSNPYHTF4a59+JIG5 VCxD7M6OEBqeGDfZAcaPXAz68yhVEFfH+23ctezQB59QOkm69ddM/F94FNylpY33RcTp A1Aq/RjcUBv6axL44uF148NRx7sMTPSKbB9MRqJxaX8CEGRm2UwyL4gAZ8js46dctc8h nTmW8Osb7db4ENK6I0sM1rv9UIg7RLbsKcTdDidA4vPC98yTavowj5OMFxFNKM4bx/qd R5OQ== X-Gm-Message-State: AMCzsaX4PI/T8TlMtVV2O6ZtFct+wJGudrU/F2E+1LFBC1V32nJxCBG2 ae/2VZkF9w4TgkRgvp+ZEZGc2Q== X-Google-Smtp-Source: ABhQp+R927zBBcwBigZQkg4oQrY83PNz1gn5t788dp97oay2FL+rLlFyyk51ZL2Ji8khnr2YpYbFjQ== X-Received: by 10.129.183.21 with SMTP id v21mr620390ywh.349.1508409697995; Thu, 19 Oct 2017 03:41:37 -0700 (PDT) Received: from tleilax.poochiereds.net (cpe-71-70-156-158.nc.res.rr.com. [71.70.156.158]) by smtp.gmail.com with ESMTPSA id u134sm5505923ywc.40.2017.10.19.03.41.36 (version=TLS1_2 cipher=ECDHE-RSA-CHACHA20-POLY1305 bits=256/256); Thu, 19 Oct 2017 03:41:37 -0700 (PDT) Message-ID: <1508409695.4912.7.camel@poochiereds.net> Subject: Re: [PATCH] build: assign extra flags to ALL_CFLAGS instead of CFLAGS From: Jeff Layton To: Christopher Li Cc: Linux-Sparse , Luc Van Oostenryck Date: Thu, 19 Oct 2017 06:41:35 -0400 In-Reply-To: References: <20171018150727.26821-1-jlayton@kernel.org> <1508358019.4628.35.camel@poochiereds.net> <1508377663.19876.20.camel@poochiereds.net> X-Mailer: Evolution 3.24.6 (3.24.6-1.fc26) Mime-Version: 1.0 Sender: linux-sparse-owner@vger.kernel.org Precedence: bulk List-ID: X-Mailing-List: linux-sparse@vger.kernel.org X-Virus-Scanned: ClamAV using ClamSMTP On Wed, 2017-10-18 at 19:25 -0700, Christopher Li wrote: > On Wed, Oct 18, 2017 at 6:47 PM, Jeff Layton wrote: > > > > No, it wasn't intentional. Could we just turn that into a += > > assignment? > > No, turn that into += does not work either. The variable come from > command line has the $(origin varname) set to "command line". > make will ignore normal modification to that variable, including "+=" > > If you really need to make modification of that variable. > You need to use the "override" directive. > > https://www.gnu.org/software/make/manual/make.html > > 6.7 The override Directive > > override CFLAGS += "...." > > For that reason, I would suggest using a different variable > to assign from the command line. > > > > > Got it, thanks. Basically I just need a way to pass in a basic set of > > flags to gcc, that are either appended or prepended to whatever you > > need to have in there. If we need to call it something other than > > CFLAGS, then that's fine. > > Right. Given any makefile, I can always pick some internal variable, > assign that variable from command line, then the make process does > not work as intended. In our case, CFLAGS is such a variable. > > I would suggest have one variable like CFLAGS_CMD to get > overwrite from command line. You can suggest the variable name. > > You should also examine your rpmbuild script for other open > source packages. Weather they suffer from the same CFLAGS overwrite > problems. > I don't think it's generally a problem. Most of them use autotools or cmake, which handle this correctly. This is mainly an issue with hand- rolled makefiles. > Again, patch is welcome. Ok, how about this instead? This changes the makefile to just use BASIC_CFLAGS internally. If CFLAGS is specified on the command line, it's appended to the list of options. This allows us to stick with passing in CFLAGS= to make during the build process: ---------------------8<------------------------- [PATCH] build: clean up CFLAGS handling in the makefile The fedora packaging tools want to override $CFLAGS when building sparse, but that fails on a couple of targets. There are a couple of build targets in the makefile that want to add options to $CFLAGS. When we do a build though, we pass $ALL_CFLAGS to the compiler, and that ends up without those extra options, if CFLAGS= was specified on the command line. Change the code to append the options to ALL_CFLAGS instead of CFLAGS. At the same time, passing a CFLAGS argument to make ends up the initial CFLAGS assignment being clobbered such that we lose the options that are assigned to it internally. Fold the internal usage of CFLAGS into BASIC_CFLAGS. They both just end up as part of ALL_CFLAGS anyway, so this should be functionally equivalent. Cc: Luc Van Oostenryck Signed-off-by: Jeff Layton --- Makefile | 12 ++++++------ 1 file changed, 6 insertions(+), 6 deletions(-) diff --git a/Makefile b/Makefile index d0341764158e..335dcfff54ce 100644 --- a/Makefile +++ b/Makefile @@ -12,8 +12,8 @@ OS = linux CC = gcc -CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g -CFLAGS += -Wall -Wwrite-strings +BASIC_CFLAGS = -O2 -finline-functions -fno-strict-aliasing -g +BASIC_CFLAGS += -Wall -Wwrite-strings LDFLAGS += -g LD = gcc AR = ar @@ -21,7 +21,7 @@ PKG_CONFIG = pkg-config CHECKER = ./cgcc -no-compile CHECKER_FLAGS = -ALL_CFLAGS = $(CFLAGS) $(BASIC_CFLAGS) +ALL_CFLAGS = $(BASIC_CFLAGS) $(CFLAGS) # # For debugging, put this in local.mk: # @@ -44,7 +44,7 @@ LLVM_CONFIG:=llvm-config HAVE_LLVM:=$(shell $(LLVM_CONFIG) --version >/dev/null 2>&1 && echo 'yes') GCC_BASE := $(shell $(CC) --print-file-name=) -BASIC_CFLAGS = -DGCC_BASE=\"$(GCC_BASE)\" +BASIC_CFLAGS += -DGCC_BASE=\"$(GCC_BASE)\" MULTIARCH_TRIPLET := $(shell $(CC) -print-multiarch 2>/dev/null) BASIC_CFLAGS += -DMULTIARCH_TRIPLET=\"$(MULTIARCH_TRIPLET)\" @@ -83,7 +83,7 @@ PROGRAMS += test-inspect INST_PROGRAMS += test-inspect test-inspect_EXTRA_DEPS := ast-model.o ast-view.o ast-inspect.o test-inspect_OBJS := test-inspect.o $(test-inspect_EXTRA_DEPS) -$(test-inspect_OBJS) $(test-inspect_OBJS:.o=.sc): CFLAGS += $(GTK_CFLAGS) +$(test-inspect_OBJS) $(test-inspect_OBJS:.o=.sc): ALL_CFLAGS += $(GTK_CFLAGS) test-inspect_EXTRA_OBJS := $(GTK_LIBS) else $(warning Your system does not have gtk3/gtk2, disabling test-inspect) @@ -208,7 +208,7 @@ ifneq ($(DEP_FILES),) include $(DEP_FILES) endif -c2xml.o c2xml.sc: CFLAGS += $(LIBXML_CFLAGS) +c2xml.o c2xml.sc: ALL_CFLAGS += $(LIBXML_CFLAGS) pre-process.sc: CHECKER_FLAGS += -Wno-vla