[Fuego] [PATCH] Optimize the code of assert_has_program.

[Tim.Bird at sony.com]

I'm not sure this gives the same results at the original code.
See my comments inline below.

Also, combining multiple programs into a single 'assert_has_program'
lines will make it a bit more difficult to implement a cache system
for programs detected on the board.

I will have to think about this one.

> -----Original Message-----
> From: Wang Mingyu on Tuesday, September 03, 2019 3:09 PM
> 
> It is faster to check the prgoram or file on the target.
prgoram -> program

> 
> Usage:
> before:
>     assert_has_program AAA
>     assert_has_program BBB
> 
> after:
>     assert_has_program "AAA BBB"
>     assert_has_program "AAA BBB" SEARCH_PATH
> 
> Note: SEARCH_PATH is optional.

Can you explain when SEARCH_PATH is needed?

> 
> Signed-off-by: Wang Mingyu <wangmy at cn.fujitsu.com>
> ---
>  scripts/functions.sh  | 65
> +++++++++++++++++++++++++++++++++++++++----
>  scripts/need_check.sh | 11 ++------
>  2 files changed, 61 insertions(+), 15 deletions(-)
> 
> diff --git a/scripts/functions.sh b/scripts/functions.sh
> index 0fa80b8..3aea0c8 100755
> --- a/scripts/functions.sh
> +++ b/scripts/functions.sh
> @@ -1010,6 +1010,57 @@ function is_on_target_path {
>      is_on_target $1 $2 $TARGET_PATH
>  }
> 
> +# check for a program or file on the target, and set a variable if it's present
> +# $1 - file, dir or program on target
> +# $2 is the path of target to search the program/file
> +# $3 - tmpfile to save the detect result
> +function set_cmd_str {
In general, I like this approach of executing a command on the
target that does the search loop.

> +   tmpfile=$3
> +   cmd_str="touch $tmpfile
> +         # split $1 on whitespace, without file globbing
> +         for prg in \$(echo $1 | tr \" \" \"\\n\") ; do
> +             # split search path on colon
> +             for d in \$(echo $2 | tr \":\" \"\\n\") ; do
> +                 # execute a command on the target to detect \$d/\$prg
> +                 if [ -z \"\$(cat $tmpfile)\" -a -e \"\$d/\$prg\" ]
> +                 then
> +                     echo \"\$d/\$prg\" >$tmpfile ;break;
> +                 fi
> +             done
> +             if [ ! -s $tmpfile ]
> +             then
> +                 find / -name \"\$prg\" | head -n 1 >$tmpfile
> +             fi
This will detect the program anywhere in the filesystem, not just on the PATH.
That changes the behavior of assert_has_program, which currently only
looks on the target's PATH.

> +             if [ ! -s $tmpfile ]
> +             then
> +                 echo \"\$prg\" >$tmpfile ; break;
This would put the program name into tmpfile, which means that even
if the program was not detected, tmpfile will have a non-empty result.
I don't understand this.

> +             else
> +                 > $tmpfile
I also don't understand why this is needed.  The file is always touched
above, so it exists as an empty file.  Why does it need to have
the empty redirection into it?

> +             fi
> +         done"
> +}
> +
> +# check for a program or file on the target, and set a variable if it's present
> +# $1 - file, dir or program on target
> +# $2 - variable to set during the build
> +# $3 is the path of target to search the program/file (optional, default:
> $PATH)
> +function assert_on_target {
> +    local TARGET_PATH=${3}
> +    if [ -z "$TARGET_PATH" ]; then
> +       TARGET_PATH=$(cmd "echo \$PATH")
> +    fi
> +
> +    tmpfile=$(mktemp /tmp/found_loc.XXXXXX)
> +    set_cmd_str "$1" $TARGET_PATH $tmpfile
> +    cmd "$cmd_str"
> +    get $tmpfile $tmpfile
> +    if [ -s $tmpfile ] ; then
> +        export $2=$(cat $tmpfile)
> +    fi
> +    cmd "rm $tmpfile"
> +    rm -f $tmpfile # -f for tests running on the host
> +}
> +
>  # check for a library on the SDK, and set a variable if it's present
>  # $1 - the file (library) we want to search for in the SDK
>  # $2 - variable to set the location (if the file is found)
> @@ -1036,13 +1087,15 @@ function is_on_sdk {
> 
>  # check for a program or file on the target, and send message if the program
> or file is missing
>  # $1 has the program that is required on the target board
> -# this has the side effect of defining PROGRAM_$1 (uppercased)
> -# with the value as the directory where $1 is found on the board.
> +# $2 is the path of target to search the program/file (optional, default:
> $PATH)
>  function assert_has_program {
> -   upName=${1^^}
> -   progVar=PROGRAM_${upName//[-,.]/_}
> -   is_on_target_path $1 ${progVar}
> -   assert_define ${progVar} "Missing '$1' program on target board"
> +   assert_on_target "$1" prgName $2
> +   if [ -n "$prgName" ]
> +   then
> +       upName=${prgName^^}
> +       progVar=PROGRAM_${upName//[-,.]/_}
> +       assert_define ${progVar} "Missing '$prgName' program on target board"
> +   fi
>  }
> 
>  # check for a module on the target, and abort if it is missing
> diff --git a/scripts/need_check.sh b/scripts/need_check.sh
> index c9bbb75..9bb0098 100755
> --- a/scripts/need_check.sh
> +++ b/scripts/need_check.sh
> @@ -319,18 +319,11 @@ function check_root {
>  # check if those specified commands exist on board
>  # $1 has single string with a list of command entries to check for
>  function check_program {
> -  # split $1 on whitespace, without file globbing
> -  set -f
> -  arg_array=($1)
> -  set +f
> -
> -  for prg in "${arg_array[@]}" ; do
> -    is_on_target_path $prg prg_path
> -    if [ -z "$prg_path" ] ; then
> +    assert_on_target "$1" prg
> +    if [ -n "$prg" ] ; then
>        echo -e "\n\nABORTED: Expected command \"$prg\" on the target, but
> it's not there!"
>        return 1
>      fi
> -  done
> 
>    # return OK if all necessary commands exist on the target
>    return 0
> --
> 2.17.1

I'm not sure this does the same thing as the original assert_has_program.

I can't tell what happens if I provide the name of a file which isn't on
the path, but is in the filesystem, or which isn't in the filesystem at all.
Did you test for these cases?
 -- Tim