[v1] Fix linkage to libutil and libkvm on FreeBSD 11

[PATCH] Fix linkage to libutil and libkvm on FreeBSD 11

Posted by Daniel P. Berrangé 3 years, 7 months ago

We are currently adding -lutil and -lkvm to the linker using the
add_project_link_arguments method. On FreeBSD 11.4, this results in
build errors because the args appear too early in the command line.

We need to pass the libraries as dependencies so that they get placed
at the same point in the linker args as other dependencies.

Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
---
 meson.build           | 16 +++++++---------
 src/bhyve/meson.build | 15 +++++++++++----
 src/util/meson.build  | 26 +++++++++++++++++---------
 3 files changed, 35 insertions(+), 22 deletions(-)

Using the CI patch I posted earlier to add FreeBSD 11:

  https://gitlab.com/berrange/libvirt/-/pipelines/185834799


diff --git a/meson.build b/meson.build
index 1eadea33bf..c30ff187aa 100644
--- a/meson.build
+++ b/meson.build
@@ -1086,7 +1086,8 @@ endif
 # Check for BSD kvm (kernel memory interface)
 if host_machine.system() == 'freebsd'
   kvm_dep = cc.find_library('kvm')
-  add_project_link_arguments('-lkvm', language: 'c')
+else
+  kvm_dep = disabler()
 endif
 
 libiscsi_version = '1.18.0'
@@ -1203,11 +1204,9 @@ have_gnu_gettext_tools = false
 if not get_option('nls').disabled()
   have_gettext = cc.has_function('gettext')
   if not have_gettext
-    intl_lib = cc.find_library('intl', required: false)
-    have_gettext = intl_lib.found()
-    if have_gettext
-      add_project_link_arguments('-lintl', language: 'c')
-    endif
+    intl_dep = cc.find_library('intl', required: false)
+  else
+    intl_dep = disabler()
   endif
   if not have_gettext and get_option('nls').enabled()
     error('gettext() is required to build libvirt')
@@ -1235,6 +1234,8 @@ if not get_option('nls').disabled()
       have_gnu_gettext_tools = true
     endif
   endif
+else
+  intl_dep = disabler()
 endif
 
 numactl_dep = cc.find_library('numa', required: get_option('numactl'))
@@ -1402,9 +1403,6 @@ if udev_dep.found()
 endif
 
 util_dep = cc.find_library('util', required: false)
-if util_dep.found()
-  add_project_link_arguments('-lutil', language: 'c')
-endif
 
 if not get_option('virtualport').disabled()
   if cc.has_header_symbol('linux/if_link.h', 'IFLA_PORT_MAX')
diff --git a/src/bhyve/meson.build b/src/bhyve/meson.build
index 7d54718820..c382f64aee 100644
--- a/src/bhyve/meson.build
+++ b/src/bhyve/meson.build
@@ -14,15 +14,22 @@ driver_source_files += bhyve_sources
 stateful_driver_source_files += bhyve_sources
 
 if conf.has('WITH_BHYVE')
