[U-Boot] [PATCH] imx: mkimage: avoid stop CI when required files not exists

[Peng Fan <peng.fan@nxp.com>]

> -----Original Message-----
> From: Wolfgang Denk [mailto:wd at denx.de]
> Sent: 2018年10月24日 22:09
> To: Peng Fan <peng.fan@nxp.com>
> Cc: sbabic at denx.de; u-boot at 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}" ]; then
> > +			echo "WARNING ${name} not found, resulting binary is
> not-functional" >&2
> > +			exit 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.

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 at denx.de It is
> better to marry than to burn.
>                                 - Bible ``I Corinthians'' ch. 7, v. 9