Re: [PATCH] kbuild: add script check for cross compilation utilities

2019-05-13 Thread Nick Desaulniers
On Sat, May 11, 2019 at 8:05 PM Masahiro Yamada
 wrote:
>
> On Sat, May 11, 2019 at 11:25 AM Nathan Chancellor
>  wrote:
> >
> > Few comments below but nothing major, this seems to work fine as is.
> >
> > On Thu, May 09, 2019 at 01:19:21PM -0700, 'Nick Desaulniers' via Clang 
> > Built Linux wrote:
> > > When cross compiling via setting CROSS_COMPILE, if the prefixed tools
> > > are not found, then the host utilities are often instead invoked, and
> > > produce often difficult to understand errors.  This is most commonly the
> > > case for developers new to cross compiling the kernel that have yet to
> > > install the proper cross compilation toolchain. Rather than charge
> > > headlong into a build that will fail obscurely, check that the tools
> > > exist before starting to compile, and fail with a friendly error
> > > message.
> >
> > This part of the commit message makes it sound like this is a generic
> > problem when it is actually specific to clang. make will fail on its
> > own when building with gcc if CROSS_COMPILE is not properly set (since
> > gcc won't be found).
> >
> > On a side note, seems kind of odd that clang falls back to the host
> > tools when a non-host --target argument is used... (how in the world is
> > that expected to work?)
>
>
> I agree.
> Failure is much better than falling back to host tools.

It was probably assumed that the default case is usually not cross
compilation.  But I think we can add a check to Clang's driver where
`if target_triple != host_triple then don't invoke host tools`.

-- 
Thanks,
~Nick Desaulniers


Re: [PATCH] kbuild: add script check for cross compilation utilities

2019-05-11 Thread Masahiro Yamada
On Sat, May 11, 2019 at 11:25 AM Nathan Chancellor
 wrote:
>
> Few comments below but nothing major, this seems to work fine as is.
>
> On Thu, May 09, 2019 at 01:19:21PM -0700, 'Nick Desaulniers' via Clang Built 
> Linux wrote:
> > When cross compiling via setting CROSS_COMPILE, if the prefixed tools
> > are not found, then the host utilities are often instead invoked, and
> > produce often difficult to understand errors.  This is most commonly the
> > case for developers new to cross compiling the kernel that have yet to
> > install the proper cross compilation toolchain. Rather than charge
> > headlong into a build that will fail obscurely, check that the tools
> > exist before starting to compile, and fail with a friendly error
> > message.
>
> This part of the commit message makes it sound like this is a generic
> problem when it is actually specific to clang. make will fail on its
> own when building with gcc if CROSS_COMPILE is not properly set (since
> gcc won't be found).
>
> On a side note, seems kind of odd that clang falls back to the host
> tools when a non-host --target argument is used... (how in the world is
> that expected to work?)


I agree.
Failure is much better than falling back to host tools.


-- 
Best Regards
Masahiro Yamada


Re: [PATCH] kbuild: add script check for cross compilation utilities

2019-05-11 Thread Masahiro Yamada
Hi.

On Fri, May 10, 2019 at 5:19 AM Nick Desaulniers
 wrote:
>
> When cross compiling via setting CROSS_COMPILE, if the prefixed tools
> are not found, then the host utilities are often instead invoked, and
> produce often difficult to understand errors.  This is most commonly the
> case for developers new to cross compiling the kernel that have yet to
> install the proper cross compilation toolchain. Rather than charge
> headlong into a build that will fail obscurely, check that the tools
> exist before starting to compile, and fail with a friendly error
> message.

I see one drawback in adding the check script
so early in the top Makefile.

Make targets that do not require toolchain at all
("make clean", "make headers_install", etc.)
would fail too.


>
> Before:
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
> ...
> /usr/bin/as: unrecognized option '-EL'
> clang: error: assembler command failed with exit code 1 (use -v to see
> invocation)
> make[2]: *** [../scripts/Makefile.build:279: scripts/mod/empty.o] Error 1
> make[2]: *** Waiting for unfinished jobs
> make[1]: *** [/linux/Makefile:1118:
> prepare0] Error 2
> make: *** [Makefile:179: sub-make] Error 2


What a coincidence, I had sent this patch before:
https://patchwork.kernel.org/patch/10936811/


With my patch applied, the command above would be failed like this:


$ make ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf-  CC=clang defconfig all
  HOSTCC  scripts/basic/fixdep
  HOSTCC  scripts/kconfig/conf.o
  HOSTCC  scripts/kconfig/confdata.o
  HOSTCC  scripts/kconfig/expr.o
  LEX scripts/kconfig/lexer.lex.c
  YACCscripts/kconfig/parser.tab.h
  HOSTCC  scripts/kconfig/lexer.lex.o
  YACCscripts/kconfig/parser.tab.c
  HOSTCC  scripts/kconfig/parser.tab.o
  HOSTCC  scripts/kconfig/preprocess.o
  HOSTCC  scripts/kconfig/symbol.o
  HOSTLD  scripts/kconfig/conf
*** Default configuration is based on 'multi_v7_defconfig'
scripts/Kconfig.include:35: linker 'arm-linux-gnueabihf-ld' not found
make[2]: *** [scripts/kconfig/Makefile;82: defconfig] Error 1
make[1]: *** [Makefile;557: defconfig] Error 2
make: *** [Makefile;325: __build_one_by_one] Error 2




The presence $(CC) and $(LD) are checked in the Kconfig stage.
If it is not enough, it is OK to add $(AS) check.

$(error-if,$(failure,command -v $(AS)),assembler '$(AS)' not found)


But, I do not want to add all of $(AR), $(NM), ... etc.


Thanks.



> After:
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
> $CROSS_COMPILE set to arm-linux-gnueabihf-, but unable to find
> arm-linux-gnueabihf-as.
> Makefile:522: recipe for target 'outputmakefile' failed
> make: *** [outputmakefile] Error 1
>
> Signed-off-by: Nick Desaulniers 
> ---
> Note: this is probably more generally useful, but after a few minutes
> wrestling with Make errors related to "recipe commences before first
> target" and "missing separator," I came to understand my hatred of GNU
> Make. Open to sugguestions for where better to invoke this from the top
> level Makefile.
>
>  Makefile  |  1 +
>  scripts/check_crosscompile.sh | 18 ++
>  2 files changed, 19 insertions(+)
>  create mode 100755 scripts/check_crosscompile.sh
>
> diff --git a/Makefile b/Makefile
> index a61a95b6b38f..774339674b59 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -519,6 +519,7 @@ endif
>
>  ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
>  ifneq ($(CROSS_COMPILE),)
> +   $(Q)$(CONFIG_SHELL) $(srctree)/scripts/check_crosscompile.sh
>  CLANG_FLAGS:= --target=$(notdir $(CROSS_COMPILE:%-=%))
>  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS+= --prefix=$(GCC_TOOLCHAIN_DIR)
> diff --git a/scripts/check_crosscompile.sh b/scripts/check_crosscompile.sh
> new file mode 100755
> index ..f4586fbfee18
> --- /dev/null
> +++ b/scripts/check_crosscompile.sh
> @@ -0,0 +1,18 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# (c) 2019, Nick Desaulniers 
> +function check () {
> +  # Remove trailing commands, for example arch/arm/Makefile may add `-EL`.
> +  utility=$(echo ${1} | awk '{print $1;}')
> +  command -v "${utility}" &> /dev/null
> +  if [[ $? != 0 ]]; then
> +echo "\$CROSS_COMPILE set to ${CROSS_COMPILE}," \
> +  "but unable to find ${utility}."
> +exit 1
> +  fi
> +}
> +utilities=("${AS}" "${LD}" "${CC}" "${AR}" "${NM}" "${STRIP}" "${OBJCOPY}"
> +  "${OBJDUMP}")
> +for utility in "${utilities[@]}"; do
> +  check "${utility}"
> +done
> --
> 2.21.0.1020.gf2820cf01a-goog
>


--
Best Regards
Masahiro Yamada


Re: [PATCH] kbuild: add script check for cross compilation utilities

2019-05-10 Thread Nathan Chancellor
Few comments below but nothing major, this seems to work fine as is.

On Thu, May 09, 2019 at 01:19:21PM -0700, 'Nick Desaulniers' via Clang Built 
Linux wrote:
> When cross compiling via setting CROSS_COMPILE, if the prefixed tools
> are not found, then the host utilities are often instead invoked, and
> produce often difficult to understand errors.  This is most commonly the
> case for developers new to cross compiling the kernel that have yet to
> install the proper cross compilation toolchain. Rather than charge
> headlong into a build that will fail obscurely, check that the tools
> exist before starting to compile, and fail with a friendly error
> message.

This part of the commit message makes it sound like this is a generic
problem when it is actually specific to clang. make will fail on its
own when building with gcc if CROSS_COMPILE is not properly set (since
gcc won't be found).

On a side note, seems kind of odd that clang falls back to the host
tools when a non-host --target argument is used... (how in the world is
that expected to work?)

> 
> Before:
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
> ...
> /usr/bin/as: unrecognized option '-EL'
> clang: error: assembler command failed with exit code 1 (use -v to see
> invocation)
> make[2]: *** [../scripts/Makefile.build:279: scripts/mod/empty.o] Error 1
> make[2]: *** Waiting for unfinished jobs
> make[1]: *** [/linux/Makefile:1118:
> prepare0] Error 2
> make: *** [Makefile:179: sub-make] Error 2
> 
> After:
> $ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
> $CROSS_COMPILE set to arm-linux-gnueabihf-, but unable to find
> arm-linux-gnueabihf-as.
> Makefile:522: recipe for target 'outputmakefile' failed
> make: *** [outputmakefile] Error 1
> 
> Signed-off-by: Nick Desaulniers 

Reviewed-by: Nathan Chancellor 

> ---
> Note: this is probably more generally useful, but after a few minutes
> wrestling with Make errors related to "recipe commences before first
> target" and "missing separator," I came to understand my hatred of GNU
> Make. Open to sugguestions for where better to invoke this from the top
> level Makefile.
> 
>  Makefile  |  1 +
>  scripts/check_crosscompile.sh | 18 ++
>  2 files changed, 19 insertions(+)
>  create mode 100755 scripts/check_crosscompile.sh
> 
> diff --git a/Makefile b/Makefile
> index a61a95b6b38f..774339674b59 100644
> --- a/Makefile
> +++ b/Makefile
> @@ -519,6 +519,7 @@ endif
>  
>  ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
>  ifneq ($(CROSS_COMPILE),)
> + $(Q)$(CONFIG_SHELL) $(srctree)/scripts/check_crosscompile.sh
>  CLANG_FLAGS  := --target=$(notdir $(CROSS_COMPILE:%-=%))
>  GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
>  CLANG_FLAGS  += --prefix=$(GCC_TOOLCHAIN_DIR)
> diff --git a/scripts/check_crosscompile.sh b/scripts/check_crosscompile.sh
> new file mode 100755
> index ..f4586fbfee18
> --- /dev/null
> +++ b/scripts/check_crosscompile.sh
> @@ -0,0 +1,18 @@
> +#!/bin/bash
> +# SPDX-License-Identifier: GPL-2.0
> +# (c) 2019, Nick Desaulniers 

I think a space between the comment and function here would look nicer.

> +function check () {
> +  # Remove trailing commands, for example arch/arm/Makefile may add `-EL`.
> +  utility=$(echo ${1} | awk '{print $1;}')

Shellcheck mentions the ${1} should be quoted.

> +  command -v "${utility}" &> /dev/null
> +  if [[ $? != 0 ]]; then

This can be simplified into:

if ! command -v "${utility}" &> /dev/null; then

> +echo "\$CROSS_COMPILE set to ${CROSS_COMPILE}," \
> +  "but unable to find ${utility}."
> +exit 1
> +  fi
> +}

Maybe a space here and after utilities?

> +utilities=("${AS}" "${LD}" "${CC}" "${AR}" "${NM}" "${STRIP}" "${OBJCOPY}"
> +  "${OBJDUMP}")

I think this would look a little better with the "${OBJDUMP}" aligned to
the "${AS}" (and maybe split the lines to make them evenly align?)

Another note, this script could in theory be invoked via 'sh' if bash
doesn't exist on a system (see CONFIG_SHELL's definition), where only
POSIX compliant constructs should be used (so no arrays). I don't know
how often this occurs to matter (or if it does in this case) but worth
mentioning.

> +for utility in "${utilities[@]}"; do
> +  check "${utility}"
> +done
> -- 
> 2.21.0.1020.gf2820cf01a-goog


[PATCH] kbuild: add script check for cross compilation utilities

2019-05-09 Thread Nick Desaulniers
When cross compiling via setting CROSS_COMPILE, if the prefixed tools
are not found, then the host utilities are often instead invoked, and
produce often difficult to understand errors.  This is most commonly the
case for developers new to cross compiling the kernel that have yet to
install the proper cross compilation toolchain. Rather than charge
headlong into a build that will fail obscurely, check that the tools
exist before starting to compile, and fail with a friendly error
message.

Before:
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
...
/usr/bin/as: unrecognized option '-EL'
clang: error: assembler command failed with exit code 1 (use -v to see
invocation)
make[2]: *** [../scripts/Makefile.build:279: scripts/mod/empty.o] Error 1
make[2]: *** Waiting for unfinished jobs
make[1]: *** [/linux/Makefile:1118:
prepare0] Error 2
make: *** [Makefile:179: sub-make] Error 2

After:
$ ARCH=arm CROSS_COMPILE=arm-linux-gnueabihf- make CC=clang
$CROSS_COMPILE set to arm-linux-gnueabihf-, but unable to find
arm-linux-gnueabihf-as.
Makefile:522: recipe for target 'outputmakefile' failed
make: *** [outputmakefile] Error 1

Signed-off-by: Nick Desaulniers 
---
Note: this is probably more generally useful, but after a few minutes
wrestling with Make errors related to "recipe commences before first
target" and "missing separator," I came to understand my hatred of GNU
Make. Open to sugguestions for where better to invoke this from the top
level Makefile.

 Makefile  |  1 +
 scripts/check_crosscompile.sh | 18 ++
 2 files changed, 19 insertions(+)
 create mode 100755 scripts/check_crosscompile.sh

diff --git a/Makefile b/Makefile
index a61a95b6b38f..774339674b59 100644
--- a/Makefile
+++ b/Makefile
@@ -519,6 +519,7 @@ endif
 
 ifneq ($(shell $(CC) --version 2>&1 | head -n 1 | grep clang),)
 ifneq ($(CROSS_COMPILE),)
+   $(Q)$(CONFIG_SHELL) $(srctree)/scripts/check_crosscompile.sh
 CLANG_FLAGS:= --target=$(notdir $(CROSS_COMPILE:%-=%))
 GCC_TOOLCHAIN_DIR := $(dir $(shell which $(CROSS_COMPILE)elfedit))
 CLANG_FLAGS+= --prefix=$(GCC_TOOLCHAIN_DIR)
diff --git a/scripts/check_crosscompile.sh b/scripts/check_crosscompile.sh
new file mode 100755
index ..f4586fbfee18
--- /dev/null
+++ b/scripts/check_crosscompile.sh
@@ -0,0 +1,18 @@
+#!/bin/bash
+# SPDX-License-Identifier: GPL-2.0
+# (c) 2019, Nick Desaulniers 
+function check () {
+  # Remove trailing commands, for example arch/arm/Makefile may add `-EL`.
+  utility=$(echo ${1} | awk '{print $1;}')
+  command -v "${utility}" &> /dev/null
+  if [[ $? != 0 ]]; then
+echo "\$CROSS_COMPILE set to ${CROSS_COMPILE}," \
+  "but unable to find ${utility}."
+exit 1
+  fi
+}
+utilities=("${AS}" "${LD}" "${CC}" "${AR}" "${NM}" "${STRIP}" "${OBJCOPY}"
+  "${OBJDUMP}")
+for utility in "${utilities[@]}"; do
+  check "${utility}"
+done
-- 
2.21.0.1020.gf2820cf01a-goog