Kbuild change breaks the ppc64 build

Kbuild change breaks the ppc64 build

[Paul Mackerras]

Commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7 breaks the build on
64-bit powerpc because we no longer get the -m64 flag passed to gcc.
There is code in arch/powerpc/Makefile which adds (or used to add)
-m64 to AS, LD and CC if we are running on a 64-bit machine (which I
am) and have a biarch toolchain (which I do).  Without -m64, the
toolchain assumes 32-bit and all sorts of things break spectacularly.

I haven't yet tracked down exactly why this commit has this effect,
since I find it takes considerable time and effort to understand
Kbuild.

Paul.

Kbuild change breaks the ppc64 build

[Paul Mackerras]

Commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7 breaks the build on
64-bit powerpc because we no longer get the -m64 flag passed to gcc.
There is code in arch/powerpc/Makefile which adds (or used to add)
-m64 to AS, LD and CC if we are running on a 64-bit machine (which I
am) and have a biarch toolchain (which I do).  Without -m64, the
toolchain assumes 32-bit and all sorts of things break spectacularly.

I haven't yet tracked down exactly why this commit has this effect,
since I find it takes considerable time and effort to understand
Kbuild.

Paul.

Re: Kbuild change breaks the ppc64 build

[David Miller]

From: Paul Mackerras <paulus@samba.org>
Date: Thu, 8 Feb 2007 22:10:54 +1100

> Commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7 breaks the build on
> 64-bit powerpc because we no longer get the -m64 flag passed to gcc.
> There is code in arch/powerpc/Makefile which adds (or used to add)
> -m64 to AS, LD and CC if we are running on a 64-bit machine (which I
> am) and have a biarch toolchain (which I do).  Without -m64, the
> toolchain assumes 32-bit and all sorts of things break spectacularly.
> 
> I haven't yet tracked down exactly why this commit has this effect,
> since I find it takes considerable time and effort to understand
> Kbuild.

It's this commit:

commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7
Author: Oleg Verych <olecom@flower.upol.cz>
Date:   Tue Feb 6 02:18:21 2007 +0100

    [PATCH] kbuild: improve option checking, Kbuild.include cleanup
    
     GNU binutils, root users, tmpfiles, external modules ro builds must
     be fixed to do the right thing now.
    
    Cc: Roman Zippel <zippel@linux-m68k.org>
    Cc: Sam Ravnborg <sam@ravnborg.org>
    Cc: Horst Schirmeier <horst@schirmeier.com>
    Cc: Jan Beulich <jbeulich@novell.com>
    Cc: Daniel Drake <dsd@gentoo.org>
    Cc: Andi Kleen <ak@suse.de>
    Cc: Randy Dunlap <randy.dunlap@oracle.com>
    Signed-off-by: Oleg Verych <olecom@flower.upol.cz>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Linus and Oleg tried to fix it with various subsequent changes
to scripts/Kbuild.include, but it's still broken.

For some reason $(call ...) invocations add spaces.  I tried
another test case:

define test
 $(shell echo -n)
endef

VAR:=$(call test)

all:
	@echo "\'$(VAR)\'"

And this always prints:

' TEST'

even with GNU Make version 3.81

If I put a $(strip ...) around the define, the space is
still there.  If I put the $(strip ...) around VAR's
$(call), the space goes away.

Re: Kbuild change breaks the ppc64 build

[David Miller]

From: Paul Mackerras <paulus@samba.org>
Date: Thu, 8 Feb 2007 22:10:54 +1100

> Commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7 breaks the build on
> 64-bit powerpc because we no longer get the -m64 flag passed to gcc.
> There is code in arch/powerpc/Makefile which adds (or used to add)
> -m64 to AS, LD and CC if we are running on a 64-bit machine (which I
> am) and have a biarch toolchain (which I do).  Without -m64, the
> toolchain assumes 32-bit and all sorts of things break spectacularly.
> 
> I haven't yet tracked down exactly why this commit has this effect,
> since I find it takes considerable time and effort to understand
> Kbuild.

It's this commit:

commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7
Author: Oleg Verych <olecom@flower.upol.cz>
Date:   Tue Feb 6 02:18:21 2007 +0100

    [PATCH] kbuild: improve option checking, Kbuild.include cleanup
    
     GNU binutils, root users, tmpfiles, external modules ro builds must
     be fixed to do the right thing now.
    
    Cc: Roman Zippel <zippel@linux-m68k.org>
    Cc: Sam Ravnborg <sam@ravnborg.org>
    Cc: Horst Schirmeier <horst@schirmeier.com>
    Cc: Jan Beulich <jbeulich@novell.com>
    Cc: Daniel Drake <dsd@gentoo.org>
    Cc: Andi Kleen <ak@suse.de>
    Cc: Randy Dunlap <randy.dunlap@oracle.com>
    Signed-off-by: Oleg Verych <olecom@flower.upol.cz>
    Signed-off-by: Linus Torvalds <torvalds@linux-foundation.org>

Linus and Oleg tried to fix it with various subsequent changes
to scripts/Kbuild.include, but it's still broken.

For some reason $(call ...) invocations add spaces.  I tried
another test case:

define test
 $(shell echo -n)
endef

VAR:=$(call test)

all:
	@echo "\'$(VAR)\'"

And this always prints:

' TEST'

even with GNU Make version 3.81

If I put a $(strip ...) around the define, the space is
still there.  If I put the $(strip ...) around VAR's
$(call), the space goes away.

Re: Kbuild change breaks the ppc64 build

[Oleg Verych]

On Thu, Feb 08, 2007 at 04:00:03AM -0800, David Miller wrote:
> From: Paul Mackerras <paulus@samba.org>
> Date: Thu, 8 Feb 2007 22:10:54 +1100
[]
> Linus and Oleg tried to fix it with various subsequent changes
> to scripts/Kbuild.include, but it's still broken.

And Roland McGrath, but it turned to be very bad.

Maybe you have older set of fixes. I have x86-64 and i386 built,
though with make 3.81(debian).

But it may be ppc specific, as

        CFLAGS += $(call cc-option,-mcpu=970)
	else
	CFLAGS += $(call cc-option,-mcpu=power4)

may be turned to be options without spaces (make V=1)!

> For some reason $(call ...) invocations add spaces.  I tried
> another test case:
> 
> define test
>  $(shell echo -n)
> endef
> 
> VAR:=$(call test)
> 
> all:
> 	@echo "\'$(VAR)\'"
> 
> And this always prints:
> 
> ' TEST'
> 
> even with GNU Make version 3.81
> 
> If I put a $(strip ...) around the define, the space is
> still there.  If I put the $(strip ...) around VAR's
> $(call), the space goes away.

____

Re: Kbuild change breaks the ppc64 build

[Oleg Verych]

On Thu, Feb 08, 2007 at 04:00:03AM -0800, David Miller wrote:
> From: Paul Mackerras <paulus@samba.org>
> Date: Thu, 8 Feb 2007 22:10:54 +1100
[]
> Linus and Oleg tried to fix it with various subsequent changes
> to scripts/Kbuild.include, but it's still broken.

And Roland McGrath, but it turned to be very bad.

Maybe you have older set of fixes. I have x86-64 and i386 built,
though with make 3.81(debian).

But it may be ppc specific, as

        CFLAGS += $(call cc-option,-mcpu=970)
	else
	CFLAGS += $(call cc-option,-mcpu=power4)

may be turned to be options without spaces (make V=1)!

> For some reason $(call ...) invocations add spaces.  I tried
> another test case:
> 
> define test
>  $(shell echo -n)
> endef
> 
> VAR:=$(call test)
> 
> all:
> 	@echo "\'$(VAR)\'"
> 
> And this always prints:
> 
> ' TEST'
> 
> even with GNU Make version 3.81
> 
> If I put a $(strip ...) around the define, the space is
> still there.  If I put the $(strip ...) around VAR's
> $(call), the space goes away.

____

Re: Kbuild change breaks the ppc64 build

[Michal Ostrowski]

Isn't the extra space there because you've included it in the definition
of "test"?  The attempt at introducing indentation introduces the extra
space character.  Defining test without the internal indentation should
produce the results you are looking for.


-- 
Michal Ostrowski <mostrows@watson.ibm.com>


On Thu, 2007-02-08 at 04:00 -0800, David Miller wrote:

> For some reason $(call ...) invocations add spaces.  I tried
> another test case:
> 
> define test
>  $(shell echo -n)
> endef
> 
> VAR:=$(call test)
> 
> all:
> 	@echo "\'$(VAR)\'"
> 
> And this always prints:
> 
> ' TEST'
> 
> even with GNU Make version 3.81
> 
> If I put a $(strip ...) around the define, the space is
> still there.  If I put the $(strip ...) around VAR's
> $(call), the space goes away.

