On 30 October 2015 at 19:20, Andre Przywara <andre.przyw...@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.przyw...@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.dea...@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.

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

>  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 majord...@vger.kernel.org
> More majordomo info at  http://vger.kernel.org/majordomo-info.html
--
To unsubscribe from this list: send the line "unsubscribe kvm" in
the body of a message to majord...@vger.kernel.org
More majordomo info at  http://vger.kernel.org/majordomo-info.html

Reply via email to