kbuild - introduce support for subdir-ccflags-y

kbuild - introduce support for subdir-ccflags-y

[Sam Ravnborg]

Following patch introduce support for setting options
to gcc that has effect for current directory and all
subdirectories.


The typical use cases are an architecture or a subsystem that
decide to cover all files with -Werror.
Today alpha, mips and sparc uses -Werror in almost all their
Makefile - with subdir-ccflag-y it is now simpler to do so
as only the top-level directories needs to be covered.

Likewise if we decide to cover a full subsystem such
as net/ with -Werror this is done by adding a single
line to net/Makefile.

The feature is added because the x86 guys are planning
to (conditionally) cover all of arch/x86/ with -Werrror
and adding this statement in each single Makefile
was *the wrong way* to do so.

	Sam

diff --git a/Documentation/kbuild/makefiles.txt b/Documentation/kbuild/makefiles.txt
index d4b0567..d76cfd8 100644
--- a/Documentation/kbuild/makefiles.txt
+++ b/Documentation/kbuild/makefiles.txt
@@ -316,6 +316,16 @@ more details, with real examples.
 		#arch/m68k/fpsp040/Makefile
 		ldflags-y := -x
 
+    subdir-ccflags-y, subdir-asflags-y
+	The two flags listed above are similar to ccflags-y and as-falgs-y.
+	The difference is that the subdir- variants has effect for the kbuild
+	file where tey are present and all subdirectories.
+	Options specified using subdir-* are added to the commandline before
+	the options specified using the non-subdir variants.
+
+	Example:
+		subdir-ccflags-y := -Werror
+
     CFLAGS_$@, AFLAGS_$@
 
 	CFLAGS_$@ and AFLAGS_$@ only apply to commands in current
diff --git a/scripts/Makefile.build b/scripts/Makefile.build
index 39a9642..5c4b7a4 100644
--- a/scripts/Makefile.build
+++ b/scripts/Makefile.build
@@ -27,6 +27,9 @@ ccflags-y  :=
 cppflags-y :=
 ldflags-y  :=
 
+subdir-asflags-y :=
+subdir-ccflags-y :=
+
 # Read auto.conf if it exists, otherwise ignore
 -include include/config/auto.conf
 
diff --git a/scripts/Makefile.lib b/scripts/Makefile.lib
index 9796195..cba61ca 100644
--- a/scripts/Makefile.lib
+++ b/scripts/Makefile.lib
@@ -4,6 +4,11 @@ ccflags-y  += $(EXTRA_CFLAGS)
 cppflags-y += $(EXTRA_CPPFLAGS)
 ldflags-y  += $(EXTRA_LDFLAGS)
 
+#
+# flags that take effect in sub directories
+export KBUILD_SUBDIR_ASFLAGS := $(KBUILD_SUBDIR_ASFLAGS) $(subdir-asflags-y)
+export KBUILD_SUBDIR_CCFLAGS := $(KBUILD_SUBDIR_CCFLAGS) $(subdir-ccflags-y)
+
 # Figure out what we need to build from the various variables
 # ===========================================================================
 
@@ -104,10 +109,10 @@ else
 debug_flags =
 endif
 
-orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS)                     \
+orig_c_flags   = $(KBUILD_CPPFLAGS) $(KBUILD_CFLAGS) $(KBUILD_SUBDIR_CCFLAGS) \
                  $(ccflags-y) $(CFLAGS_$(basetarget).o)
 _c_flags       = $(filter-out $(CFLAGS_REMOVE_$(basetarget).o), $(orig_c_flags))
-_a_flags       = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS)                     \
+_a_flags       = $(KBUILD_CPPFLAGS) $(KBUILD_AFLAGS) $(KBUILD_SUBDIR_ASFLAGS) \
                  $(asflags-y) $(AFLAGS_$(basetarget).o)
 _cpp_flags     = $(KBUILD_CPPFLAGS) $(cppflags-y) $(CPPFLAGS_$(@F))
 

Re: kbuild - introduce support for subdir-ccflags-y

[Russell King]

On Sat, Apr 18, 2009 at 02:51:59PM +0200, Sam Ravnborg wrote:
> The typical use cases are an architecture or a subsystem that
> decide to cover all files with -Werror.
> Today alpha, mips and sparc uses -Werror in almost all their
> Makefile - with subdir-ccflag-y it is now simpler to do so
> as only the top-level directories needs to be covered.

