[v1] configure: Improve zstd test

[PATCH] configure: Improve zstd test

Posted by Juan Quintela 5 years, 8 months ago

There were one error on the test (missing an s for --exists).
But we really need a recent zstd (0.8.1).
That version was released in 2016, so it is newer that some of our travis
images.  Just check for the version that we need.

Signed-off-by: Juan Quintela <quintela@redhat.com>
Reported-by: Richard Henderson <richard.henderson@linaro.org>
---
 configure | 3 ++-
 1 file changed, 2 insertions(+), 1 deletion(-)

diff --git a/configure b/configure
index 7b373bc0bb..1bf48df1ef 100755
--- a/configure
+++ b/configure
@@ -2464,7 +2464,8 @@ fi
 # zstd check
 
 if test "$zstd" != "no" ; then
-    if $pkg_config --exist libzstd ; then
+    libzstd_minver="0.8.1"
+    if $pkg_config --atleast-version=$libzstd_minver libzstd ; then
         zstd_cflags="$($pkg_config --cflags libzstd)"
         zstd_libs="$($pkg_config --libs libzstd)"
         LIBS="$zstd_libs $LIBS"
-- 
2.24.1

Re: [PATCH] configure: Improve zstd test

Posted by Alex Bennée 5 years, 8 months ago

Juan Quintela <quintela@redhat.com> writes:

> There were one error on the test (missing an s for --exists).
> But we really need a recent zstd (0.8.1).
> That version was released in 2016, so it is newer that some of our travis
> images.  Just check for the version that we need.
>
> Signed-off-by: Juan Quintela <quintela@redhat.com>
> Reported-by: Richard Henderson <richard.henderson@linaro.org>
> ---
>  configure | 3 ++-
>  1 file changed, 2 insertions(+), 1 deletion(-)
>
> diff --git a/configure b/configure
> index 7b373bc0bb..1bf48df1ef 100755
> --- a/configure
> +++ b/configure
> @@ -2464,7 +2464,8 @@ fi
>  # zstd check
>  
>  if test "$zstd" != "no" ; then
> -    if $pkg_config --exist libzstd ; then
> +    libzstd_minver="0.8.1"
> +    if $pkg_config --atleast-version=$libzstd_minver libzstd ; then
>          zstd_cflags="$($pkg_config --cflags libzstd)"
>          zstd_libs="$($pkg_config --libs libzstd)"
>          LIBS="$zstd_libs $LIBS"

Hmm still breaks with:

   make docker-test-build@ubuntu J=9 V=1

With:

  FY_SOURCE=2 -g   -c -o monitor/qmp.o /tmp/qemu-test/src/monitor/qmp.c
  /tmp/qemu-test/src/migration/multifd-zstd.c: In function 'zstd_send_prepare':
  /tmp/qemu-test/src/migration/multifd-zstd.c:125:9: error: unknown type name 'ZSTD_EndDirective'; did you mean 'ZSTD_DDict'?
           ZSTD_EndDirective flush = ZSTD_e_continue;
           ^~~~~~~~~~~~~~~~~
           ZSTD_DDict
  /tmp/qemu-test/src/migration/multifd-zstd.c:125:35: error: 'ZSTD_e_continue' undeclared (first use in this function); did you mean 'g_timer_continue'?
           ZSTD_EndDirective flush = ZSTD_e_continue;
                                     ^~~~~~~~~~~~~~~
                                     g_timer_continue
  /tmp/qemu-test/src/migration/multifd-zstd.c:125:35: note: each undeclared identifier is reported only once for each function it appears in
  /tmp/qemu-test/src/migration/multifd-zstd.c:128:21: error: 'ZSTD_e_flush' undeclared (first use in this function); did you mean 'ZSTD_DCtx_s'?
               flush = ZSTD_e_flush;
                       ^~~~~~~~~~~~
                       ZSTD_DCtx_s
  /tmp/qemu-test/src/migration/multifd-zstd.c:143:19: error: implicit declaration of function 'ZSTD_compressStream2'; did you mean 'ZSTD_compressStream'? [-Werror=implicit-fun
  ction-declaration]
               ret = ZSTD_compressStream2(z->zcs, &z->out, &z->in, flush);
                     ^~~~~~~~~~~~~~~~~~~~
                     ZSTD_compressStream
  /tmp/qemu-test/src/migration/multifd-zstd.c:143:19: error: nested extern declaration of 'ZSTD_compressStream2' [-Werror=nested-externs]
  cc1: all warnings being treated as errors
  make: *** [/tmp/qemu-test/src/rules.mak:69: migration/multifd-zstd.o] Error 1
  make: *** Waiting for unfinished jobs....
  rm tests/qemu-iotests/socket_scm_helper.o

