[libvirt PATCH] meson: Add yajl kludge

Andrea Bolognani posted 1 patch 2 years, 11 months ago
Test syntax-check failed
Patches applied successfully (tree, apply log)
git fetch https://github.com/patchew-project/libvirt tags/patchew/20210514092242.1239666-1-abologna@redhat.com
meson.build | 35 +++++++++++++++++++++++++++++++++++
1 file changed, 35 insertions(+)
[libvirt PATCH] meson: Add yajl kludge
Posted by Andrea Bolognani 2 years, 11 months ago
If this looks familiar, that's because it's literally *the
same code* that we used to work around *the same issue* in
readline before 1635dca26f61def3fbf56c70fbbfe514f2b50987 :)

Note that the issue only really affects people building from
source on Apple Silicon: on Intel, Homebrew installs header
files under directories that are part of the default search
path, which explains why our CI pipeline never ran into it.

Signed-off-by: Andrea Bolognani <abologna@redhat.com>
---
Roman, can you please test this? Thanks! :)

 meson.build | 35 +++++++++++++++++++++++++++++++++++
 1 file changed, 35 insertions(+)

diff --git a/meson.build b/meson.build
index 1f97842319..4f23f9104e 100644
--- a/meson.build
+++ b/meson.build
@@ -1318,6 +1318,41 @@ endif
 yajl_version = '2.0.3'
 yajl_dep = dependency('yajl', version: '>=' + yajl_version, required: get_option('yajl'))
 if yajl_dep.found()
+  # Kludge for yajl include path on non-Linux
+  #
+  # As of 2.1.0, upstream yajl.pc has -I${includedir}/yajl among
+  # its Cflags, which is clearly wrong. This does not affect Linux
+  # because ${includedir} is already part of the default include path,
+  # but on other platforms that's not the case and the result is that
+  # <yajl/yajl.h> can't be located, causing the build to fail.
+  #
+  # Since upstream development for yajl has stopped years ago, there's
+  # little hope of this issue getting fixed by a new upstream release.
+  # Some non-Linux operating systems such as FreeBSD have elected to
+  # carry a small downstream patch, but in the case of Homebrew on
+  # macOS this approach has been rejected[1] and so we're left with no
+  # choice but to work around the issue ourselves.
+  #
+  # [1] https://github.com/Homebrew/homebrew-core/pull/74516
+  if host_machine.system() != 'linux'
+    pkg_config_prog = find_program('pkg-config')
+    rc = run_command(pkg_config_prog, '--cflags', 'yajl', check: true)
+    cflags = rc.stdout().strip()
+    if cflags.contains('include/yajl')
+      rc = run_command(
+        'python3', '-c',
+        'print("@0@".replace("@1@", "@2@"))'.format(
+          cflags, 'include/yajl', 'include',
+        ),
+        check: true,
+      )
+      yajl_dep = declare_dependency(
+        compile_args: rc.stdout().strip().split(),
+        dependencies: [ yajl_dep ],
+      )
+    endif
+  endif
+
   conf.set('WITH_YAJL', 1)
 endif
 
-- 
2.26.3

Re: [libvirt PATCH] meson: Add yajl kludge
Posted by Ján Tomko 2 years, 11 months ago
On a Friday in 2021, Andrea Bolognani wrote:
>If this looks familiar, that's because it's literally *the
>same code* that we used to work around *the same issue* in
>readline before 1635dca26f61def3fbf56c70fbbfe514f2b50987 :)
>
>Note that the issue only really affects people building from
>source on Apple Silicon: on Intel, Homebrew installs header
>files under directories that are part of the default search
>path, which explains why our CI pipeline never ran into it.
>
>Signed-off-by: Andrea Bolognani <abologna@redhat.com>
>---
>Roman, can you please test this? Thanks! :)
>
> meson.build | 35 +++++++++++++++++++++++++++++++++++
> 1 file changed, 35 insertions(+)
>

Reviewed-by: Ján Tomko <jtomko@redhat.com>

(not tested, obviously)

Jano
Re: [libvirt PATCH] meson: Add yajl kludge
Posted by Roman Bolshakov 2 years, 11 months ago
On Fri, May 14, 2021 at 11:22:42AM +0200, Andrea Bolognani wrote:
> If this looks familiar, that's because it's literally *the
> same code* that we used to work around *the same issue* in
> readline before 1635dca26f61def3fbf56c70fbbfe514f2b50987 :)
> 
> Note that the issue only really affects people building from
> source on Apple Silicon: on Intel, Homebrew installs header
> files under directories that are part of the default search
> path, which explains why our CI pipeline never ran into it.
> 
> Signed-off-by: Andrea Bolognani <abologna@redhat.com>
> ---
> Roman, can you please test this? Thanks! :)
> 

Thanks Andrea!

It works fine :)

Reviewed-by: Roman Bolshakov <r.bolshakov@yadro.com>
Tested-by: Roman Bolshakov <r.bolshakov@yadro.com>

>  meson.build | 35 +++++++++++++++++++++++++++++++++++
>  1 file changed, 35 insertions(+)
> 
> diff --git a/meson.build b/meson.build
> index 1f97842319..4f23f9104e 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1318,6 +1318,41 @@ endif
>  yajl_version = '2.0.3'
>  yajl_dep = dependency('yajl', version: '>=' + yajl_version, required: get_option('yajl'))
>  if yajl_dep.found()
> +  # Kludge for yajl include path on non-Linux
> +  #
> +  # As of 2.1.0, upstream yajl.pc has -I${includedir}/yajl among
> +  # its Cflags, which is clearly wrong. This does not affect Linux
> +  # because ${includedir} is already part of the default include path,
> +  # but on other platforms that's not the case and the result is that
> +  # <yajl/yajl.h> can't be located, causing the build to fail.
> +  #
> +  # Since upstream development for yajl has stopped years ago, there's
> +  # little hope of this issue getting fixed by a new upstream release.
> +  # Some non-Linux operating systems such as FreeBSD have elected to
> +  # carry a small downstream patch, but in the case of Homebrew on
> +  # macOS this approach has been rejected[1] and so we're left with no
> +  # choice but to work around the issue ourselves.
> +  #
> +  # [1] https://github.com/Homebrew/homebrew-core/pull/74516
> +  if host_machine.system() != 'linux'
> +    pkg_config_prog = find_program('pkg-config')
> +    rc = run_command(pkg_config_prog, '--cflags', 'yajl', check: true)
> +    cflags = rc.stdout().strip()
> +    if cflags.contains('include/yajl')
> +      rc = run_command(
> +        'python3', '-c',
> +        'print("@0@".replace("@1@", "@2@"))'.format(
> +          cflags, 'include/yajl', 'include',
> +        ),
> +        check: true,
> +      )
> +      yajl_dep = declare_dependency(
> +        compile_args: rc.stdout().strip().split(),
> +        dependencies: [ yajl_dep ],
> +      )
> +    endif
> +  endif
> +
>    conf.set('WITH_YAJL', 1)
>  endif
>  
> -- 
> 2.26.3
>