[ovs-dev] [PATCH ovn v2 3/3] configure: Also find and verify version of ovsdb2ddlog.

Mark Michelson mmichels at redhat.com
Mon May 24 16:14:31 UTC 2021


On 5/21/21 11:02 PM, Ben Pfaff wrote:
> This tool is also needed and also varies from one version of DDlog to
> another, so we should find it and check its version in the same way.
> 
> Signed-off-by: Ben Pfaff <blp at ovn.org>
> ---
>   acinclude.m4       | 24 ++++++++++++++++--------
>   northd/automake.mk |  4 ++--
>   2 files changed, 18 insertions(+), 10 deletions(-)
> 
> diff --git a/acinclude.m4 b/acinclude.m4
> index 7009f1dd39c2..20c5ed02309e 100644
> --- a/acinclude.m4
> +++ b/acinclude.m4
> @@ -85,14 +85,22 @@ AC_DEFUN([OVS_CHECK_DDLOG], [
>           AC_MSG_ERROR([ddlog is required to build with DDlog])
>       fi
>   
> -    AC_MSG_CHECKING([$DDLOG version])
> -    $DDLOG --version >&AS_MESSAGE_LOG_FD 2>&1
> -    ddlog_version=$($DDLOG --version | sed -n 's/^DDlog v\([[^ ]]*\).*/\1/p')
> -    AC_MSG_RESULT([$ddlog_version])
> -    m4_if([$1], [], [], [
> -        AS_CASE([$ddlog_version],
> -            [$1 | $1.*], [],
> -            [*], [AC_MSG_ERROR([DDlog version $1.x is required, but $ddlog_version is installed])])])
> +    AC_ARG_VAR([OVSDB2DDLOG], [path to ovsdb2ddlog binary])
> +    AC_PATH_PROGS([OVSDB2DDLOG], [ovsdb2ddlog], [none], [$DDLOG_PATH])
> +    if test X"$OVSDB2DDLOG" = X"none"; then
> +        AC_MSG_ERROR([ovsdb2ddlog is required to build with DDlog])
> +    fi
> +
> +    for tool in "$DDLOG" "$OVSDB2DDLOG"; do
> +      AC_MSG_CHECKING([$tool version])
> +      $tool --version >&AS_MESSAGE_LOG_FD 2>&1
> +      tool_version=$($tool --version | sed -n 's/^.* v\([[^ ]]*\).*/\1/p')

(I should make this comment on patch 1, but I'm making it here so I can 
just send one email response for this patch series)

There is a small danger of this regex matching on something unexpected 
in case a DDLog tool ever has a word starting with a lowercase v in it. 
For example, a hypothetical "DDLog virtualization suite" would give us 
the version string "irtualization". It may be worthwhile to at least 
ensure the character directly after "v" is a number.

Maybe I'm just being overly nitpicky here though.

> +      AC_MSG_RESULT([$tool_version])
> +      m4_if([$1], [], [], [
> +          AS_CASE([$tool_version],
> +              [$1 | $1.*], [],
> +              [*], [AC_MSG_ERROR([DDlog version $1.x is required, but $tool is version $ddlog_version])])])

This error message should print $tool_version instead of $ddlog_version. 
$ddlog_version is no longer set, so this just always prints an empty 
string for the version if you have the wrong one installed.

> +    done
>   
>       AC_ARG_VAR([CARGO])
>       AC_CHECK_PROGS([CARGO], [cargo], [none])
> diff --git a/northd/automake.mk b/northd/automake.mk
> index aaea7e1b1336..4fc81c17bfa3 100644
> --- a/northd/automake.mk
> +++ b/northd/automake.mk
> @@ -50,13 +50,13 @@ northd_ovn_northd_ddlog_LDADD = \
>   
>   nb_opts = $$(cat $(srcdir)/northd/ovn-nb.dlopts)
>   northd/OVN_Northbound.dl: ovn-nb.ovsschema northd/ovn-nb.dlopts
> -	$(AM_V_GEN)ovsdb2ddlog -f $< --output-file $@ $(nb_opts)
> +	$(AM_V_GEN)$(OVSDB2DDLOG) -f $< --output-file $@ $(nb_opts)
>   northd/ovn-northd-ddlog-nb.inc: ovn-nb.ovsschema northd/ovn-nb.dlopts northd/ovsdb2ddlog2c
>   	$(AM_V_GEN)$(run_python) $(srcdir)/northd/ovsdb2ddlog2c -p nb_ -f $< --output-file $@ $(nb_opts)
>   
>   sb_opts = $$(cat $(srcdir)/northd/ovn-sb.dlopts)
>   northd/OVN_Southbound.dl: ovn-sb.ovsschema northd/ovn-sb.dlopts
> -	$(AM_V_GEN)ovsdb2ddlog -f $< --output-file $@ $(sb_opts)
> +	$(AM_V_GEN)$(OVSDB2DDLOG) -f $< --output-file $@ $(sb_opts)
>   northd/ovn-northd-ddlog-sb.inc: ovn-sb.ovsschema northd/ovn-sb.dlopts northd/ovsdb2ddlog2c
>   	$(AM_V_GEN)$(run_python) $(srcdir)/northd/ovsdb2ddlog2c -p sb_ -f $< --output-file $@ $(sb_opts)
>   
> 



More information about the dev mailing list