diff mbox

[U-Boot,v4,32/37] Makefile: refactor tools-all targets

Message ID 1389336274-7234-33-git-send-email-yamada.m@jp.panasonic.com
State Superseded
Delegated to: Tom Rini
Headers show

Commit Message

Masahiro Yamada Jan. 10, 2014, 6:44 a.m. UTC
- Move "easylogo", "env", "gdb" tagets to tools/Makefile
 - Delete "gdbtools" target (same as "gdb")

Signed-off-by: Masahiro Yamada <yamada.m@jp.panasonic.com>
---

Changes in v4: None
Changes in v3: None
Changes in v2: None

 Makefile       | 7 +------
 tools/Makefile | 6 +++++-
 2 files changed, 6 insertions(+), 7 deletions(-)

Comments

Gerhard Sittig Jan. 10, 2014, 2:26 p.m. UTC | #1
On Fri, Jan 10, 2014 at 15:44 +0900, Masahiro Yamada wrote:
> 
>  - Move "easylogo", "env", "gdb" tagets to tools/Makefile
>  - Delete "gdbtools" target (same as "gdb")

This appears to be an "incompatible" change with regards to how
users can compile these tools.  Please see tools/env/README and
update it as well.  I'm aware of external projects which grab a
U-Boot source tree and 'make env' to just get the fw_printenv(1)
binary.


There is another issue for those who do

  make HOSTCC=${CROSS_COMPILE}gcc env
  (needs a HOSTSTRIP spec as well in recent mainline sources)

to get an fw_printenv(1) binary that runs on the target (in
contrast to the build machine).  Some yocto recipes do this, to
not re-invent how to build this tool or how to read and write the
environment image.

With your changes, the 'env' make target is gone, instead there
is a 'tools-all' target but it has a wider scope, includes the
BMP manipulation/conversion stuff and uses HOSTCC to build a
bmp_logo(1) tool, which breaks in the mentioned use case (cross
compiled bmp_logo(1) executable, empty bmp_logo.h header, aborted
build sequence).

Can you please look into whether the sources in the tools/env/
directory can get compiled without involving other directories?
I tried 'make tools/env/', but this "make dir/" syntax as known
from kernel builds appears to not be supported in U-Boot (yet? or
have I done something wrong?).  I did a quick hack and extended
the top level Makefile as outlined below (diff copied here with
the clipboard, whitespace will be broken, build tested but not
yet run tested, only checked file(1) output).

  --- a/Makefile
  +++ b/Makefile
  @@ -1124,6 +1124,9 @@ xmldocs pdfdocs psdocs htmldocs mandocs: tools/kernel-doc/docproc
   tools-all: $(VERSION_FILE) $(TIMESTAMP_FILE)
	  $(Q)$(MAKE) $(build)=tools HOST_TOOLS_ALL=y
   
  +tools-env: $(VERSION_FILE) $(TIMESTAMP_FILE)
  +       $(Q)$(MAKE) $(build)=tools/env
  +
   .PHONY : CHANGELOG
   CHANGELOG:
	  git log --no-merges U-Boot-1_1_5.. | \


While testing your series I noticed a probably missing
dependency:  Running something different from 'make all' (or make
without a target spec) after 'make <board>_config' won't work
since the fixdeps(1) tool is missing.  It's a "byproduct" of
'make all', afterwards other targets can get built separately.
Is there a "preparation" step that one needs to take if not
calling 'make all' immediately after configuration?


