Allow (O=...) from file

Allow (O=...) from file

[Jay Cliburn]

Sam,

This piece of the top-level Makefile in current git causes an
out-of-tree driver Makefile to fail.

101 ifdef O
102   ifeq ("$(origin O)", "command line")
103     KBUILD_OUTPUT := $(O)
104   endif
105 endif

The out-of-tree driver Makefile contains an O=... directive that
(correctly) does _not_ specify the kernel source dir, and apparently
isn't overridden by the command line either. If in the above Makefile
snippet I change "command line" to "file", my out-of-tree make
succeeds. What do you think about allowing O= to come from a file in
addition to the command line?

Here are my attempts:

[jcliburn@osprey atl1-2.0.7-20071202]$ make
make -C /lib/modules/2.6.24-rc3/source O=/lib/modules/2.6.24-rc3/build SUBDIRS=/home/jcliburn/atl1/atl1-2.0.7-20071202 modules
make[1]: Entering directory `/home/jcliburn/kernel-work/netdev/netdev-2.6.git'
Makefile:119: *** Output directory (O=...) specifies kernel src dir.  Stop.
make[1]: Leaving directory `/home/jcliburn/kernel-work/netdev/netdev-2.6.git'
make: *** [default] Error 2

[jcliburn@osprey atl1-2.0.7-20071202]$ make O=/lib/modules/2.6.24-rc3/build
make -C /lib/modules/2.6.24-rc3/source O=/lib/modules/2.6.24-rc3/build SUBDIRS=/home/jcliburn/atl1/atl1-2.0.7-20071202 modules
make[1]: Entering directory `/home/jcliburn/kernel-work/netdev/netdev-2.6.git'
Makefile:119: *** Output directory (O=...) specifies kernel src dir.  Stop.
make[1]: Leaving directory `/home/jcliburn/kernel-work/netdev/netdev-2.6.git'
make: *** [default] Error 2

Thanks for your help.

Jay

Re: Allow (O=...) from file

[Erik Mouw]

