On Fri, Feb 20, 2015 at 12:27:13PM -0500, Michael Smalley wrote:
> Reported-by: Michael J. Smalley <michaeljsmal...@gmail.com>
> Tested-by: Michael J. Smalley <michaeljsmal...@gmail.com>
>
> This logic was breaking on a CentOS 6.6 box with all dependencies (ovs-vsctl,
> docker, and uuidgen) installed and working. The check_command_avail function
> checks to see if the commands exist by running "$command --version". This is
> flawed logic because it assumes "--version" will work with all of the commands
> that are listed in the function call. It's much more common (and future proof)
> to simply check to see that the required binaries are available by running the
> "which" command. Also, I added a ! to make the control structure a little more
> terse.
>
> Evidence of the problem with checking --version:
> [root@docker ~]# uuidgen --version
> uuidgen: invalid option -- '-'
> Usage: uuidgen [-r] [-t]
> [root@docker ~]# echo $?
> 1
>
> ---
> utilities/ovs-docker | 5 +----
> 1 file changed, 1 insertion(+), 4 deletions(-)
>
> diff --git a/utilities/ovs-docker b/utilities/ovs-docker
> index 48908b1..d0b5f81 100755
> --- a/utilities/ovs-docker
> +++ b/utilities/ovs-docker
> @@ -15,10 +15,7 @@
>
> check_command_avail () {
> while [ $# -ne 0 ]; do
> - if ("$1" --version) > /dev/null 2>&1; then :; else
> - echo >&2 "$UTIL: missing $1, cannot proceed"
> - exit 1
> - fi
> + if ! $(which "$1" > /dev/null); then echo >&2 "$UTIL: missing
> $1, cannot proceed"; exit 1; fi
> shift
> done
> }
Thanks for the bug report, and the patch.
search_path from build-aux/dist-docs is probably more portable (it
doesn't depend on the "which" program or !).
# Check for programs we'll need.
search_path () {
save_IFS=$IFS
IFS=:
for dir in $PATH; do
IFS=$save_IFS
if test -x "$dir/$1"; then
return 0
fi
done
IFS=$save_IFS
echo >&2 "$0: $1 not found in \$PATH, please install and try again"
exit 1
}
search_path man
search_path markdown
search_path ps2pdf
In any case we'll need a Signed-off-by. Please see CONTRIBUTING.md
for its syntax and meaning.
Thanks,
Ben.
_______________________________________________
dev mailing list
dev@openvswitch.org
http://openvswitch.org/mailman/listinfo/dev