For the record:  My test was with your v4 series on top of
v2014.01-rc2-43-ge7be18225fbe plus your "sandbox: Use system
headers first for sandbox's os.c in a different way" change, with
ELDK 5.3 and a PowerPC target.

  $ . /opt/eldk-5.3-qte/powerpc/environment-setup-powerpc-linux
  $ env -u LDFLAGS ARCH=powerpc CROSS_COMPILE=$TARGET_PREFIX $SHELL

  $ mkdir output-ac14xx && cd $_
  $ make -C .. O=`pwd` ac14xx_config

  $ make HOSTCC=${CROSS_COMPILE}gcc tools/env/
  "Nothing to be done ..."
  $ make HOSTCC=${CROSS_COMPILE}gcc tools-all
    HOSTCC  tools/easylogo/easylogo
  /bin/sh: 1: scripts/basic/fixdep: not found
  make[4]: *** [tools/easylogo/easylogo] Error 127
  make[3]: *** [tools/easylogo] Error 2
  make[2]: *** [tools-all] Error 2
  make[1]: *** [sub-make] Error 2
  make: *** [all] Error 2
  $ make HOSTCC=${CROSS_COMPILE}gcc tools
    CC      lib/asm-offsets.s
    GEN     include/generated/generic-asm-offsets.h
    CC      /asm-offsets.s
  touch: cannot touch `/asm-offsets.s': Permission denied
  make[2]: *** [/asm-offsets.s] Error 1
  make[1]: *** [sub-make] Error 2
  make: *** [all] Error 2

  [ identical errors without the HOSTCC spec ]

  $ time make
  ...
  HOSTCC  scripts/basic/fixdep
  ...
  [ native tools/ gets built as well, but not tools/env/ it seems ]

  $ make HOSTCC=${CROSS_COMPILE}gcc tools-all
  ...
  tools/bmp_logo: 1: tools/bmp_logo: ELF: not found
  tools/bmp_logo: 2: tools/bmp_logo: Syntax error: "(" unexpected
  ...

  $ $EDITOR ../Makefile
  $ make tools-all
  $ make HOSTCC=${CROSS_COMPILE}gcc tools-env
  $ file tools/env/fw_printenv
  tools/env/fw_printenv: ELF 32-bit MSB executable, PowerPC or cisco 4500 [ ... ]


Let me say that I do appreciate the work that you spend to
provide us all with the comfort of Kbuild and "real"
out-of-source builds, and cleaned up build infrastructure for
improved maintenance, and future Kconfig support. :-]

I will report back after doing run time tests.


virtually yours
Gerhard Sittig
Masahiro Yamada Jan. 15, 2014, 2:23 a.m. UTC | #2
Hello Gerhard.


> 
> There is another issue for those who do
> 
>   make HOSTCC=${CROSS_COMPILE}gcc env
>   (needs a HOSTSTRIP spec as well in recent mainline sources)
> 
> to get an fw_printenv(1) binary that runs on the target (in
> contrast to the build machine).  Some yocto recipes do this, to
> not re-invent how to build this tool or how to read and write the
> environment image.
> 
> With your changes, the 'env' make target is gone, instead there
> is a 'tools-all' target but it has a wider scope, includes the
> BMP manipulation/conversion stuff and uses HOSTCC to build a
> bmp_logo(1) tool, which breaks in the mentioned use case (cross
> compiled bmp_logo(1) executable, empty bmp_logo.h header, aborted
> build sequence).

I did not know how "env" target have been used.
Your comments are highly appreciated!

I will revive "env" target at v5.

But whan I cannot understand is why HOSTCC must be tweaked.
(I am also asking this question in "Compiling fw_printenv tool" thread)

If fw_printenv must be always compiled with cross-tools,
I'd like to suggest to use $(CC) instead of $(HOSTCC)
in tools/env/Makefile.

> While testing your series I noticed a probably missing
> dependency:  Running something different from 'make all' (or make
> without a target spec) after 'make <board>_config' won't work
> since the fixdeps(1) tool is missing.  It's a "byproduct" of
> 'make all', afterwards other targets can get built separately.
> Is there a "preparation" step that one needs to take if not
> calling 'make all' immediately after configuration?

Good catch!
I will fix this problem at v5.


Best Regards
Masahiro Yamada
Gerhard Sittig Jan. 15, 2014, 2:52 p.m. UTC | #3
On Wed, Jan 15, 2014 at 11:23 +0900, Masahiro Yamada wrote:
> 
> I did not know how "env" target have been used.
> Your comments are highly appreciated!
> 
> I will revive "env" target at v5.
> 
> But whan I cannot understand is why HOSTCC must be tweaked.
> (I am also asking this question in "Compiling fw_printenv tool" thread)
> 
> If fw_printenv must be always compiled with cross-tools,
> I'd like to suggest to use $(CC) instead of $(HOSTCC)
> in tools/env/Makefile.

for the record:  I've replied in the other thread, to not clobber
the kbuild review thread and archives with env related discussion


virtually yours
Gerhard Sittig
diff mbox

Patch

diff --git a/Makefile b/Makefile
index c2fd3d7..aec713a 100644
--- a/Makefile
+++ b/Makefile
@@ -1062,15 +1062,10 @@  $(TIMESTAMP_FILE):
 		@LC_ALL=C date +'#define U_BOOT_TIME "%T"' >> $@.tmp
 		@cmp -s $@ $@.tmp && rm -f $@.tmp || mv -f $@.tmp $@
 
-easylogo env gdb:
-	$(Q)$(MAKE) $(build)=tools/$@
-
-gdbtools: gdb
-
 xmldocs pdfdocs psdocs htmldocs mandocs: tools/kernel-doc/docproc
 	$(Q)$(MAKE) U_BOOT_VERSION=$(U_BOOT_VERSION) $(build)=doc/DocBook $@
 
-tools-all: easylogo env gdb $(VERSION_FILE) $(TIMESTAMP_FILE)
+tools-all: $(VERSION_FILE) $(TIMESTAMP_FILE)
 	$(Q)$(MAKE) $(build)=tools HOST_TOOLS_ALL=y
 
 .PHONY : CHANGELOG
diff --git a/tools/Makefile b/tools/Makefile
index 143bbe0..1ff8c13 100644
--- a/tools/Makefile
+++ b/tools/Makefile
@@ -24,6 +24,10 @@  CONFIG_NETCONSOLE = y
 CONFIG_SHA1_CHECK_UB_IMG = y
 endif
 
+subdir-$(HOST_TOOLS_ALL) += easylogo
+subdir-$(HOST_TOOLS_ALL) += env
+subdir-$(HOST_TOOLS_ALL) += gdb
+
 # Merge all the different vars for envcrc into one
 ENVCRC-$(CONFIG_ENV_IS_EMBEDDED) = y
 ENVCRC-$(CONFIG_ENV_IS_IN_DATAFLASH) = y
@@ -178,7 +182,7 @@  HOST_EXTRACFLAGS += -include $(SRCTREE)/include/libfdt_env.h \
 
 __build:	$(LOGO-y)
 
-subdir-y := kernel-doc
+subdir-y += kernel-doc
 
 $(LOGO_H):	$(obj)/bmp_logo $(LOGO_BMP)
 	$(obj)/bmp_logo --gen-info $(LOGO_BMP) > $@