+  bhyve_driver_deps = [
+    access_dep,
+    src_dep,
+  ]
+  if kvm_dep.found()
+    bhyve_driver_deps += kvm_dep
+  endif
+  if util_dep.found()
+    bhyve_driver_deps += util_dep
+  endif
   bhyve_driver_impl = static_library(
     'virt_driver_bhyve_impl',
     [
       bhyve_sources,
     ],
-    dependencies: [
-      access_dep,
-      src_dep,
-    ],
+    dependencies: bhyve_driver_deps,
     include_directories: [
       conf_inc_dir,
       hypervisor_inc_dir,
diff --git a/src/util/meson.build b/src/util/meson.build
index f7092cc3f1..c899f232e6 100644
--- a/src/util/meson.build
+++ b/src/util/meson.build
@@ -172,15 +172,7 @@ io_helper_sources = [
   'iohelper.c',
 ]
 
-virt_util_lib = static_library(
-  'virt_util',
-  [
-    util_sources,
-    util_public_sources,
-    keycode_gen_sources,
-    dtrace_gen_headers,
-  ],
-  dependencies: [
+virt_util_deps = [
     acl_dep,
     audit_dep,
     capng_dep,
@@ -195,7 +187,23 @@ virt_util_lib = static_library(
     thread_dep,
     win32_dep,
     yajl_dep,
+  ]
+if util_dep.found()
+  virt_util_deps += util_dep
+endif
+if intl_dep.found()
+  virt_util_deps += intl_dep
+endif
+
+virt_util_lib = static_library(
+  'virt_util',
+  [
+    util_sources,
+    util_public_sources,
+    keycode_gen_sources,
+    dtrace_gen_headers,
   ],
+  dependencies: virt_util_deps,
 )
 
 libvirt_libs += virt_util_lib
-- 
2.26.2

Re: [PATCH] Fix linkage to libutil and libkvm on FreeBSD 11

Posted by Ján Tomko 3 years, 7 months ago

On a Friday in 2020, Daniel P. Berrangé wrote:
>We are currently adding -lutil and -lkvm to the linker using the
>add_project_link_arguments method. On FreeBSD 11.4, this results in
>build errors because the args appear too early in the command line.
>
>We need to pass the libraries as dependencies so that they get placed
>at the same point in the linker args as other dependencies.
>
>Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
>---
> meson.build           | 16 +++++++---------
> src/bhyve/meson.build | 15 +++++++++++----
> src/util/meson.build  | 26 +++++++++++++++++---------
> 3 files changed, 35 insertions(+), 22 deletions(-)
>
>Using the CI patch I posted earlier to add FreeBSD 11:
>
>  https://gitlab.com/berrange/libvirt/-/pipelines/185834799
>
>

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

And it looks like a fun and safe change to push on a Friday afternoon.

Jano

Re: [PATCH] Fix linkage to libutil and libkvm on FreeBSD 11

Posted by Roman Bogorodskiy 3 years, 7 months ago

  Daniel P. Berrangé wrote:

> We are currently adding -lutil and -lkvm to the linker using the
> add_project_link_arguments method. On FreeBSD 11.4, this results in
> build errors because the args appear too early in the command line.
> 
> We need to pass the libraries as dependencies so that they get placed
> at the same point in the linker args as other dependencies.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  meson.build           | 16 +++++++---------
>  src/bhyve/meson.build | 15 +++++++++++----
>  src/util/meson.build  | 26 +++++++++++++++++---------
>  3 files changed, 35 insertions(+), 22 deletions(-)
> 
> Using the CI patch I posted earlier to add FreeBSD 11:
> 
>   https://gitlab.com/berrange/libvirt/-/pipelines/185834799
> 
> 
> diff --git a/meson.build b/meson.build
> index 1eadea33bf..c30ff187aa 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1086,7 +1086,8 @@ endif
>  # Check for BSD kvm (kernel memory interface)
>  if host_machine.system() == 'freebsd'
>    kvm_dep = cc.find_library('kvm')
> -  add_project_link_arguments('-lkvm', language: 'c')
> +else
> +  kvm_dep = disabler()
>  endif
>  
>  libiscsi_version = '1.18.0'
> @@ -1203,11 +1204,9 @@ have_gnu_gettext_tools = false
>  if not get_option('nls').disabled()
>    have_gettext = cc.has_function('gettext')
>    if not have_gettext
> -    intl_lib = cc.find_library('intl', required: false)
> -    have_gettext = intl_lib.found()
> -    if have_gettext
> -      add_project_link_arguments('-lintl', language: 'c')
> -    endif
> +    intl_dep = cc.find_library('intl', required: false)
> +  else
> +    intl_dep = disabler()
>    endif
>    if not have_gettext and get_option('nls').enabled()
>      error('gettext() is required to build libvirt')

Looks like this error is issued even if we found intl_dep.
Should this check be updated to:

 if not (have_gettext or intl_dep.found()) and get_option('nls').enabled()
   error(...)

?

> @@ -1235,6 +1234,8 @@ if not get_option('nls').disabled()
>        have_gnu_gettext_tools = true
>      endif
>    endif
> +else
> +  intl_dep = disabler()
>  endif
>  
>  numactl_dep = cc.find_library('numa', required: get_option('numactl'))
> @@ -1402,9 +1403,6 @@ if udev_dep.found()
>  endif
>  
>  util_dep = cc.find_library('util', required: false)
> -if util_dep.found()
> -  add_project_link_arguments('-lutil', language: 'c')
> -endif
>  
>  if not get_option('virtualport').disabled()
>    if cc.has_header_symbol('linux/if_link.h', 'IFLA_PORT_MAX')

Roman Bogorodskiy

Re: [PATCH] Fix linkage to libutil and libkvm on FreeBSD 11

Posted by Pavel Hrdina 3 years, 7 months ago

On Fri, Sep 04, 2020 at 02:23:48PM +0100, Daniel P. Berrangé wrote:
> We are currently adding -lutil and -lkvm to the linker using the
> add_project_link_arguments method. On FreeBSD 11.4, this results in
> build errors because the args appear too early in the command line.
> 
> We need to pass the libraries as dependencies so that they get placed
> at the same point in the linker args as other dependencies.
> 
> Signed-off-by: Daniel P. Berrangé <berrange@redhat.com>
> ---
>  meson.build           | 16 +++++++---------
>  src/bhyve/meson.build | 15 +++++++++++----
>  src/util/meson.build  | 26 +++++++++++++++++---------
>  3 files changed, 35 insertions(+), 22 deletions(-)
> 
> Using the CI patch I posted earlier to add FreeBSD 11:
> 
>   https://gitlab.com/berrange/libvirt/-/pipelines/185834799
> 
> 
> diff --git a/meson.build b/meson.build
> index 1eadea33bf..c30ff187aa 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -1086,7 +1086,8 @@ endif
>  # Check for BSD kvm (kernel memory interface)
>  if host_machine.system() == 'freebsd'
>    kvm_dep = cc.find_library('kvm')
> -  add_project_link_arguments('-lkvm', language: 'c')
> +else
> +  kvm_dep = disabler()
>  endif
>  
>  libiscsi_version = '1.18.0'
> @@ -1203,11 +1204,9 @@ have_gnu_gettext_tools = false
>  if not get_option('nls').disabled()
>    have_gettext = cc.has_function('gettext')
>    if not have_gettext
> -    intl_lib = cc.find_library('intl', required: false)
> -    have_gettext = intl_lib.found()
> -    if have_gettext
> -      add_project_link_arguments('-lintl', language: 'c')
> -    endif
> +    intl_dep = cc.find_library('intl', required: false)
> +  else
> +    intl_dep = disabler()
>    endif
>    if not have_gettext and get_option('nls').enabled()
>      error('gettext() is required to build libvirt')
> @@ -1235,6 +1234,8 @@ if not get_option('nls').disabled()
>        have_gnu_gettext_tools = true
>      endif
>    endif
> +else
> +  intl_dep = disabler()
>  endif

We can use the same construct as other places in libvirt:

    dependency('', required: false)

which returns empty dependency that can be used unconditionally.

>  
>  numactl_dep = cc.find_library('numa', required: get_option('numactl'))
> @@ -1402,9 +1403,6 @@ if udev_dep.found()
>  endif
>  
>  util_dep = cc.find_library('util', required: false)
> -if util_dep.found()
> -  add_project_link_arguments('-lutil', language: 'c')
> -endif
>  
>  if not get_option('virtualport').disabled()
>    if cc.has_header_symbol('linux/if_link.h', 'IFLA_PORT_MAX')
> diff --git a/src/bhyve/meson.build b/src/bhyve/meson.build
> index 7d54718820..c382f64aee 100644
> --- a/src/bhyve/meson.build
> +++ b/src/bhyve/meson.build
> @@ -14,15 +14,22 @@ driver_source_files += bhyve_sources
>  stateful_driver_source_files += bhyve_sources
>  
>  if conf.has('WITH_BHYVE')
> +  bhyve_driver_deps = [
> +    access_dep,
> +    src_dep,
> +  ]
> +  if kvm_dep.found()
> +    bhyve_driver_deps += kvm_dep
> +  endif
> +  if util_dep.found()
> +    bhyve_driver_deps += util_dep
> +  endif
>    bhyve_driver_impl = static_library(
>      'virt_driver_bhyve_impl',
>      [
>        bhyve_sources,
>      ],
> -    dependencies: [
> -      access_dep,
> -      src_dep,
> -    ],

Here you would simply add kvm_dep and util_dep.

Pavel

> +    dependencies: bhyve_driver_deps,
>      include_directories: [
>        conf_inc_dir,
>        hypervisor_inc_dir,
> diff --git a/src/util/meson.build b/src/util/meson.build
> index f7092cc3f1..c899f232e6 100644
> --- a/src/util/meson.build
> +++ b/src/util/meson.build
> @@ -172,15 +172,7 @@ io_helper_sources = [
>    'iohelper.c',
>  ]
>  
> -virt_util_lib = static_library(
> -  'virt_util',
> -  [
> -    util_sources,
> -    util_public_sources,
> -    keycode_gen_sources,
> -    dtrace_gen_headers,
> -  ],
> -  dependencies: [
> +virt_util_deps = [
>      acl_dep,
>      audit_dep,
>      capng_dep,
> @@ -195,7 +187,23 @@ virt_util_lib = static_library(
>      thread_dep,
>      win32_dep,
>      yajl_dep,
> +  ]
> +if util_dep.found()
> +  virt_util_deps += util_dep
> +endif
> +if intl_dep.found()
> +  virt_util_deps += intl_dep
> +endif
> +
> +virt_util_lib = static_library(
> +  'virt_util',
> +  [
> +    util_sources,
> +    util_public_sources,
> +    keycode_gen_sources,
> +    dtrace_gen_headers,
>    ],
> +  dependencies: virt_util_deps,
>  )
>  
>  libvirt_libs += virt_util_lib
> -- 
> 2.26.2
>