Re: Kbuild change breaks the ppc64 build

[Michal Ostrowski]

Isn't the extra space there because you've included it in the definition
of "test"?  The attempt at introducing indentation introduces the extra
space character.  Defining test without the internal indentation should
produce the results you are looking for.


-- 
Michal Ostrowski <mostrows@watson.ibm.com>


On Thu, 2007-02-08 at 04:00 -0800, David Miller wrote:

> For some reason $(call ...) invocations add spaces.  I tried
> another test case:
> 
> define test
>  $(shell echo -n)
> endef
> 
> VAR:=$(call test)
> 
> all:
> 	@echo "\'$(VAR)\'"
> 
> And this always prints:
> 
> ' TEST'
> 
> even with GNU Make version 3.81
> 
> If I put a $(strip ...) around the define, the space is
> still there.  If I put the $(strip ...) around VAR's
> $(call), the space goes away.

Re: Kbuild change breaks the ppc64 build

[Oleg Verych]

On Thu, Feb 08, 2007 at 10:10:54PM +1100, Paul Mackerras wrote:
> Commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7 breaks the build on
> 64-bit powerpc because we no longer get the -m64 flag passed to gcc.
> There is code in arch/powerpc/Makefile which adds (or used to add)
> -m64 to AS, LD and CC if we are running on a 64-bit machine (which I
> am) and have a biarch toolchain (which I do).  Without -m64, the
> toolchain assumes 32-bit and all sorts of things break spectacularly.
> 
> I haven't yet tracked down exactly why this commit has this effect,
> since I find it takes considerable time and effort to understand
> Kbuild.

As i have refactored some CC checking code in Kbuild.include, it
turned, that some versions of `make' after calling nested functions,
add (or leave) prefix whitespace to result, thus ifeq[0] fails:

 .-*- (ppc Makefile)
 |HAS_BIARCH      := $(call cc-option-yn, -m32)
0|ifeq ($(HAS_BIARCH),y)
 |AS              := $(AS) -a32
 |LD              := $(LD) -m elf32ppc
 |CC              := $(CC) -m32
 |endif

David wrote and showed a testcase.
Adding $(strip ) wrapper helped with this, but may screw many other
things. Maybe you will help, unless Linus will finally revert this :)

____

Re: Kbuild change breaks the ppc64 build

[Oleg Verych]

On Thu, Feb 08, 2007 at 10:10:54PM +1100, Paul Mackerras wrote:
> Commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7 breaks the build on
> 64-bit powerpc because we no longer get the -m64 flag passed to gcc.
> There is code in arch/powerpc/Makefile which adds (or used to add)
> -m64 to AS, LD and CC if we are running on a 64-bit machine (which I
> am) and have a biarch toolchain (which I do).  Without -m64, the
> toolchain assumes 32-bit and all sorts of things break spectacularly.
> 
> I haven't yet tracked down exactly why this commit has this effect,
> since I find it takes considerable time and effort to understand
> Kbuild.

As i have refactored some CC checking code in Kbuild.include, it
turned, that some versions of `make' after calling nested functions,
add (or leave) prefix whitespace to result, thus ifeq[0] fails:

 .-*- (ppc Makefile)
 |HAS_BIARCH      := $(call cc-option-yn, -m32)
0|ifeq ($(HAS_BIARCH),y)
 |AS              := $(AS) -a32
 |LD              := $(LD) -m elf32ppc
 |CC              := $(CC) -m32
 |endif

David wrote and showed a testcase.
Adding $(strip ) wrapper helped with this, but may screw many other
things. Maybe you will help, unless Linus will finally revert this :)

____

Re: Kbuild change breaks the ppc64 build

[David Miller]

From: Oleg Verych <olecom@flower.upol.cz>
Date: Thu, 8 Feb 2007 13:47:56 +0100