Hmm, this won't make sense for ARM.  We have things like #warning and
deprecated functions in machine specific headers, and adding -Werror
to the whole of arch/arm/ will result in these causing builds to fail.

Adding it just for boot,common,kernel,lib,mm,nwfpe,oprofile,vfp (iow
everything except the mach-* and plat-* directories) would make sense
though.

> The feature is added because the x86 guys are planning
> to (conditionally) cover all of arch/x86/ with -Werrror
> and adding this statement in each single Makefile
> was *the wrong way* to do so.

I think that's the way we're going to _have_ to do it on ARM.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: kbuild - introduce support for subdir-ccflags-y

[Ingo Molnar]

* Russell King <rmk+lkml@arm.linux.org.uk> wrote:

> On Sat, Apr 18, 2009 at 02:51:59PM +0200, Sam Ravnborg wrote:
> > The typical use cases are an architecture or a subsystem that
> > decide to cover all files with -Werror.
> > Today alpha, mips and sparc uses -Werror in almost all their
> > Makefile - with subdir-ccflag-y it is now simpler to do so
> > as only the top-level directories needs to be covered.
> 
> Hmm, this won't make sense for ARM.  We have things like #warning 
> and deprecated functions in machine specific headers, and adding 
> -Werror to the whole of arch/arm/ will result in these causing 
> builds to fail.

This is optional - if you dont want it, you dont set it.

	Ingo

Re: kbuild - introduce support for subdir-ccflags-y

[Russell King]

On Sat, Apr 18, 2009 at 05:07:50PM +0200, Ingo Molnar wrote:
> 
> * Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> 
> > On Sat, Apr 18, 2009 at 02:51:59PM +0200, Sam Ravnborg wrote:
> > > The typical use cases are an architecture or a subsystem that
> > > decide to cover all files with -Werror.
> > > Today alpha, mips and sparc uses -Werror in almost all their
> > > Makefile - with subdir-ccflag-y it is now simpler to do so
> > > as only the top-level directories needs to be covered.
> > 
> > Hmm, this won't make sense for ARM.  We have things like #warning 
> > and deprecated functions in machine specific headers, and adding 
> > -Werror to the whole of arch/arm/ will result in these causing 
> > builds to fail.
> 
> This is optional - if you dont want it, you dont set it.

Please read _all_ of my mail, particularly the bit where it talks about
it being useful for a certain subset.

-- 
Russell King
 Linux kernel    2.6 ARM Linux   - http://www.arm.linux.org.uk/
 maintainer of:

Re: kbuild - introduce support for subdir-ccflags-y

[Kyle Moffett]

On Sat, Apr 18, 2009 at 2:09 PM, Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> On Sat, Apr 18, 2009 at 05:07:50PM +0200, Ingo Molnar wrote:
>> * Russell King <rmk+lkml@arm.linux.org.uk> wrote:
>> > On Sat, Apr 18, 2009 at 02:51:59PM +0200, Sam Ravnborg wrote:
>> > > The typical use cases are an architecture or a subsystem that
>> > > decide to cover all files with -Werror.
>> > > Today alpha, mips and sparc uses -Werror in almost all their
>> > > Makefile - with subdir-ccflag-y it is now simpler to do so
>> > > as only the top-level directories needs to be covered.
>> >
>> > Hmm, this won't make sense for ARM.  We have things like #warning
>> > and deprecated functions in machine specific headers, and adding
>> > -Werror to the whole of arch/arm/ will result in these causing
>> > builds to fail.
>>
>> This is optional - if you dont want it, you dont set it.
>
> Please read _all_ of my mail, particularly the bit where it talks about
> it being useful for a certain subset.

It's my impression that on x86 it's a config option whether or not to
build with -Werror.  You could do the exact same thing with an
internal inverted-logic CONFIG_ARM_ALLOW_WARNINGS option and make all
the boards triggering warnings "select ARM_ALLOW_WARNINGS".  Then have
a user-visible config option "ARM_USER_ALLOW_WARNINGS" which also
selects the internal option.  That adds some additional kconfig-level
documentation on which subarch combos need some love.

Cheers,
Kyle Moffett

Re: kbuild - introduce support for subdir-ccflags-y

[Sam Ravnborg]

