[PATCH] samples: build fix

[PATCH] samples: build fix

[Roland McGrath]

The samples/ subdirectory contains only modules.
But the only make run done there is in commands for vmlinux.
I can't see why this was ever done in this nonstandard fashion.
As things stand, the modules don't get built by 'make modules'.

I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
because there is no other conditional like that in the top-level Makefile
and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
always to be included.

Signed-off-by: Roland McGrath <roland@redhat.com>
---
 Makefile |    5 +----
 1 files changed, 1 insertions(+), 4 deletions(-)

diff --git a/Makefile b/Makefile
index c162370..9e9ce33 100644
--- a/Makefile
+++ b/Makefile
@@ -602,7 +602,7 @@ export mod_strip_cmd
 
 
 ifeq ($(KBUILD_EXTMOD),)
-core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/
+core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/ samples/
 
 vmlinux-dirs	:= $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \
 		     $(core-y) $(core-m) $(drivers-y) $(drivers-m) \
@@ -802,9 +802,6 @@ vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o $(kallsyms.o)
 ifdef CONFIG_HEADERS_CHECK
 	$(Q)$(MAKE) -f $(srctree)/Makefile headers_check
 endif
-ifdef CONFIG_SAMPLES
-	$(Q)$(MAKE) $(build)=samples
-endif
 	$(call vmlinux-modpost)
 	$(call if_changed_rule,vmlinux__)
 	$(Q)rm -f .old_version

Re: [PATCH] samples: build fix

[Mathieu Desnoyers]

* Roland McGrath (roland@redhat.com) wrote:
> 
> The samples/ subdirectory contains only modules.
> But the only make run done there is in commands for vmlinux.
> I can't see why this was ever done in this nonstandard fashion.
> As things stand, the modules don't get built by 'make modules'.
> 
> I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
> because there is no other conditional like that in the top-level Makefile
> and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
> always to be included.
> 

Sam, is this ok with the samples intent ? I think as long as we do not
include them with the kernel image and have a "make samples" to build
them, it's ok. Having them built upon make modules seems like a good
idea to me.

Mathieu