[-- Attachment #1: Type: text/plain, Size: 996 bytes --]

On Tue, Dec 04, 2007 at 09:04:33PM -0600, Jay Cliburn wrote:
> Sam,
> 
> This piece of the top-level Makefile in current git causes an
> out-of-tree driver Makefile to fail.
> 
> 101 ifdef O
> 102   ifeq ("$(origin O)", "command line")
> 103     KBUILD_OUTPUT := $(O)
> 104   endif
> 105 endif
> 
> The out-of-tree driver Makefile contains an O=... directive that
> (correctly) does _not_ specify the kernel source dir, and apparently
> isn't overridden by the command line either. If in the above Makefile
> snippet I change "command line" to "file", my out-of-tree make
> succeeds. What do you think about allowing O= to come from a file in
> addition to the command line?

Oh great, changing "command line" to "file" fixes my problem as well
(see http://lkml.org/lkml/2007/11/19/146 ). "make targz-pkg" works
flawless again.


Erik

-- 
They're all fools. Don't worry. Darwin may be slow, but he'll
eventually get them. -- Matthew Lammers in alt.sysadmin.recovery

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Allow (O=...) from file

[Sam Ravnborg]

On Tue, Dec 04, 2007 at 09:04:33PM -0600, Jay Cliburn wrote:
> Sam,
> 
> This piece of the top-level Makefile in current git causes an
> out-of-tree driver Makefile to fail.
> 
> 101 ifdef O
> 102   ifeq ("$(origin O)", "command line")
> 103     KBUILD_OUTPUT := $(O)
> 104   endif
> 105 endif
> 
> The out-of-tree driver Makefile contains an O=... directive that
> (correctly) does _not_ specify the kernel source dir, and apparently
> isn't overridden by the command line either. If in the above Makefile
> snippet I change "command line" to "file", my out-of-tree make
> succeeds. What do you think about allowing O= to come from a file in
> addition to the command line?

When you change "command line" to "file" you actually makes kbuild
ignore the O=... value which is why it succeeds.
The problem we solve with the error below is that in some case
the Makefile for the kernel were overwritten.
And I do not really understand why this does not happen in yours
and Erik's case.

Anyway - the right fix seems to detect that the two directories
are equal and then just ignore the O=... setting.
But I am lacking time atm to fix it - only sparsely working on
Linux the next few weeks.

	Sam

Re: Allow (O=...) from file

[Erik Mouw]

[-- Attachment #1: Type: text/plain, Size: 2717 bytes --]

On Wed, Dec 05, 2007 at 10:00:03PM +0100, Sam Ravnborg wrote:
> On Tue, Dec 04, 2007 at 09:04:33PM -0600, Jay Cliburn wrote:
> > Sam,
> > 
> > This piece of the top-level Makefile in current git causes an
> > out-of-tree driver Makefile to fail.
> > 
> > 101 ifdef O
> > 102   ifeq ("$(origin O)", "command line")
> > 103     KBUILD_OUTPUT := $(O)
> > 104   endif
> > 105 endif
> > 
> > The out-of-tree driver Makefile contains an O=... directive that
> > (correctly) does _not_ specify the kernel source dir, and apparently
> > isn't overridden by the command line either. If in the above Makefile
> > snippet I change "command line" to "file", my out-of-tree make
> > succeeds. What do you think about allowing O= to come from a file in
> > addition to the command line?
> 
> When you change "command line" to "file" you actually makes kbuild
> ignore the O=... value which is why it succeeds.
> The problem we solve with the error below is that in some case
> the Makefile for the kernel were overwritten.
> And I do not really understand why this does not happen in yours
> and Erik's case.

I just RTFM for GNU make, changing "command line" into "default" also
results in a succesful build.

> Anyway - the right fix seems to detect that the two directories
> are equal and then just ignore the O=... setting.
> But I am lacking time atm to fix it - only sparsely working on
> Linux the next few weeks.

Here's a clue: when I build with ARCH=x86, I get some warnings, but the
targz-pkg builds succesfully:

erik@arthur:~/git/linux-2.6 > make ARCH=x86 allnoconfig
[...]
erik@arthur:~/git/linux-2.6 > make ARCH=x86 targz-pkg
[...]
Kernel: arch/x86/boot/bzImage is ready  (#1)
/bin/sh /home/erik/git/linux-2.6/scripts/package/buildtar targz-pkg
`/home/erik/git/linux-2.6/System.map' ->
/`/home/erik/git/linux-2.6/tar-install/boot/System.map-2.6.24-rc4'
`/home/erik/git/linux-2.6/.config' ->
/`/home/erik/git/linux-2.6/tar-install/boot/config-2.6.24-rc4'
`/home/erik/git/linux-2.6/vmlinux' ->
/`/home/erik/git/linux-2.6/tar-install/boot/vmlinux-2.6.24-rc4'
`arch/x86/boot/bzImage' ->
/`/home/erik/git/linux-2.6/tar-install/boot/vmlinux-kbuild-2.6.24-rc4'

** ** **  WARNING  ** ** **

Your architecture did not define any architecture-dependant files
to be placed into the tarball. Please add those to
/home/erik/git/linux-2.6/scripts/package/buildtar ...

Tarball successfully created in
/home/erik/git/linux-2.6/linux-2.6.24-rc4.tar.gz

So it looks like the i386-x86_64 merge has something to do with it.


Erik

-- 
They're all fools. Don't worry. Darwin may be slow, but he'll
eventually get them. -- Matthew Lammers in alt.sysadmin.recovery

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Allow (O=...) from file

[Erik Mouw]

[-- Attachment #1: Type: text/plain, Size: 1477 bytes --]

On Wed, Dec 05, 2007 at 10:37:03PM +0100, Erik Mouw wrote:
> Here's a clue: when I build with ARCH=x86, I get some warnings, but the
> targz-pkg builds succesfully:
> 
> erik@arthur:~/git/linux-2.6 > make ARCH=x86 allnoconfig
> [...]
> erik@arthur:~/git/linux-2.6 > make ARCH=x86 targz-pkg
> [...]
> Kernel: arch/x86/boot/bzImage is ready  (#1)
> /bin/sh /home/erik/git/linux-2.6/scripts/package/buildtar targz-pkg
> `/home/erik/git/linux-2.6/System.map' ->
> /`/home/erik/git/linux-2.6/tar-install/boot/System.map-2.6.24-rc4'
> `/home/erik/git/linux-2.6/.config' ->
> /`/home/erik/git/linux-2.6/tar-install/boot/config-2.6.24-rc4'
> `/home/erik/git/linux-2.6/vmlinux' ->
> /`/home/erik/git/linux-2.6/tar-install/boot/vmlinux-2.6.24-rc4'
> `arch/x86/boot/bzImage' ->
> /`/home/erik/git/linux-2.6/tar-install/boot/vmlinux-kbuild-2.6.24-rc4'
> 
> ** ** **  WARNING  ** ** **
> 
> Your architecture did not define any architecture-dependant files
> to be placed into the tarball. Please add those to
> /home/erik/git/linux-2.6/scripts/package/buildtar ...
> 
> Tarball successfully created in
> /home/erik/git/linux-2.6/linux-2.6.24-rc4.tar.gz
> 
> So it looks like the i386-x86_64 merge has something to do with it.

Sorry, wrong clue. It builds because there are no modules to be made
(allnoconfig).


Erik

-- 
They're all fools. Don't worry. Darwin may be slow, but he'll
eventually get them. -- Matthew Lammers in alt.sysadmin.recovery

[-- Attachment #2: Digital signature --]
[-- Type: application/pgp-signature, Size: 189 bytes --]

Re: Allow (O=...) from file

[Jay Cliburn]

On Wed, 5 Dec 2007 22:00:03 +0100
Sam Ravnborg <sam@ravnborg.org> wrote:

> On Tue, Dec 04, 2007 at 09:04:33PM -0600, Jay Cliburn wrote:
> > Sam,
> > 
> > This piece of the top-level Makefile in current git causes an
> > out-of-tree driver Makefile to fail.
> > 
> > 101 ifdef O
> > 102   ifeq ("$(origin O)", "command line")
> > 103     KBUILD_OUTPUT := $(O)
> > 104   endif
> > 105 endif
> > 
> > The out-of-tree driver Makefile contains an O=... directive that
> > (correctly) does _not_ specify the kernel source dir, and apparently
> > isn't overridden by the command line either. If in the above
> > Makefile snippet I change "command line" to "file", my out-of-tree
> > make succeeds. What do you think about allowing O= to come from a
> > file in addition to the command line?
> 
> When you change "command line" to "file" you actually makes kbuild
> ignore the O=... value which is why it succeeds.

I'm puzzled by your statement.  Isn't the opposite true?  When using
"command line", doesn't the following happen?

1. My makefile sets O=/foo
2. My makefile invokes your makefile with O=/foo
3. Your makefile ignores my O=/foo because it requires O=/foo to
originate from the command line
4. KBUILD_OUTPUT never gets set to /foo and we hit the error

OTOH, if I use "file":
1. My makefile sets O=/foo
2. My makefile invokes your makefile with O=/foo
3. Your makefile accepts my O=/foo because it requires O=/foo to
originate from another makefile
4. KBUILD_OUTPUT gets set to /foo and my make succeeds

This all used to work the last time I tried it, which was sometime
during 2.6.23 development, IIRC.  Isn't the current structure going to
break just about all out-of-tree driver builds?

Jay

Re: Allow (O=...) from file

[Sam Ravnborg]

On Wed, Dec 05, 2007 at 09:31:26PM -0600, Jay Cliburn wrote:
> On Wed, 5 Dec 2007 22:00:03 +0100
> Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Tue, Dec 04, 2007 at 09:04:33PM -0600, Jay Cliburn wrote:
> > > Sam,
> > > 
> > > This piece of the top-level Makefile in current git causes an
> > > out-of-tree driver Makefile to fail.
> > > 
> > > 101 ifdef O
> > > 102   ifeq ("$(origin O)", "command line")
> > > 103     KBUILD_OUTPUT := $(O)
> > > 104   endif
> > > 105 endif
> > > 
> > > The out-of-tree driver Makefile contains an O=... directive that
> > > (correctly) does _not_ specify the kernel source dir, and apparently
> > > isn't overridden by the command line either. If in the above
> > > Makefile snippet I change "command line" to "file", my out-of-tree
> > > make succeeds. What do you think about allowing O= to come from a
> > > file in addition to the command line?
> > 
> > When you change "command line" to "file" you actually makes kbuild
> > ignore the O=... value which is why it succeeds.
> 
> I'm puzzled by your statement.  Isn't the opposite true?  When using
> "command line", doesn't the following happen?
> 
> 1. My makefile sets O=/foo
It does so as an argument to make so it is considered "command line" by make.
> 2. My makefile invokes your makefile with O=/foo
> 3. Your makefile ignores my O=/foo because it requires O=/foo to
> originate from the command line
It comes from the command line in your case.
> 4. KBUILD_OUTPUT never gets set to /foo and we hit the error
We hit the error because kbuild see the srctree and objtree points at the
same directory so outputmakefile will overwrite the kernel Makefile
if it is writeable.

> OTOH, if I use "file":
Then you never get KBUILD_OUTPUT set.
Try adding a:
$(warning origin=$(origin O))
just above the lines with the check that fails.

I will try to fix this during the weeken - but Linux time is sparse these days.

	Sam

Re: Allow (O=...) from file

[Sam Ravnborg]

On Wed, Dec 05, 2007 at 09:31:26PM -0600, Jay Cliburn wrote:
> On Wed, 5 Dec 2007 22:00:03 +0100
> Sam Ravnborg <sam@ravnborg.org> wrote:
> 
> > On Tue, Dec 04, 2007 at 09:04:33PM -0600, Jay Cliburn wrote:
> > > Sam,
> > > 
> > > This piece of the top-level Makefile in current git causes an
> > > out-of-tree driver Makefile to fail.
> > > 
> > > 101 ifdef O
> > > 102   ifeq ("$(origin O)", "command line")
> > > 103     KBUILD_OUTPUT := $(O)
> > > 104   endif
> > > 105 endif
> > > 
> > > The out-of-tree driver Makefile contains an O=... directive that
> > > (correctly) does _not_ specify the kernel source dir, and apparently
> > > isn't overridden by the command line either. If in the above
> > > Makefile snippet I change "command line" to "file", my out-of-tree
> > > make succeeds. What do you think about allowing O= to come from a
> > > file in addition to the command line?
> > 
> > When you change "command line" to "file" you actually makes kbuild
> > ignore the O=... value which is why it succeeds.
> 
> I'm puzzled by your statement.  Isn't the opposite true?  When using
> "command line", doesn't the following happen?
> 
> 1. My makefile sets O=/foo
> 2. My makefile invokes your makefile with O=/foo
> 3. Your makefile ignores my O=/foo because it requires O=/foo to
> originate from the command line
> 4. KBUILD_OUTPUT never gets set to /foo and we hit the error
> 
> OTOH, if I use "file":
> 1. My makefile sets O=/foo
> 2. My makefile invokes your makefile with O=/foo
> 3. Your makefile accepts my O=/foo because it requires O=/foo to
> originate from another makefile
> 4. KBUILD_OUTPUT gets set to /foo and my make succeeds
> 
> This all used to work the last time I tried it, which was sometime
> during 2.6.23 development, IIRC.  Isn't the current structure going to
> break just about all out-of-tree driver builds?

Jay - can I ask you to try out following patch.
If is a workaround for the root issue but this is the sort of
fix I would like to push this late in -rc.

	Sam

diff --git a/Makefile b/Makefile
index a5252f4..7fb1a2c 100644
--- a/Makefile
+++ b/Makefile
@@ -118,9 +118,6 @@ saved-output := $(KBUILD_OUTPUT)
 KBUILD_OUTPUT := $(shell cd $(KBUILD_OUTPUT) && /bin/pwd)
 $(if $(KBUILD_OUTPUT),, \
      $(error output directory "$(saved-output)" does not exist))
-# Check that OUTPUT directory is not the same as where we have kernel src
-$(if $(filter-out $(KBUILD_OUTPUT),$(shell /bin/pwd)),, \
-     $(error Output directory (O=...) specifies kernel src dir))
 
 PHONY += $(MAKECMDGOALS) sub-make
 
diff --git a/scripts/mkmakefile b/scripts/mkmakefile
index ee39fac..9ad1bd7 100644
--- a/scripts/mkmakefile
+++ b/scripts/mkmakefile
@@ -11,6 +11,12 @@
 
 
 test ! -r $2/Makefile -o -O $2/Makefile || exit 0
+# Only overwrite automatically generated Makefiles
+# (so we do not overwrite kernel Makefile)
+if ! grep -q Automatically $2/Makefile
+then
+	exit 0
+fi
 echo "  GEN     $2/Makefile"
 
 cat << EOF > $2/Makefile

Re: Allow (O=...) from file

[Jay Cliburn]

On Sat, 8 Dec 2007 21:14:09 +0100
Sam Ravnborg <sam@ravnborg.org> wrote:

> Jay - can I ask you to try out following patch.

Hello Sam,

Yes, your patch works for me.

Thank you very much.

> diff --git a/Makefile b/Makefile
> index a5252f4..7fb1a2c 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -118,9 +118,6 @@ saved-output := $(KBUILD_OUTPUT)
>  KBUILD_OUTPUT := $(shell cd $(KBUILD_OUTPUT) && /bin/pwd)
>  $(if $(KBUILD_OUTPUT),, \
>       $(error output directory "$(saved-output)" does not exist))
> -# Check that OUTPUT directory is not the same as where we have
> kernel src -$(if $(filter-out $(KBUILD_OUTPUT),$(shell /bin/pwd)),, \
> -     $(error Output directory (O=...) specifies kernel src dir))
>  
>  PHONY += $(MAKECMDGOALS) sub-make
>  
> diff --git a/scripts/mkmakefile b/scripts/mkmakefile
> index ee39fac..9ad1bd7 100644
> --- a/scripts/mkmakefile
> +++ b/scripts/mkmakefile
> @@ -11,6 +11,12 @@
>  
>  
>  test ! -r $2/Makefile -o -O $2/Makefile || exit 0
> +# Only overwrite automatically generated Makefiles
> +# (so we do not overwrite kernel Makefile)
> +if ! grep -q Automatically $2/Makefile
> +then
> +	exit 0
> +fi
>  echo "  GEN     $2/Makefile"
>  
>  cat << EOF > $2/Makefile
> --

Re: Allow (O=...) from file

[Jan Engelhardt]

On Dec 4 2007 21:04, Jay Cliburn wrote:
>
>This piece of the top-level Makefile in current git causes an
>out-of-tree driver Makefile to fail.
>
>101 ifdef O
>102   ifeq ("$(origin O)", "command line")
>103     KBUILD_OUTPUT := $(O)
>104   endif
>105 endif

Should not it just use the usual boilerplate?

kdir := /lib/modules/$(shell uname -r)/build
all:
	make -C ${kdir} M=$$PWD

Re: Allow (O=...) from file

[Jay Cliburn]

On Thu, 6 Dec 2007 15:57:38 +0100 (CET)
Jan Engelhardt <jengelh@computergmbh.de> wrote:

> 
> On Dec 4 2007 21:04, Jay Cliburn wrote:
> >
> >This piece of the top-level Makefile in current git causes an
> >out-of-tree driver Makefile to fail.
> >
> >101 ifdef O
> >102   ifeq ("$(origin O)", "command line")
> >103     KBUILD_OUTPUT := $(O)
> >104   endif
> >105 endif
> 
> Should not it just use the usual boilerplate?
> 
> kdir := /lib/modules/$(shell uname -r)/build
> all:
> 	make -C ${kdir} M=$$PWD

Yep, that certainly works, but I was using a vendor-provided Makefile
that employs O=...