> On Thu, Feb 08, 2007 at 10:10:54PM +1100, Paul Mackerras wrote:
> > Commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7 breaks the build on
> > 64-bit powerpc because we no longer get the -m64 flag passed to gcc.
> > There is code in arch/powerpc/Makefile which adds (or used to add)
> > -m64 to AS, LD and CC if we are running on a 64-bit machine (which I
> > am) and have a biarch toolchain (which I do).  Without -m64, the
> > toolchain assumes 32-bit and all sorts of things break spectacularly.
> > 
> > I haven't yet tracked down exactly why this commit has this effect,
> > since I find it takes considerable time and effort to understand
> > Kbuild.
> 
> As i have refactored some CC checking code in Kbuild.include, it
> turned, that some versions of `make' after calling nested functions,
> add (or leave) prefix whitespace to result, thus ifeq[0] fails:

That's not it, did you see my fix for this problem posted
already?  The issue is the leading spaces on the first
line of the define for checker-shell.  Those propagate
into the callers, although they are reduced down to a
single space.

This will give you spaces in the result on any version of
make.

Here is my fix again for your reference.

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 8d7eabf..a1880e8 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -60,16 +60,15 @@ endef
 # Usage: option = $(call checker-shell,$(CC)...-o $$OUT,option-ok,otherwise)
 # Exit code chooses option. $$OUT is safe location for needless output.
 define checker-shell
- $(strip
-  $(shell set -e; \
-    DIR=$(KBUILD_EXTMOD); \
-    cd $${DIR:-$(objtree)}; \
-    OUT=$$PWD/.$$$$.null; \
-    if $(1) >/dev/null 2>&1; \
-      then echo "$(2)"; \
-      else echo "$(3)"; \
-    fi; \
-    rm -f $$OUT))
+$(shell set -e; \
+  DIR=$(KBUILD_EXTMOD); \
+  cd $${DIR:-$(objtree)}; \
+  OUT=$$PWD/.$$$$.null; \
+  if $(1) >/dev/null 2>&1; \
+    then echo "$(2)"; \
+    else echo "$(3)"; \
+  fi; \
+  rm -f $$OUT)
 endef
 
 # as-option

Re: Kbuild change breaks the ppc64 build

[David Miller]

From: Oleg Verych <olecom@flower.upol.cz>
Date: Thu, 8 Feb 2007 13:47:56 +0100

> On Thu, Feb 08, 2007 at 10:10:54PM +1100, Paul Mackerras wrote:
> > Commit 5de043f4bd11a9e0a3e8daec7d1905da575a76b7 breaks the build on
> > 64-bit powerpc because we no longer get the -m64 flag passed to gcc.
> > There is code in arch/powerpc/Makefile which adds (or used to add)
> > -m64 to AS, LD and CC if we are running on a 64-bit machine (which I
> > am) and have a biarch toolchain (which I do).  Without -m64, the
> > toolchain assumes 32-bit and all sorts of things break spectacularly.
> > 
> > I haven't yet tracked down exactly why this commit has this effect,
> > since I find it takes considerable time and effort to understand
> > Kbuild.
> 
> As i have refactored some CC checking code in Kbuild.include, it
> turned, that some versions of `make' after calling nested functions,
> add (or leave) prefix whitespace to result, thus ifeq[0] fails:

That's not it, did you see my fix for this problem posted
already?  The issue is the leading spaces on the first
line of the define for checker-shell.  Those propagate
into the callers, although they are reduced down to a
single space.

This will give you spaces in the result on any version of
make.

Here is my fix again for your reference.

diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
index 8d7eabf..a1880e8 100644
--- a/scripts/Kbuild.include
+++ b/scripts/Kbuild.include
@@ -60,16 +60,15 @@ endef
 # Usage: option = $(call checker-shell,$(CC)...-o $$OUT,option-ok,otherwise)
 # Exit code chooses option. $$OUT is safe location for needless output.
 define checker-shell
- $(strip
-  $(shell set -e; \
-    DIR=$(KBUILD_EXTMOD); \
-    cd $${DIR:-$(objtree)}; \
-    OUT=$$PWD/.$$$$.null; \
-    if $(1) >/dev/null 2>&1; \
-      then echo "$(2)"; \
-      else echo "$(3)"; \
-    fi; \
-    rm -f $$OUT))
+$(shell set -e; \
+  DIR=$(KBUILD_EXTMOD); \
+  cd $${DIR:-$(objtree)}; \
+  OUT=$$PWD/.$$$$.null; \
+  if $(1) >/dev/null 2>&1; \
+    then echo "$(2)"; \
+    else echo "$(3)"; \
+  fi; \
+  rm -f $$OUT)
 endef
 
 # as-option

final (Re: Kbuild change breaks the ppc64 build)

[Oleg Verych]

Date: Feb 08, 2007 at 05:17:06AM -0800
From: David Miller