Running manually you get:

  14:48:20 [alex@zen:~/l/q/b/all] review/misc-bits|… 1 + docker run --rm -it qemu:ubuntu pkg-config --exists libzstd
  14:48:24 [alex@zen:~/l/q/b/all] review/misc-bits|… + echo $status
  0
  14:48:28 [alex@zen:~/l/q/b/all] review/misc-bits|… + docker run --rm -it qemu:ubuntu pkg-config --atleast-version="0.8.1" libzstd
  14:48:52 [alex@zen:~/l/q/b/all] review/misc-bits|… + echo $status
  0
  14:49:33 [alex@zen:~/l/q/b/all] review/misc-bits|… + docker run --rm -it qemu:ubuntu pkg-config --cflags libzstd

  14:49:45 [alex@zen:~/l/q/b/all] review/misc-bits|… + docker run --rm -it qemu:ubuntu pkg-config --libs libzstd
  -lzstd
  14:51:14 [alex@zen:~/l/q/b/all] review/misc-bits|… + docker run --rm -it qemu:ubuntu dpkg --status libzstd-dev
  Package: libzstd-dev
  Status: install ok installed
  Priority: optional
  Section: libdevel
  Installed-Size: 985
  Maintainer: Ubuntu Developers <ubuntu-devel-discuss@lists.ubuntu.com>
  Architecture: amd64
  Multi-Arch: same
  Source: libzstd
  Version: 1.3.8+dfsg-3
  Depends: libzstd1 (= 1.3.8+dfsg-3)
  Description: fast lossless compression algorithm -- development files
   Zstd, short for Zstandard, is a fast lossless compression algorithm, targeting
   real-time compression scenarios at zlib-level compression ratio.
   .
   This package contains the headers and static library.
  Original-Maintainer: Debian Med Packaging Team <debian-med-packaging@lists.alioth.debian.org>
  Homepage: https://github.com/facebook/zstd


-- 
Alex Bennée

Re: [PATCH] configure: Improve zstd test

Posted by Juan Quintela 5 years, 8 months ago

Alex Bennée <alex.bennee@linaro.org> wrote:
> Juan Quintela <quintela@redhat.com> writes:
>
>> There were one error on the test (missing an s for --exists).
>> But we really need a recent zstd (0.8.1).
>> That version was released in 2016, so it is newer that some of our travis
>> images.  Just check for the version that we need.
>>
>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>> Reported-by: Richard Henderson <richard.henderson@linaro.org>
>> ---
>>  configure | 3 ++-
>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>
>> diff --git a/configure b/configure
>> index 7b373bc0bb..1bf48df1ef 100755
>> --- a/configure
>> +++ b/configure
>> @@ -2464,7 +2464,8 @@ fi
>>  # zstd check
>>  
>>  if test "$zstd" != "no" ; then
>> -    if $pkg_config --exist libzstd ; then
>> +    libzstd_minver="0.8.1"
>> +    if $pkg_config --atleast-version=$libzstd_minver libzstd ; then
>>          zstd_cflags="$($pkg_config --cflags libzstd)"
>>          zstd_libs="$($pkg_config --libs libzstd)"
>>          LIBS="$zstd_libs $LIBS"
>
> Hmm still breaks with:
>
>    make docker-test-build@ubuntu J=9 V=1

Thanks.

> With:
>
>   FY_SOURCE=2 -g   -c -o monitor/qmp.o /tmp/qemu-test/src/monitor/qmp.c
>   /tmp/qemu-test/src/migration/multifd-zstd.c: In function 'zstd_send_prepare':
>   /tmp/qemu-test/src/migration/multifd-zstd.c:125:9: error: unknown type name 'ZSTD_EndDirective'; did you mean 'ZSTD_DDict'?
>            ZSTD_EndDirective flush = ZSTD_e_continue;
>            ^~~~~~~~~~~~~~~~~

Greate, more things were introduced later.
As it would be too easy, the zstd repository is not lineal, you need to
checkout the tag you want to see when something has been introduced.

Will try to get this fixed.

Sorry for the inconveniences.


>   Version: 1.3.8+dfsg-3
>   Depends: libzstd1 (= 1.3.8+dfsg-3)
>   Description: fast lossless compression algorithm -- development files

I don't undertsand now.

ZSTD_EndDirective was included in 1.3.0.

I can just change that for 1.3.9, but I don't know why is that there.
Could you do a grep ZSTD_EndDirective /usr/lib/zstd.h?

Or if there is some documentation that shows how to use that docker
images.

Later, Juan.

Re: [PATCH] configure: Improve zstd test

Posted by Michal Prívozník 5 years, 8 months ago