On Sat, Apr 18, 2009 at 11:17:44PM -0400, Kyle Moffett wrote:
> On Sat, Apr 18, 2009 at 2:09 PM, Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> > On Sat, Apr 18, 2009 at 05:07:50PM +0200, Ingo Molnar wrote:
> >> * Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> >> > On Sat, Apr 18, 2009 at 02:51:59PM +0200, Sam Ravnborg wrote:
> >> > > The typical use cases are an architecture or a subsystem that
> >> > > decide to cover all files with -Werror.
> >> > > Today alpha, mips and sparc uses -Werror in almost all their
> >> > > Makefile - with subdir-ccflag-y it is now simpler to do so
> >> > > as only the top-level directories needs to be covered.
> >> >
> >> > Hmm, this won't make sense for ARM.  We have things like #warning
> >> > and deprecated functions in machine specific headers, and adding
> >> > -Werror to the whole of arch/arm/ will result in these causing
> >> > builds to fail.
> >>
> >> This is optional - if you dont want it, you dont set it.
> >
> > Please read _all_ of my mail, particularly the bit where it talks about
> > it being useful for a certain subset.
> 
> It's my impression that on x86 it's a config option whether or not to
> build with -Werror.  You could do the exact same thing with an
> internal inverted-logic CONFIG_ARM_ALLOW_WARNINGS option and make all
> the boards triggering warnings "select ARM_ALLOW_WARNINGS".  Then have
> a user-visible config option "ARM_USER_ALLOW_WARNINGS" which also
> selects the internal option.  That adds some additional kconfig-level
> documentation on which subarch combos need some love.

Correct - but Russell's original comment was that the subdir-* feature
was of no use for arm. And this is correct with the current way
the individual directories are specified.

If you compare arch/x86/kernel/ and arch/arm/kernel/ you will see
that x86 has 3 subdirectories where arm has none.

Another measure:
Arm has 12 Makefiles outside mach* and plat* where x86 has 24 Makefiles.

Ao unless Russell decide to take the patch that refactor all the core-*
stuff out in a Kbuild file arm has no use of subdir-ccflags-y and
can use the already existing ccflags-y if they decide to cover
most of arm with -Werror.

	Sam

Re: kbuild - introduce support for subdir-ccflags-y

[Ingo Molnar]

* Sam Ravnborg <sam@ravnborg.org> wrote:

> On Sat, Apr 18, 2009 at 11:17:44PM -0400, Kyle Moffett wrote:
> > On Sat, Apr 18, 2009 at 2:09 PM, Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> > > On Sat, Apr 18, 2009 at 05:07:50PM +0200, Ingo Molnar wrote:
> > >> * Russell King <rmk+lkml@arm.linux.org.uk> wrote:
> > >> > On Sat, Apr 18, 2009 at 02:51:59PM +0200, Sam Ravnborg wrote:
> > >> > > The typical use cases are an architecture or a subsystem that
> > >> > > decide to cover all files with -Werror.
> > >> > > Today alpha, mips and sparc uses -Werror in almost all their
> > >> > > Makefile - with subdir-ccflag-y it is now simpler to do so
> > >> > > as only the top-level directories needs to be covered.
> > >> >
> > >> > Hmm, this won't make sense for ARM.  We have things like #warning
> > >> > and deprecated functions in machine specific headers, and adding
> > >> > -Werror to the whole of arch/arm/ will result in these causing
> > >> > builds to fail.
> > >>
> > >> This is optional - if you dont want it, you dont set it.
> > >
> > > Please read _all_ of my mail, particularly the bit where it talks about
> > > it being useful for a certain subset.
> > 
> > It's my impression that on x86 it's a config option whether or not to
> > build with -Werror.  You could do the exact same thing with an
> > internal inverted-logic CONFIG_ARM_ALLOW_WARNINGS option and make all
> > the boards triggering warnings "select ARM_ALLOW_WARNINGS".  Then have
> > a user-visible config option "ARM_USER_ALLOW_WARNINGS" which also
> > selects the internal option.  That adds some additional kconfig-level
> > documentation on which subarch combos need some love.
> 
> Correct - but Russell's original comment was that the subdir-* 
> feature was of no use for arm. And this is correct with the 
> current way the individual directories are specified.
> 
> If you compare arch/x86/kernel/ and arch/arm/kernel/ you will see 
> that x86 has 3 subdirectories where arm has none.
> 
> Another measure: Arm has 12 Makefiles outside mach* and plat* 
> where x86 has 24 Makefiles.
> 
> Ao unless Russell decide to take the patch that refactor all the 
> core-* stuff out in a Kbuild file arm has no use of 
> subdir-ccflags-y and can use the already existing ccflags-y if 
> they decide to cover most of arm with -Werror.

Yes. This is an optional facility entirely - if some arch does
not need or does not want it, it does not have to care.