> From: Oleg Verych <olecom@flower.upol.cz>
> Date: Thu, 8 Feb 2007 13:47:56 +0100
> 
[]
> > As i have refactored some CC checking code in Kbuild.include, it
> > turned, that some versions of `make' after calling nested functions,
> > add (or leave) prefix whitespace to result, thus ifeq[0] fails:
> 
> That's not it, did you see my fix for this problem posted
> already?

Yes, i did. Thanks, David, for fix and for comprehencive approach.

Kudos to everyone, who was forced to deal with this cr@p. Sorry for my
confusion, that fast commit was a bit of stress.

> The issue is the leading spaces on the first line of the define for
> checker-shell.  Those propagate into the callers, although they are
> reduced down to a single space.

Yes, defines leave spaces/tabs as is, it's known thing, but again,
sorry.

> This will give you spaces in the result on any version of
> make.
> 
> Here is my fix again for your reference.
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 8d7eabf..a1880e8 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -60,16 +60,15 @@ endef
>  # Usage: option = $(call checker-shell,$(CC)...-o $$OUT,option-ok,otherwise)
>  # Exit code chooses option. $$OUT is safe location for needless output.
>  define checker-shell
> - $(strip
   ^
Roland had no space here in his patch ;)

> -  $(shell set -e; \
> -    DIR=$(KBUILD_EXTMOD); \
> -    cd $${DIR:-$(objtree)}; \
> -    OUT=$$PWD/.$$$$.null; \
> -    if $(1) >/dev/null 2>&1; \
> -      then echo "$(2)"; \
> -      else echo "$(3)"; \
> -    fi; \
> -    rm -f $$OUT))
> +$(shell set -e; \
> +  DIR=$(KBUILD_EXTMOD); \
> +  cd $${DIR:-$(objtree)}; \
> +  OUT=$$PWD/.$$$$.null; \
> +  if $(1) >/dev/null 2>&1; \
> +    then echo "$(2)"; \
> +    else echo "$(3)"; \
> +  fi; \
> +  rm -f $$OUT)
>  endef
>  
>  # as-option
>
> 

Thanks!
____

final (Re: Kbuild change breaks the ppc64 build)

[Oleg Verych]

Date: Feb 08, 2007 at 05:17:06AM -0800
From: David Miller

> From: Oleg Verych <olecom@flower.upol.cz>
> Date: Thu, 8 Feb 2007 13:47:56 +0100
> 
[]
> > As i have refactored some CC checking code in Kbuild.include, it
> > turned, that some versions of `make' after calling nested functions,
> > add (or leave) prefix whitespace to result, thus ifeq[0] fails:
> 
> That's not it, did you see my fix for this problem posted
> already?

Yes, i did. Thanks, David, for fix and for comprehencive approach.

Kudos to everyone, who was forced to deal with this cr@p. Sorry for my
confusion, that fast commit was a bit of stress.

> The issue is the leading spaces on the first line of the define for
> checker-shell.  Those propagate into the callers, although they are
> reduced down to a single space.

Yes, defines leave spaces/tabs as is, it's known thing, but again,
sorry.

> This will give you spaces in the result on any version of
> make.
> 
> Here is my fix again for your reference.
> 
> diff --git a/scripts/Kbuild.include b/scripts/Kbuild.include
> index 8d7eabf..a1880e8 100644
> --- a/scripts/Kbuild.include
> +++ b/scripts/Kbuild.include
> @@ -60,16 +60,15 @@ endef
>  # Usage: option = $(call checker-shell,$(CC)...-o $$OUT,option-ok,otherwise)
>  # Exit code chooses option. $$OUT is safe location for needless output.
>  define checker-shell
> - $(strip
   ^
Roland had no space here in his patch ;)

> -  $(shell set -e; \
> -    DIR=$(KBUILD_EXTMOD); \
> -    cd $${DIR:-$(objtree)}; \
> -    OUT=$$PWD/.$$$$.null; \
> -    if $(1) >/dev/null 2>&1; \
> -      then echo "$(2)"; \
> -      else echo "$(3)"; \
> -    fi; \
> -    rm -f $$OUT))
> +$(shell set -e; \
> +  DIR=$(KBUILD_EXTMOD); \
> +  cd $${DIR:-$(objtree)}; \
> +  OUT=$$PWD/.$$$$.null; \
> +  if $(1) >/dev/null 2>&1; \
> +    then echo "$(2)"; \
> +    else echo "$(3)"; \
> +  fi; \
> +  rm -f $$OUT)
>  endef
>  
>  # as-option
>
> 

Thanks!
____