On 5. 3. 2020 16:27, Juan Quintela wrote:
> Alex Bennée <alex.bennee@linaro.org> wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>>> There were one error on the test (missing an s for --exists).
>>> But we really need a recent zstd (0.8.1).
>>> That version was released in 2016, so it is newer that some of our travis
>>> images.  Just check for the version that we need.
>>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>> Reported-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>  configure | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index 7b373bc0bb..1bf48df1ef 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2464,7 +2464,8 @@ fi
>>>  # zstd check
>>>  
>>>  if test "$zstd" != "no" ; then
>>> -    if $pkg_config --exist libzstd ; then
>>> +    libzstd_minver="0.8.1"
>>> +    if $pkg_config --atleast-version=$libzstd_minver libzstd ; then
>>>          zstd_cflags="$($pkg_config --cflags libzstd)"
>>>          zstd_libs="$($pkg_config --libs libzstd)"
>>>          LIBS="$zstd_libs $LIBS"
>>
>> Hmm still breaks with:
>>
>>    make docker-test-build@ubuntu J=9 V=1
> 
> Thanks.
> 
>> With:
>>
>>   FY_SOURCE=2 -g   -c -o monitor/qmp.o /tmp/qemu-test/src/monitor/qmp.c
>>   /tmp/qemu-test/src/migration/multifd-zstd.c: In function 'zstd_send_prepare':
>>   /tmp/qemu-test/src/migration/multifd-zstd.c:125:9: error: unknown type name 'ZSTD_EndDirective'; did you mean 'ZSTD_DDict'?
>>            ZSTD_EndDirective flush = ZSTD_e_continue;
>>            ^~~~~~~~~~~~~~~~~
> 
> Greate, more things were introduced later.
> As it would be too easy, the zstd repository is not lineal, you need to
> checkout the tag you want to see when something has been introduced.
> 
> Will try to get this fixed.
> 
> Sorry for the inconveniences.
> 
> 
>>   Version: 1.3.8+dfsg-3
>>   Depends: libzstd1 (= 1.3.8+dfsg-3)
>>   Description: fast lossless compression algorithm -- development files
> 
> I don't undertsand now.
> 
> ZSTD_EndDirective was included in 1.3.0.
> 
> I can just change that for 1.3.9, but I don't know why is that there.
> Could you do a grep ZSTD_EndDirective /usr/lib/zstd.h?

Thing is, they have so called experimental APIs. You get them only if
you define ZSTD_STATIC_LINKING_ONLY before including zstd.h. So the
plain grep of a symbol tells us nothing. We need to check if it's not in
#ifdef. Looks like 1.3.9 is the minimal version which has everything we
want.

Michal

Re: [PATCH] configure: Improve zstd test

Posted by Juan Quintela 5 years, 8 months ago

Michal Prívozník <mprivozn@redhat.com> wrote:
> On 5. 3. 2020 16:27, Juan Quintela wrote:
>> Alex Bennée <alex.bennee@linaro.org> wrote:
>>> Juan Quintela <quintela@redhat.com> writes:
>>>
>>>> There were one error on the test (missing an s for --exists).
>>>> But we really need a recent zstd (0.8.1).
>>>> That version was released in 2016, so it is newer that some of our travis
>>>> images.  Just check for the version that we need.
>>>>
>>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>>> Reported-by: Richard Henderson <richard.henderson@linaro.org>
>>>> ---
>>>>  configure | 3 ++-
>>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>>
>>>> diff --git a/configure b/configure
>>>> index 7b373bc0bb..1bf48df1ef 100755
>>>> --- a/configure
>>>> +++ b/configure
>>>> @@ -2464,7 +2464,8 @@ fi
>>>>  # zstd check
>>>>  
>>>>  if test "$zstd" != "no" ; then
>>>> -    if $pkg_config --exist libzstd ; then
>>>> +    libzstd_minver="0.8.1"
>>>> +    if $pkg_config --atleast-version=$libzstd_minver libzstd ; then
>>>>          zstd_cflags="$($pkg_config --cflags libzstd)"
>>>>          zstd_libs="$($pkg_config --libs libzstd)"
>>>>          LIBS="$zstd_libs $LIBS"
>>>
>>> Hmm still breaks with:
>>>
>>>    make docker-test-build@ubuntu J=9 V=1
>> 
>> Thanks.
>> 
>>> With:
>>>
>>>   FY_SOURCE=2 -g   -c -o monitor/qmp.o /tmp/qemu-test/src/monitor/qmp.c
>>>   /tmp/qemu-test/src/migration/multifd-zstd.c: In function 'zstd_send_prepare':
>>>   /tmp/qemu-test/src/migration/multifd-zstd.c:125:9: error: unknown type name 'ZSTD_EndDirective'; did you mean 'ZSTD_DDict'?
>>>            ZSTD_EndDirective flush = ZSTD_e_continue;
>>>            ^~~~~~~~~~~~~~~~~
>> 
>> Greate, more things were introduced later.
>> As it would be too easy, the zstd repository is not lineal, you need to
>> checkout the tag you want to see when something has been introduced.
>> 
>> Will try to get this fixed.
>> 
>> Sorry for the inconveniences.
>> 
>> 
>>>   Version: 1.3.8+dfsg-3
>>>   Depends: libzstd1 (= 1.3.8+dfsg-3)
>>>   Description: fast lossless compression algorithm -- development files
>> 
>> I don't undertsand now.
>> 
>> ZSTD_EndDirective was included in 1.3.0.
>> 
>> I can just change that for 1.3.9, but I don't know why is that there.
>> Could you do a grep ZSTD_EndDirective /usr/lib/zstd.h?
>
> Thing is, they have so called experimental APIs. You get them only if
> you define ZSTD_STATIC_LINKING_ONLY before including zstd.h. So the
> plain grep of a symbol tells us nothing. We need to check if it's not in
> #ifdef. Looks like 1.3.9 is the minimal version which has everything we
> want.

