Re: [PATCH 1/2] Makefile: allow overriding CFLAGS on the command line

[Andre Przywara <andre.przywara@arm.com>]

Hi Riku,

On 04/11/15 10:02, Riku Voipio wrote:
> On 30 October 2015 at 19:20, Andre Przywara <andre.przywara@arm.com> wrote:
>> When a Makefile variable is set on the make command line, all
>> Makefile-internal assignments to that very variable are _ignored_.
>> Since we add quite some essential values to CFLAGS internally,
>> specifying some CFLAGS on the command line will usually break the
>> build (and not fix any include file problems you hoped to overcome
>> with that).
>> Somewhat against intuition GNU make provides the "override" directive
>> to change this behavior; with that assignments in the Makefile get
>> _appended_ to the value given on the command line. [1]
>>
>> Change any internal assignments to use that directive, so that a user
>> can use:
>> $ make CFLAGS=/path/to/my/include/dir
>> to teach kvmtool about non-standard header file locations (helpful
>> for cross-compilation) or to tweak other compiler options.
>>
>> Signed-off-by: Andre Przywara <andre.przywara@arm.com>
>>
>> [1] https://www.gnu.org/software/make/manual/html_node/Override-Directive.html
>> ---
>>  Makefile | 15 +++++++--------
>>  1 file changed, 7 insertions(+), 8 deletions(-)
>>
>> diff --git a/Makefile b/Makefile
>> index f8f7cc4..77a7c9f 100644
>> --- a/Makefile
>> +++ b/Makefile
>> @@ -15,9 +15,7 @@ include config/utilities.mak
>>  include config/feature-tests.mak
>>
>>  CC     := $(CROSS_COMPILE)gcc
>> -CFLAGS :=
>>  LD     := $(CROSS_COMPILE)ld
>> -LDFLAGS        :=
> 
> This breaks builds of debian packages as dpkg-buildpackage sets LDFLAGS
> to something unsuitable for guest init.
> 
> Looks like this has been an issue before:

> 
> commit 57fa349a9792a629e4ed2d89e1309cc96dcc39af
> Author: Will Deacon <will.deacon@arm.com>
> Date:   Thu Jun 4 16:25:36 2015 +0100
> 
>     Don't inherit CFLAGS and LDFLAGS from the environment
> 
>     kvmtool doesn't build with arbitrary flags, so let's clear CFLAGS and
>     LDFLAGS by default at the top of the Makefile, allowing people to add
>     additional options there if they really want to.
> 
>     Reported by Dave Jones, who ended up passing -std=gnu99 by mistake.

Well, I fixed this issue later with making kvmtool compilation more
robust when using modern compiler standards.
That's why I wanted this kludge to go away.

> I think it's better to have EXTRA_CFLAGS and EXTRA_LDFLAGS like the kernel
> has.

Mmmh, I'd rather see guest_init creation use their own flags for it,
since it is so special and actually independent from the target userland.
Let me check this out and send out my guest_init Makefile fix I have
lying around here on the way.

What LDFLAGS are actually causing your issues?

Cheers,
Andre.

> 
>>  FIND   := find
>>  CSCOPE := cscope
>> @@ -162,7 +160,7 @@ ifeq ($(ARCH), arm)
>>         OBJS            += arm/aarch32/kvm-cpu.o
>>         ARCH_INCLUDE    := $(HDRS_ARM_COMMON)
>>         ARCH_INCLUDE    += -Iarm/aarch32/include
>> -       CFLAGS          += -march=armv7-a
>> +       override CFLAGS += -march=armv7-a
>>
>>         ARCH_WANT_LIBFDT := y
>>  endif
>> @@ -274,12 +272,12 @@ endif
>>  ifeq ($(LTO),1)
>>         FLAGS_LTO := -flto
>>         ifeq ($(call try-build,$(SOURCE_HELLO),$(CFLAGS),$(FLAGS_LTO)),y)
>> -               CFLAGS          += $(FLAGS_LTO)
>> +               override CFLAGS += $(FLAGS_LTO)
>>         endif
>>  endif
>>
>>  ifeq ($(call try-build,$(SOURCE_STATIC),,-static),y)
>> -       CFLAGS          += -DCONFIG_GUEST_INIT
>> +       override CFLAGS += -DCONFIG_GUEST_INIT
>>         GUEST_INIT      := guest/init
>>         GUEST_OBJS      = guest/guest_init.o
>>         ifeq ($(ARCH_PRE_INIT),)
>> @@ -331,7 +329,8 @@ DEFINES     += -DKVMTOOLS_VERSION='"$(KVMTOOLS_VERSION)"'
>>  DEFINES        += -DBUILD_ARCH='"$(ARCH)"'
>>
>>  KVM_INCLUDE := include
>> -CFLAGS += $(CPPFLAGS) $(DEFINES) -I$(KVM_INCLUDE) -I$(ARCH_INCLUDE) -O2 -fno-strict-aliasing -g
>> +override CFLAGS        += $(CPPFLAGS) $(DEFINES) -I$(KVM_INCLUDE) -I$(ARCH_INCLUDE)
>> +override CFLAGS        += -O2 -fno-strict-aliasing -g
>>
>>  WARNINGS += -Wall
>>  WARNINGS += -Wformat=2
>> @@ -349,10 +348,10 @@ WARNINGS += -Wvolatile-register-var
>>  WARNINGS += -Wwrite-strings
>>  WARNINGS += -Wno-format-nonliteral
>>
>> -CFLAGS += $(WARNINGS)
>> +override CFLAGS        += $(WARNINGS)
>>
>>  ifneq ($(WERROR),0)
>> -       CFLAGS += -Werror
>> +       override CFLAGS += -Werror
>>  endif
>>
>>  all: $(PROGRAM) $(PROGRAM_ALIAS) $(GUEST_INIT) $(GUEST_PRE_INIT)
>> --
>> 2.5.1
>>
>> --
>> To unsubscribe from this list: send the line "unsubscribe kvm" in
>> the body of a message to majordomo@vger.kernel.org
>> More majordomo info at  http://vger.kernel.org/majordomo-info.html
>