[libvirt] [PATCHv2] build: restore support for libyajl 2.0.1

Ján Tomko posted 1 patch 4 years, 11 months ago
Test syntax-check passed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/05f6217e5eaf24723bf8dc43f3962f661cf2357a.1557410810.git.jtomko@redhat.com
m4/virt-yajl.m4 | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)
[libvirt] [PATCHv2] build: restore support for libyajl 2.0.1
Posted by Ján Tomko 4 years, 11 months ago
Commit 105756660f944e7db02de3b55b98bb7c11cd03bf was too eager and did
not consider SLE 12 which still has 2.0.1 that does not ship
a pkg-config file.

Similar to how we check for readline, prefer pkg-config if available
and fall back to the old detection code if not found.

NB: this is not a clean revert because we're not reintroducing support
for YAJL 1.

Signed-off-by: Ján Tomko <jtomko@redhat.com>
Reported-by: Olaf Hering <olaf@aepfle.de>
---
 m4/virt-yajl.m4 | 14 +++++++++++++-
 1 file changed, 13 insertions(+), 1 deletion(-)

diff --git a/m4/virt-yajl.m4 b/m4/virt-yajl.m4
index 494e722963..3caf0c5d07 100644
--- a/m4/virt-yajl.m4
+++ b/m4/virt-yajl.m4
@@ -24,7 +24,19 @@ AC_DEFUN([LIBVIRT_ARG_YAJL],[
 AC_DEFUN([LIBVIRT_CHECK_YAJL],[
   dnl YAJL JSON library http://lloyd.github.com/yajl/
 
-  LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3])
+  PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0])
+
+  if test $use_pkgconfig = 1; then
+    dnl 2.0.3 was the version where the pkg-config file was first added
+    LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3])
+  else
+    dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1
+    dnl TODO: delete this in July 2020
+    LIBVIRT_CHECK_LIB([YAJL], [yajl],
+                      [yajl_tree_parse], [yajl/yajl_common.h])
+
+  fi
+
 ])
 
 AC_DEFUN([LIBVIRT_RESULT_YAJL],[
-- 
2.19.2

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] build: restore support for libyajl 2.0.1
Posted by Andrea Bolognani 4 years, 11 months ago
On Thu, 2019-05-09 at 16:07 +0200, Ján Tomko wrote:
> Commit 105756660f944e7db02de3b55b98bb7c11cd03bf was too eager and did
> not consider SLE 12 which still has 2.0.1 that does not ship

"SLE" is no longer a thing, so either

  s/SLE/SUSE Linux Enterprise Server/

(preferred) or

  s/SLE/SLES/

[...]
> +  PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0])
> +
> +  if test $use_pkgconfig = 1; then
> +    dnl 2.0.3 was the version where the pkg-config file was first added
> +    LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3])
> +  else
> +    dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1

"SUSE SLE" was never a thing, so either

  s/SUSE SLE/SUSE Linux Enterprise Server/

(preferred) or

  s/SUSE SLE/SLES/

> +    dnl TODO: delete this in July 2020
> +    LIBVIRT_CHECK_LIB([YAJL], [yajl],
> +                      [yajl_tree_parse], [yajl/yajl_common.h])
> +
> +  fi
> +

Please drop the empty lines right before and right after the 'if'.

With the above nits addressed,

  Reviewed-by: Andrea Bolognani <abologna@redhat.com>

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] build: restore support for libyajl 2.0.1
Posted by Jim Fehlig 4 years, 11 months ago
On 5/9/19 9:18 AM, Andrea Bolognani wrote:
> On Thu, 2019-05-09 at 16:07 +0200, Ján Tomko wrote:
>> Commit 105756660f944e7db02de3b55b98bb7c11cd03bf was too eager and did
>> not consider SLE 12 which still has 2.0.1 that does not ship
> 
> "SLE" is no longer a thing, so either
> 
>    s/SLE/SUSE Linux Enterprise Server/
> 
> (preferred) or
> 
>    s/SLE/SLES/
> 
> [...]
>> +  PKG_CHECK_EXISTS([readline], [use_pkgconfig=1], [use_pkgconfig=0])
>> +
>> +  if test $use_pkgconfig = 1; then
>> +    dnl 2.0.3 was the version where the pkg-config file was first added
>> +    LIBVIRT_CHECK_PKG([YAJL], [yajl], [2.0.3])
>> +  else
>> +    dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1
> 
> "SUSE SLE" was never a thing, so either
> 
>    s/SUSE SLE/SUSE Linux Enterprise Server/

That's a lot of typing :-). I think SLES is fine. And to be super pedantic, 
s/OpenSUSE/openSUSE/.

Regards,
Jim

> 
> (preferred) or
> 
>    s/SUSE SLE/SLES/
> 
>> +    dnl TODO: delete this in July 2020
>> +    LIBVIRT_CHECK_LIB([YAJL], [yajl],
>> +                      [yajl_tree_parse], [yajl/yajl_common.h])
>> +
>> +  fi
>> +
> 
> Please drop the empty lines right before and right after the 'if'.
> 
> With the above nits addressed,
> 
>    Reviewed-by: Andrea Bolognani <abologna@redhat.com>
> 

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list
Re: [libvirt] [PATCHv2] build: restore support for libyajl 2.0.1
Posted by Andrea Bolognani 4 years, 11 months ago
On Thu, 2019-05-09 at 11:40 -0600, Jim Fehlig wrote:
> On 5/9/19 9:18 AM, Andrea Bolognani wrote:
> > On Thu, 2019-05-09 at 16:07 +0200, Ján Tomko wrote:
> > > +    dnl SUSE SLE 12 and OpenSUSE Leap 42.3 still use 2.0.1
> > 
> > "SUSE SLE" was never a thing, so either
> > 
> >    s/SUSE SLE/SUSE Linux Enterprise Server/
> 
> That's a lot of typing :-). I think SLES is fine. And to be super pedantic,
> s/OpenSUSE/openSUSE/.

I like super pendantic! :)

Jano, please fix that as well before pushing.

-- 
Andrea Bolognani / Red Hat / Virtualization

--
libvir-list mailing list
libvir-list@redhat.com
https://www.redhat.com/mailman/listinfo/libvir-list