So easy!!!!!

I was arriving to that conclusion after brute force.

Later, Juan.

Re: [PATCH] configure: Improve zstd test

Posted by Alex Bennée 5 years, 8 months ago

Juan Quintela <quintela@redhat.com> writes:

> Alex Bennée <alex.bennee@linaro.org> wrote:
>> Juan Quintela <quintela@redhat.com> writes:
>>
>>> There were one error on the test (missing an s for --exists).
>>> But we really need a recent zstd (0.8.1).
>>> That version was released in 2016, so it is newer that some of our travis
>>> images.  Just check for the version that we need.
>>>
>>> Signed-off-by: Juan Quintela <quintela@redhat.com>
>>> Reported-by: Richard Henderson <richard.henderson@linaro.org>
>>> ---
>>>  configure | 3 ++-
>>>  1 file changed, 2 insertions(+), 1 deletion(-)
>>>
>>> diff --git a/configure b/configure
>>> index 7b373bc0bb..1bf48df1ef 100755
>>> --- a/configure
>>> +++ b/configure
>>> @@ -2464,7 +2464,8 @@ fi
>>>  # zstd check
>>>  
>>>  if test "$zstd" != "no" ; then
>>> -    if $pkg_config --exist libzstd ; then
>>> +    libzstd_minver="0.8.1"
>>> +    if $pkg_config --atleast-version=$libzstd_minver libzstd ; then
>>>          zstd_cflags="$($pkg_config --cflags libzstd)"
>>>          zstd_libs="$($pkg_config --libs libzstd)"
>>>          LIBS="$zstd_libs $LIBS"
>>
>> Hmm still breaks with:
>>
>>    make docker-test-build@ubuntu J=9 V=1
>
> Thanks.
>
>> With:
>>
>>   FY_SOURCE=2 -g   -c -o monitor/qmp.o /tmp/qemu-test/src/monitor/qmp.c
>>   /tmp/qemu-test/src/migration/multifd-zstd.c: In function 'zstd_send_prepare':
>>   /tmp/qemu-test/src/migration/multifd-zstd.c:125:9: error: unknown type name 'ZSTD_EndDirective'; did you mean 'ZSTD_DDict'?
>>            ZSTD_EndDirective flush = ZSTD_e_continue;
>>            ^~~~~~~~~~~~~~~~~
>
> Greate, more things were introduced later.
> As it would be too easy, the zstd repository is not lineal, you need to
> checkout the tag you want to see when something has been introduced.
>
> Will try to get this fixed.
>
> Sorry for the inconveniences.
>
>
>>   Version: 1.3.8+dfsg-3
>>   Depends: libzstd1 (= 1.3.8+dfsg-3)
>>   Description: fast lossless compression algorithm -- development files
>
> I don't undertsand now.
>
> ZSTD_EndDirective was included in 1.3.0.
>
> I can just change that for 1.3.9, but I don't know why is that there.
> Could you do a grep ZSTD_EndDirective /usr/lib/zstd.h?

23:07:26 [alex@zen:~/l/q/b/all] review/misc-bits|… 2 + docker run --rm -it qemu:ubuntu grep -r "ZSTD_End" /usr/include/
/usr/include/zstd.h:} ZSTD_EndDirective;
/usr/include/zstd.h:                                         ZSTD_EndDirective endOp);
/usr/include/zstd.h:                            ZSTD_EndDirective endOp);

>
> Or if there is some documentation that shows how to use that docker
> images.

Setting up docker is described in docs/devel/testing.rst - once setup
the make invocations I listed work. The final images are all tagged
qemu:FOO so for ubuntu the general form:

  docker run --rm -it qemu:ubuntu COMMAND

Will do what you want.

>
> Later, Juan.


-- 
Alex Bennée