[U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Introduce a new script to check whether file exists and use that check in Makefile to avoid break CI system.
Signed-off-by: Peng Fan peng.fan@nxp.com --- arch/arm/mach-imx/Makefile | 8 ++++++-- tools/imx8_cntr_image.sh | 32 ++++++++++++++++++++++++++++++++ 2 files changed, 38 insertions(+), 2 deletions(-) create mode 100755 tools/imx8_cntr_image.sh
diff --git a/arch/arm/mach-imx/Makefile b/arch/arm/mach-imx/Makefile index 2d79c71371..f0157ca93b 100644 --- a/arch/arm/mach-imx/Makefile +++ b/arch/arm/mach-imx/Makefile @@ -86,9 +86,11 @@ IMX_CONFIG = $(CONFIG_IMX_CONFIG:"%"=%) $(Q)mkdir -p $(dir $@) $(call if_changed_dep,cpp_cfg)
-IMAGE_TYPE = imximage +IMAGE_TYPE := imximage +DEPFILE_EXITS := 1 ifeq ($(CONFIG_ARCH_IMX8), y) -IMAGE_TYPE = imx8image +IMAGE_TYPE := imx8image +DEPFILE_EXITS := $(shell $(srctree)/tools/imx8_cntr_image.sh $(IMX_CONFIG); echo $$?) endif
MKIMAGEFLAGS_u-boot.imx = -n $(filter-out $(PLUGIN).bin $< $(PHONY),$^) \ @@ -104,8 +106,10 @@ MKIMAGEFLAGS_u-boot-dtb.imx = -n $(filter-out $(PLUGIN).bin $< $(PHONY),$^) \ u-boot-dtb.imx: MKIMAGEOUTPUT = u-boot-dtb.imx.log
u-boot-dtb.imx: u-boot-dtb.bin u-boot-dtb.cfgout $(PLUGIN).bin FORCE +ifeq ($(DEPFILE_EXITS),1) $(call if_changed,mkimage) endif +endif
MKIMAGEFLAGS_SPL = -n $(filter-out $(PLUGIN).bin $< $(PHONY),$^) \ -T $(IMAGE_TYPE) -e $(CONFIG_SPL_TEXT_BASE) diff --git a/tools/imx8_cntr_image.sh b/tools/imx8_cntr_image.sh new file mode 100755 index 0000000000..f6725d0444 --- /dev/null +++ b/tools/imx8_cntr_image.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ +# +# script to generate FIT image source for K3 Family boards with +# ATF, OPTEE, SPL and multiple device trees (given on the command line). +# Inspired from board/sunxi/mksunxi_fit_atf.sh +# +# usage: $0 <imximage.cfg> + +file=$1 + +linecount=`cat ${file} | wc -l` + +for ((i=1; i<=${linecount}; i++)); +do + name=`awk -F '\t' -F ' ' 'NR=='${i}' && /^APPEND/ {print $2}' ${file}` + if [ -n "${name}" ]; then + if [ ! -f "${name}" ]; then + echo "WARNING ${name} not found, resulting binary is not-functional" >&2 + exit 0 + fi; + fi; + name=`awk -F '\t' -F ' ' 'NR=='${i}' && (/^IMAGE/ || /^DATA/) {print $3}' ${file}` + if [ -n "${name}" ]; then + if [ ! -f "${name}" ]; then + echo "WARNING ${name} not found, resulting binary is not-functional" >&2 + exit 0 + fi; + fi; +done + +exit 1
Dear Peng Fan,
In message 20181024095456.27486-1-peng.fan@nxp.com you wrote:
Introduce a new script to check whether file exists and use that check in Makefile to avoid break CI system.
Hm... this looks overly complicate to me.
I think you should at least provide more documentation for this script, i. e. what it does, and what the reutrn codes mean.
+DEPFILE_EXITS := 1 ifeq ($(CONFIG_ARCH_IMX8), y) -IMAGE_TYPE = imx8image +IMAGE_TYPE := imx8image +DEPFILE_EXITS := $(shell $(srctree)/tools/imx8_cntr_image.sh $(IMX_CONFIG); echo $$?)
DEPFILE_EXITS ? Or ..._EXISTS ??
+file=$1
+linecount=`cat ${file} | wc -l`
+for ((i=1; i<=${linecount}; i++)); +do
- name=`awk -F '\t' -F ' ' 'NR=='${i}' && /^APPEND/ {print $2}' ${file}`
You mean you first count the lines of the file, then run a for loop over all line numbers (which are otherwise unsused), and then run a awk process for each and every line, making awk read all the file while you just want to process a single line?
Why the hell don;t you just runf awk _once_ over all lines of the files and add the logic (including message printing and return code setting) to the awk script?
This script is a awful waste of processes and CPU resources. [Not to mention the Useless Use of Cat above.]
- if [ -n "${name}" ]; then
if [ ! -f "${name}" ]; thenecho "WARNING ${name} not found, resulting binary is not-functional" >&2exit 0
If a file is not found which is supposed to be there, then this should be an error, and not just a warning. And for such scripts the return code for errors is 1, as 0 is reserved for OK.
+done
+exit 1
The return code in case of no errors is 0.
Best regards,
Wolfgang Denk
-----Original Message----- From: Wolfgang Denk [mailto:wd@denx.de] Sent: 2018年10月24日 22:09 To: Peng Fan peng.fan@nxp.com Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Dear Peng Fan,
In message 20181024095456.27486-1-peng.fan@nxp.com you wrote:
Introduce a new script to check whether file exists and use that check in Makefile to avoid break CI system.
Hm... this looks overly complicate to me.
I think you should at least provide more documentation for this script, i. e. what it does, and what the reutrn codes mean.
Fix in V2.
+DEPFILE_EXITS := 1 ifeq ($(CONFIG_ARCH_IMX8), y) -IMAGE_TYPE = imx8image +IMAGE_TYPE := imx8image +DEPFILE_EXITS := $(shell $(srctree)/tools/imx8_cntr_image.sh +$(IMX_CONFIG); echo $$?)
DEPFILE_EXITS ? Or ..._EXISTS ??
DEPFILE_EXISTS
+file=$1
+linecount=`cat ${file} | wc -l`
+for ((i=1; i<=${linecount}; i++)); +do
- name=`awk -F '\t' -F ' ' 'NR=='${i}' && /^APPEND/ {print $2}'
+${file}`
You mean you first count the lines of the file, then run a for loop over all line numbers (which are otherwise unsused), and then run a awk process for each and every line, making awk read all the file while you just want to process a single line?
Why the hell don;t you just runf awk _once_ over all lines of the files and add the logic (including message printing and return code setting) to the awk script?
The filenames are not always at the same column. So I check each line.
This script is a awful waste of processes and CPU resources. [Not to mention the Useless Use of Cat above.]
Understand, but the imximage.cfg file only has about 20~30 lines. It is very small.
- if [ -n "${name}" ]; then
if [ ! -f "${name}" ]; thenecho "WARNING ${name} not found, resulting binary isnot-functional" >&2
exit 0If a file is not found which is supposed to be there, then this should be an error, and not just a warning. And for such scripts the return code for errors is 1, as 0 is reserved for OK.
I use exit 1 when files not found, use 0 when files found.
Thanks, Peng.
+done
+exit 1
The return code in case of no errors is 0.
Best regards,
Wolfgang Denk
-- DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: (+49)-8142-66989-10 Fax: (+49)-8142-66989-80 Email: wd@denx.de It is better to marry than to burn. - Bible ``I Corinthians'' ch. 7, v. 9
Hi Peng,
On Wed, 24 Oct 2018 09:49:04 +0000 Peng Fan peng.fan@nxp.com wrote: ...
--- /dev/null +++ b/tools/imx8_cntr_image.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ +# +# script to generate FIT image source for K3 Family boards with +# ATF, OPTEE, SPL and multiple device trees (given on the command line). +# Inspired from board/sunxi/mksunxi_fit_atf.sh
Please drop this comment, it doesn't describe what the script is actually doing.
...
+file=$1
+linecount=`cat ${file} | wc -l`
+for ((i=1; i<=${linecount}; i++)); +do
[ snip ]
blobs=`awk '/^APPEND/ {print $2} /^IMAGE/ || /^DATA/ {print $3}' $file` for f in $blobs; do if [ ! -f $f ]; then echo "WARNING '$f' not found, resulting binary is not-functional" fi; done
will do the checks more efficiently.
Thanks,
Anatolij
-----Original Message----- From: Anatolij Gustschin [mailto:agust@denx.de] Sent: 2018年10月25日 7:44 To: Peng Fan peng.fan@nxp.com Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On Wed, 24 Oct 2018 09:49:04 +0000 Peng Fan peng.fan@nxp.com wrote: ...
--- /dev/null +++ b/tools/imx8_cntr_image.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ +# +# script to generate FIT image source for K3 Family boards with # +ATF, OPTEE, SPL and multiple device trees (given on the command line). +# Inspired from board/sunxi/mksunxi_fit_atf.sh
Please drop this comment, it doesn't describe what the script is actually doing.
Thanks. Fix in V2.
...
+file=$1
+linecount=`cat ${file} | wc -l`
+for ((i=1; i<=${linecount}; i++)); +do
[ snip ]
blobs=`awk '/^APPEND/ {print $2} /^IMAGE/ || /^DATA/ {print $3}' $file` for f in $blobs; do if [ ! -f $f ]; then echo "WARNING '$f' not found, resulting binary is not-functional" fi; done
will do the checks more efficiently.
Yes. Simpiler.
Thanks, Peng.
Thanks,
Anatolij
Hi Peng,
On 25/10/18 03:14, Peng Fan wrote:
-----Original Message----- From: Anatolij Gustschin [mailto:agust@denx.de] Sent: 2018年10月25日 7:44 To: Peng Fan peng.fan@nxp.com Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On Wed, 24 Oct 2018 09:49:04 +0000 Peng Fan peng.fan@nxp.com wrote: ...
--- /dev/null +++ b/tools/imx8_cntr_image.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ +# +# script to generate FIT image source for K3 Family boards with # +ATF, OPTEE, SPL and multiple device trees (given on the command line). +# Inspired from board/sunxi/mksunxi_fit_atf.sh
Please drop this comment, it doesn't describe what the script is actually doing.
Thanks. Fix in V2.
I wanted to send my PR as soon as possible to Tom. I didn't want to block i.MX8 merge just for this, and I merged V1 and sent PR.
Can you send then a follow-up patch instead of V2 ? V1 is on u-boot-imx, and Tom will merge the tree soon.
Best regards, Stefano
...
+file=$1
+linecount=`cat ${file} | wc -l`
+for ((i=1; i<=${linecount}; i++)); +do
[ snip ]
blobs=`awk '/^APPEND/ {print $2} /^IMAGE/ || /^DATA/ {print $3}' $file` for f in $blobs; do if [ ! -f $f ]; then echo "WARNING '$f' not found, resulting binary is not-functional" fi; done
will do the checks more efficiently.
Yes. Simpiler.
Thanks, Peng.
Thanks,
Anatolij
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2018年10月25日 16:26 To: Peng Fan peng.fan@nxp.com; Anatolij Gustschin agust@denx.de Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On 25/10/18 03:14, Peng Fan wrote:
-----Original Message----- From: Anatolij Gustschin [mailto:agust@denx.de] Sent: 2018年10月25日 7:44 To: Peng Fan peng.fan@nxp.com Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On Wed, 24 Oct 2018 09:49:04 +0000 Peng Fan peng.fan@nxp.com wrote: ...
--- /dev/null +++ b/tools/imx8_cntr_image.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ # # script to generate FIT +image source for K3 Family boards with # ATF, OPTEE, SPL and +multiple device trees (given on the command line). +# Inspired from board/sunxi/mksunxi_fit_atf.sh
Please drop this comment, it doesn't describe what the script is actually
doing.
Thanks. Fix in V2.
I wanted to send my PR as soon as possible to Tom. I didn't want to block i.MX8 merge just for this, and I merged V1 and sent PR.
No, please not use v1.
V2 patch will be out soon. It is under CI test. I'll send out now.
Thanks, Peng.
Can you send then a follow-up patch instead of V2 ? V1 is on u-boot-imx, and Tom will merge the tree soon.
Best regards, Stefano
...
+file=$1
+linecount=`cat ${file} | wc -l`
+for ((i=1; i<=${linecount}; i++)); +do
[ snip ]
blobs=`awk '/^APPEND/ {print $2} /^IMAGE/ || /^DATA/ {print $3}' $file` for f in $blobs; do if [ ! -f $f ]; then echo "WARNING '$f' not found, resulting binary is not-functional" fi; done
will do the checks more efficiently.
Yes. Simpiler.
Thanks, Peng.
Thanks,
Anatolij
--
===== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de ================================================================ =====
On 25/10/18 10:27, Peng Fan wrote:
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2018年10月25日 16:26 To: Peng Fan peng.fan@nxp.com; Anatolij Gustschin agust@denx.de Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On 25/10/18 03:14, Peng Fan wrote:
-----Original Message----- From: Anatolij Gustschin [mailto:agust@denx.de] Sent: 2018年10月25日 7:44 To: Peng Fan peng.fan@nxp.com Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On Wed, 24 Oct 2018 09:49:04 +0000 Peng Fan peng.fan@nxp.com wrote: ...
--- /dev/null +++ b/tools/imx8_cntr_image.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ # # script to generate FIT +image source for K3 Family boards with # ATF, OPTEE, SPL and +multiple device trees (given on the command line). +# Inspired from board/sunxi/mksunxi_fit_atf.sh
Please drop this comment, it doesn't describe what the script is actually
doing.
Thanks. Fix in V2.
I wanted to send my PR as soon as possible to Tom. I didn't want to block i.MX8 merge just for this, and I merged V1 and sent PR.
No, please not use v1.
V2 patch will be out soon. It is under CI test. I'll send out now.
Ok, I drop from server and I wait for it.
Thanks, Stefano
Thanks, Peng.
Can you send then a follow-up patch instead of V2 ? V1 is on u-boot-imx, and Tom will merge the tree soon.
Best regards, Stefano
...
+file=$1
+linecount=`cat ${file} | wc -l`
+for ((i=1; i<=${linecount}; i++)); +do
[ snip ]
blobs=`awk '/^APPEND/ {print $2} /^IMAGE/ || /^DATA/ {print $3}' $file` for f in $blobs; do if [ ! -f $f ]; then echo "WARNING '$f' not found, resulting binary is not-functional" fi; done
will do the checks more efficiently.
Yes. Simpiler.
Thanks, Peng.
Thanks,
Anatolij
--
===== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de ================================================================ =====
Hi Stefano,
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2018年10月25日 16:35 To: Peng Fan peng.fan@nxp.com; Stefano Babic sbabic@denx.de; Anatolij Gustschin agust@denx.de Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
On 25/10/18 10:27, Peng Fan wrote:
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2018年10月25日 16:26 To: Peng Fan peng.fan@nxp.com; Anatolij Gustschin agust@denx.de Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On 25/10/18 03:14, Peng Fan wrote:
-----Original Message----- From: Anatolij Gustschin [mailto:agust@denx.de] Sent: 2018年10月25日 7:44 To: Peng Fan peng.fan@nxp.com Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On Wed, 24 Oct 2018 09:49:04 +0000 Peng Fan peng.fan@nxp.com wrote: ...
--- /dev/null +++ b/tools/imx8_cntr_image.sh @@ -0,0 +1,32 @@ +#!/bin/sh +# SPDX-License-Identifier: GPL-2.0+ # # script to generate FIT +image source for K3 Family boards with # ATF, OPTEE, SPL and +multiple device trees (given on the command line). +# Inspired from board/sunxi/mksunxi_fit_atf.sh
Please drop this comment, it doesn't describe what the script is actually
doing.
Thanks. Fix in V2.
I wanted to send my PR as soon as possible to Tom. I didn't want to block i.MX8 merge just for this, and I merged V1 and sent PR.
No, please not use v1.
V2 patch will be out soon. It is under CI test. I'll send out now.
Ok, I drop from server and I wait for it.
V2 has been out. CI: https://travis-ci.org/MrVan/u-boot/builds/446043677 There is a build warning because of dts, I also send a follow up patch to fix dts build warning, but not kick a new CI build, because it is only i.mx8qxp related. In my local, with ahah image removed. ./tools/buildman/buildman imx8qxp_mek mx6sabresd Building current source for 2 boards (2 threads, 2 jobs per thread) 2 0 0 /2 mx6sabresd
I think it should be fine to be merged into your tree for the v2 and dts fix now.
Thanks, Peng.
Thanks, Stefano
Thanks, Peng.
Can you send then a follow-up patch instead of V2 ? V1 is on u-boot-imx, and Tom will merge the tree soon.
Best regards, Stefano
...
+file=$1
+linecount=`cat ${file} | wc -l`
+for ((i=1; i<=${linecount}; i++)); do
[ snip ]
blobs=`awk '/^APPEND/ {print $2} /^IMAGE/ || /^DATA/ {print $3}' $file` for f in $blobs; do if [ ! -f $f ]; then echo "WARNING '$f' not found, resulting binary is not-functional" fi; done
will do the checks more efficiently.
Yes. Simpiler.
Thanks, Peng.
Thanks,
Anatolij
--
================================================================
===== DENX Software Engineering GmbH, Managing Director: Wolfgang
Denk
HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de
================================================================
=====
--
===== DENX Software Engineering GmbH, Managing Director: Wolfgang Denk HRB 165235 Munich, Office: Kirchenstr.5, D-82194 Groebenzell, Germany Phone: +49-8142-66989-53 Fax: +49-8142-66989-80 Email: sbabic@denx.de ================================================================ =====
Hi Peng,
On 25/10/18 10:45, Peng Fan wrote:
Hi Stefano,
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2018年10月25日 16:35 To: Peng Fan peng.fan@nxp.com; Stefano Babic sbabic@denx.de; Anatolij Gustschin agust@denx.de Cc: u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
On 25/10/18 10:27, Peng Fan wrote:
-----Original Message----- From: Stefano Babic [mailto:sbabic@denx.de] Sent: 2018年10月25日 16:26 To: Peng Fan peng.fan@nxp.com; Anatolij Gustschin agust@denx.de Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On 25/10/18 03:14, Peng Fan wrote:
-----Original Message----- From: Anatolij Gustschin [mailto:agust@denx.de] Sent: 2018年10月25日 7:44 To: Peng Fan peng.fan@nxp.com Cc: sbabic@denx.de; u-boot@lists.denx.de Subject: Re: [U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists
Hi Peng,
On Wed, 24 Oct 2018 09:49:04 +0000 Peng Fan peng.fan@nxp.com wrote: ... > --- /dev/null > +++ b/tools/imx8_cntr_image.sh > @@ -0,0 +1,32 @@ > +#!/bin/sh > +# SPDX-License-Identifier: GPL-2.0+ # # script to generate FIT > +image source for K3 Family boards with # ATF, OPTEE, SPL and > +multiple device trees (given on the command line). > +# Inspired from board/sunxi/mksunxi_fit_atf.sh
Please drop this comment, it doesn't describe what the script is actually
doing.
Thanks. Fix in V2.
I wanted to send my PR as soon as possible to Tom. I didn't want to block i.MX8 merge just for this, and I merged V1 and sent PR.
No, please not use v1.
V2 patch will be out soon. It is under CI test. I'll send out now.
Ok, I drop from server and I wait for it.
V2 has been out. CI: https://travis-ci.org/MrVan/u-boot/builds/446043677 There is a build warning because of dts, I also send a follow up patch to fix dts build warning, but not kick a new CI build, because it is only i.mx8qxp related. In my local, with ahah image removed. ./tools/buildman/buildman imx8qxp_mek mx6sabresd Building current source for 2 boards (2 threads, 2 jobs per thread) 2 0 0 /2 mx6sabresd
I think it should be fine to be merged into your tree for the v2 and dts fix now.
Fully agree, I merged both, it is fine.
I prepare a new PR.
Regards, Stefano
participants (4)
-
Anatolij Gustschin -
Peng Fan -
Stefano Babic -
Wolfgang Denk