> Signed-off-by: Roland McGrath <roland@redhat.com>
> ---
>  Makefile |    5 +----
>  1 files changed, 1 insertions(+), 4 deletions(-)
> 
> diff --git a/Makefile b/Makefile
> index c162370..9e9ce33 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -602,7 +602,7 @@ export mod_strip_cmd
>  
>  
>  ifeq ($(KBUILD_EXTMOD),)
> -core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/
> +core-y		+= kernel/ mm/ fs/ ipc/ security/ crypto/ block/ samples/
>  
>  vmlinux-dirs	:= $(patsubst %/,%,$(filter %/, $(init-y) $(init-m) \
>  		     $(core-y) $(core-m) $(drivers-y) $(drivers-m) \
> @@ -802,9 +802,6 @@ vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o $(kallsyms.o)
>  ifdef CONFIG_HEADERS_CHECK
>  	$(Q)$(MAKE) -f $(srctree)/Makefile headers_check
>  endif
> -ifdef CONFIG_SAMPLES
> -	$(Q)$(MAKE) $(build)=samples
> -endif
>  	$(call vmlinux-modpost)
>  	$(call if_changed_rule,vmlinux__)
>  	$(Q)rm -f .old_version

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH] samples: build fix

[Roland McGrath]

> Sam, is this ok with the samples intent ? I think as long as we do not
> include them with the kernel image and have a "make samples" to build
> them, it's ok. Having them built upon make modules seems like a good
> idea to me.

I"m not sure what you mean by "with the kernel image".  
AFAICT, everything under samples/ can only ever be built as modules.
In no event would it affect the vmlinux link.


Thanks,
Roland

Re: [PATCH] samples: build fix

[Sam Ravnborg]

On Thu, Feb 14, 2008 at 08:27:52AM -0500, Mathieu Desnoyers wrote:
> * Roland McGrath (roland@redhat.com) wrote:
> > 
> > The samples/ subdirectory contains only modules.
> > But the only make run done there is in commands for vmlinux.
> > I can't see why this was ever done in this nonstandard fashion.
> > As things stand, the modules don't get built by 'make modules'.
> > 
> > I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
> > because there is no other conditional like that in the top-level Makefile
> > and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
> > always to be included.
> > 
> 
> Sam, is this ok with the samples intent ? I think as long as we do not
> include them with the kernel image and have a "make samples" to build
> them, it's ok. Having them built upon make modules seems like a good
> idea to me.

The samples code are supposed to be what the name says 'samples'.
This is not code that are supposed to be part of the built-in kernel.
This is not modules that are supposed to be installes when 
installing modules.

Adding it to core-y as Roland does in the patch below is plain
wrong as it enabled both points above.
The fact that the present code in samples/ does not do this is
in this respect irellevant.

Do we have problems when to build the sampls - then lets
address this issue but not by trying to upgrade the samples
to first class citizen in the kernel - they are not that
and should not be handled like that.

	Sam

Re: [PATCH] samples: build fix

[Roland McGrath]

I don't have any axe to grind here, I just could not figure out how to get
the samples/ modules built (without resorting to make M=.../samples/foo and
clobbering my source directory).  If there is another plan, point me to its
documentation.  I can't see the point of the Kconfig option for samples at all.
What does it do?

Because they are only ever modules, the samples are never built in.  I
honestly don't see why the modules shouldn't be installed like any others.
People who are really going to install the kernel for production are not
going to set this Kconfig option.


Thanks,
Roland

Re: [PATCH] samples: build fix

[Mathieu Desnoyers]

* Sam Ravnborg (sam@ravnborg.org) wrote:
> On Thu, Feb 14, 2008 at 08:27:52AM -0500, Mathieu Desnoyers wrote:
> > * Roland McGrath (roland@redhat.com) wrote:
> > > 
> > > The samples/ subdirectory contains only modules.
> > > But the only make run done there is in commands for vmlinux.
> > > I can't see why this was ever done in this nonstandard fashion.
> > > As things stand, the modules don't get built by 'make modules'.
> > > 
> > > I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
> > > because there is no other conditional like that in the top-level Makefile
> > > and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
> > > always to be included.
> > > 
> > 
> > Sam, is this ok with the samples intent ? I think as long as we do not
> > include them with the kernel image and have a "make samples" to build
> > them, it's ok. Having them built upon make modules seems like a good
> > idea to me.
> 
> The samples code are supposed to be what the name says 'samples'.
> This is not code that are supposed to be part of the built-in kernel.
> This is not modules that are supposed to be installes when 
> installing modules.
> 
> Adding it to core-y as Roland does in the patch below is plain
> wrong as it enabled both points above.
> The fact that the present code in samples/ does not do this is
> in this respect irellevant.
> 
> Do we have problems when to build the sampls - then lets
> address this issue but not by trying to upgrade the samples
> to first class citizen in the kernel - they are not that
> and should not be handled like that.
> 

Then is there some other way to have the samples built upon "make
modules" that would not install them with other modules ?

Mathieu

> 	Sam

-- 
Mathieu Desnoyers
Computer Engineering Ph.D. Student, Ecole Polytechnique de Montreal
OpenPGP key fingerprint: 8CD5 52C3 8E3C 4140 715F  BA06 3F25 A8FE 3BAE 9A68

Re: [PATCH] samples: build fix

[Randy Dunlap]

On Thu, 14 Feb 2008 12:02:57 -0800 (PST) Roland McGrath wrote:

> I don't have any axe to grind here, I just could not figure out how to get
> the samples/ modules built (without resorting to make M=.../samples/foo and
> clobbering my source directory).  If there is another plan, point me to its
> documentation.  I can't see the point of the Kconfig option for samples at all.
> What does it do?
> 
> Because they are only ever modules, the samples are never built in.  I
> honestly don't see why the modules shouldn't be installed like any others.
> People who are really going to install the kernel for production are not
> going to set this Kconfig option.

There used to be a make target called "samples_check" (I could have
misspelled it).  Where did it go?  Sam?

---
~Randy

Re: [PATCH] samples: build fix

[Sam Ravnborg]

On Thu, Feb 14, 2008 at 12:02:57PM -0800, Roland McGrath wrote:
> I don't have any axe to grind here, I just could not figure out how to get
> the samples/ modules built (without resorting to make M=.../samples/foo and
> clobbering my source directory).

> If there is another plan, point me to its documentation.  

I just double checked and the usual:
make O=output samples/

It works as expected and build all the samples.
It is documented by make help like this:
$ make help
  ...
  dir/            - Build all files in dir and below


The above is generic functionality and I do not see any
need for a special target to build the samples.

> I can't see the point of the Kconfig option for samples at all.
> What does it do?
The samples were added so we avoided them to bit-rot.
We do not want to build samples unconditionally - that should be pretty
obvious.
So maybe I did not get what you actually was asking?

> Because they are only ever modules, the samples are never built in.
There is _nothing_ preventing us from adding stuff tomorrow that has
be build as built-in. Some sample code documenting early cpu init
stuff for instance. And we do not want this code to clash with
other kernel code if global symbols are duplicated.

This thinking that samples are solely modules are wrong. It is correct that
most of it is expected to be modules but not all of it.

I only found one little glitch.
$ make modules
does not build the samples.

	Sam

Re: [PATCH] samples: build fix

[Sam Ravnborg]

On Thu, Feb 14, 2008 at 03:04:04PM -0500, Mathieu Desnoyers wrote:
> * Sam Ravnborg (sam@ravnborg.org) wrote:
> > On Thu, Feb 14, 2008 at 08:27:52AM -0500, Mathieu Desnoyers wrote:
> > > * Roland McGrath (roland@redhat.com) wrote:
> > > > 
> > > > The samples/ subdirectory contains only modules.
> > > > But the only make run done there is in commands for vmlinux.
> > > > I can't see why this was ever done in this nonstandard fashion.
> > > > As things stand, the modules don't get built by 'make modules'.
> > > > 
> > > > I didn't make the addition of the directory use core-$(CONFIG_SAMPLES)
> > > > because there is no other conditional like that in the top-level Makefile
> > > > and samples/Makefile already uses obj-$(CONFIG_SAMPLES) as if it expects
> > > > always to be included.
> > > > 
> > > 
> > > Sam, is this ok with the samples intent ? I think as long as we do not
> > > include them with the kernel image and have a "make samples" to build
> > > them, it's ok. Having them built upon make modules seems like a good
> > > idea to me.
> > 
> > The samples code are supposed to be what the name says 'samples'.
> > This is not code that are supposed to be part of the built-in kernel.
> > This is not modules that are supposed to be installes when 
> > installing modules.
> > 
> > Adding it to core-y as Roland does in the patch below is plain
> > wrong as it enabled both points above.
> > The fact that the present code in samples/ does not do this is
> > in this respect irellevant.
> > 
> > Do we have problems when to build the sampls - then lets
> > address this issue but not by trying to upgrade the samples
> > to first class citizen in the kernel - they are not that
> > and should not be handled like that.
> > 
> 
> Then is there some other way to have the samples built upon "make
> modules" that would not install them with other modules ?
The simple solution is like attached.
I do not care if they are build as part of vmlinux or modules build.
So if it is generally preferred we could move it like this.

Comments?

	Sam

diff --git a/Makefile b/Makefile
index c162370..c72afa9 100644
--- a/Makefile
+++ b/Makefile
@@ -802,9 +802,6 @@ vmlinux: $(vmlinux-lds) $(vmlinux-init) $(vmlinux-main) vmlinux.o $(kallsyms.o)
 ifdef CONFIG_HEADERS_CHECK
 	$(Q)$(MAKE) -f $(srctree)/Makefile headers_check
 endif
-ifdef CONFIG_SAMPLES
-	$(Q)$(MAKE) $(build)=samples
-endif
 	$(call vmlinux-modpost)
 	$(call if_changed_rule,vmlinux__)
 	$(Q)rm -f .old_version
@@ -1036,6 +1033,9 @@ all: modules
 PHONY += modules
 modules: $(vmlinux-dirs) $(if $(KBUILD_BUILTIN),vmlinux)
 	$(Q)$(AWK) '!x[$$0]++' $(vmlinux-dirs:%=$(objtree)/%/modules.order) > $(objtree)/modules.order
+ifdef CONFIG_SAMPLES
+	$(Q)$(MAKE) $(build)=samples
+endif
 	@echo '  Building modules, stage 2.';
 	$(Q)$(MAKE) -f $(srctree)/scripts/Makefile.modpost