Sparc is the example to follow here: it has had unconditional 
-Werror builds (for its architecture files) for years. It has
done so without any explicit kbuild help beyond a single-line
ccflags-y := -Werror rule. Sparc has 7 makefiles and it has a 
well-tested compiler environment.

We cannot do the same on x86 straight away: it's being built
on a very wide variety of compiler versions, by a very wide
range of people: more than 95% of our Linux kernel testers are
on x86.

Some of those compiler versions emit bogus warnings that cannot
be worked around in the kernel in any sane way - and a forced
-Werror build would thus become a (needless) show-stopper there.
It will also take some time to flush out any remaining warnings.

All in one, we want arch/x86 to become a -Werror-clean architecture 
and we want all the advantages of that, but we didnt want to do any 
artificial 'break the world' flag day.

So we are doing a hybrid approach instead, we are adding a 
default-off .config option, and we are doing it all smoothly and 
gradually.

The reason we asked Sam whether he can think of any kbuild help 
(again, which is strictly optional) was that we didnt want to 
propagate a 5-6 lines hybrid Makefile rule into 24 makefiles.

We could have done this all via a special makefile rule entirely
in arch/x86/ that is included in every secondary makefile - but
we also wanted to give other architectures an optional, librarized
kbuild facility to do the same, if they so choose.

Thanks,

	Ingo

Re: kbuild - introduce support for subdir-ccflags-y

[Sam Ravnborg]

On Sat, Apr 18, 2009 at 01:58:39PM +0100, Russell King wrote:
> On Sat, Apr 18, 2009 at 02:51:59PM +0200, Sam Ravnborg wrote:
> > The typical use cases are an architecture or a subsystem that
> > decide to cover all files with -Werror.
> > Today alpha, mips and sparc uses -Werror in almost all their
> > Makefile - with subdir-ccflag-y it is now simpler to do so
> > as only the top-level directories needs to be covered.
> 
> Hmm, this won't make sense for ARM.  We have things like #warning and
> deprecated functions in machine specific headers, and adding -Werror
> to the whole of arch/arm/ will result in these causing builds to fail.

For arm I would suggest the following preparational patch:

diff --git a/arch/arm/Kbuild b/arch/arm/Kbuild
new file mode 100644
index 0000000..33b6645
--- /dev/null
+++ b/arch/arm/Kbuild
@@ -0,0 +1,7 @@
+# core part of arm kernel
+# The file is referenced from arch/arm/Makefile
+
+core-y                          += kernel/ mm/ common/
+core-$(CONFIG_FPE_NWFPE)        += nwfpe/
+core-$(CONFIG_VFP)              += arch/arm/vfp/
+
diff --git a/arch/arm/Makefile b/arch/arm/Makefile
index e84729b..5ccdca1 100644
--- a/arch/arm/Makefile
+++ b/arch/arm/Makefile
@@ -186,18 +186,9 @@ endif
 
 export	TEXT_OFFSET GZFLAGS MMUEXT
 
-# Do we have FASTFPE?
-FASTFPE		:=arch/arm/fastfpe
-ifeq ($(FASTFPE),$(wildcard $(FASTFPE)))
-FASTFPE_OBJ	:=$(FASTFPE)/
-endif
-
 # If we have a machine-specific directory, then include it in the build.
-core-y				+= arch/arm/kernel/ arch/arm/mm/ arch/arm/common/
+core-y                          += arch/arm/
 core-y				+= $(machdirs) $(platdirs)
-core-$(CONFIG_FPE_NWFPE)	+= arch/arm/nwfpe/
-core-$(CONFIG_FPE_FASTFPE)	+= $(FASTFPE_OBJ)
-core-$(CONFIG_VFP)		+= arch/arm/vfp/
 
 drivers-$(CONFIG_OPROFILE)      += arch/arm/oprofile/
 

[The above patch include deletion of unused FASTFPE stuff]

Then to cover all of kernel/ mm/ common/ nwfpe/ and vfp/
is a single line in arch/arm/Kbuild.

Another benefit of arch/arm/Kbuild is that you can use:

    make arch/arm/

and get all the above listed directories built.

You would need dedicated subdir-* for oprofile/ as 
it is linked together with drivers.
And for machine and platforms it would be a case-by-case
decision.

So it can be of benefit for arm too - but only if a preparational
step is done (which brings in other benefits).

If you want the preparational patch - independent of subdir-* support
then let me know and I will submit a proper patch to you.

	Sam