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

[Riku Voipio]

On 30 October 2015 at 19:20, Andre Przywara  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 
>
> [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 
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

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

[Riku Voipio]

On 4 November 2015 at 12:13, Andre Przywara  wrote:
> Hi Riku,
>
> On 04/11/15 10:02, Riku Voipio wrote:
>> On 30 October 2015 at 19:20, Andre Przywara  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 
>>>
>>> [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 
>> 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?

  LINK guest/init
ld: unrecognized option '-Wl,-z,relro'

That's another issue - kvmtool passes LDFLAGS to both LD and CC yet they
actually take different command line options.

> 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 

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

[Andre Przywara]

Hi Riku,

On 04/11/15 10:02, Riku Voipio wrote:
> On 30 October 2015 at 19:20, Andre Przywara  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 
>>
>> [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 
> 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 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

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

[Will Deacon]

On Wed, Nov 04, 2015 at 12:02:23PM +0200, Riku Voipio wrote:
> On 30 October 2015 at 19:20, Andre Przywara  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 
> >
> > [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.

Thanks for the report, Riku. I'll revert this patch while we rethink how
to support user-supplied toolchain flags.

Will
--
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

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

[Andre Przywara]

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 

[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:=
 
 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