Fix issues with libintl and libgpgme during builds

Message ID BYAPR12MB32382ACAA2598AC77DA671A4A41A9@BYAPR12MB3238.namprd12.prod.outlook.com
State New
Headers show
Series Fix issues with libintl and libgpgme during builds | expand

Commit Message

Ziemowit Laski Dec. 7, 2022, 5:03 a.m. UTC
This is pretty simple and hopefully self-explanatory, but please do ask questions if needed.

Thanks,
--Zem


>From c4a5adf001d3124854572bab561fbe3a21a885d4 Mon Sep 17 00:00:00 2001
From: =?UTF-8?q?Ziemowit=20=C5=81=C4=85ski?=
 <15880281+zlaski@users.noreply.github.com>
Date: Tue, 6 Dec 2022 19:53:57 -0800
Subject: [PATCH] Fix issues with libintl and libgpgme

The `libintl` library was not getting pulled in if `ngettext()` was missing but option `i18n` was not specified.
The accepted `libgpgme` version was too old, leading to a missing `GPGME_KEYLIST_MODE_LOCATE` macro.

Signed-off-by: Ziemowit Łąski <zlaski@ziemas.net>
---
 meson.build | 36 +++++++++++++++++++++---------------
 1 file changed, 21 insertions(+), 15 deletions(-)

Comments

Allan McRae Dec. 13, 2022, 1:45 a.m. UTC | #1
On 7/12/22 15:03, Ziemowit Laski wrote:
> This is pretty simple and hopefully self-explanatory, but please do ask questions if needed.
> 
> Thanks,
> --Zem
> 
> 

The GPGME change is fine - I will split the patch and pull it in.

I have no idea what change you are trying to make with ngettext and i18n.

Allan

>>From c4a5adf001d3124854572bab561fbe3a21a885d4 Mon Sep 17 00:00:00 2001
> From: =?UTF-8?q?Ziemowit=20=C5=81=C4=85ski?=
>   <15880281+zlaski@users.noreply.github.com>
> Date: Tue, 6 Dec 2022 19:53:57 -0800
> Subject: [PATCH] Fix issues with libintl and libgpgme
> 
> The `libintl` library was not getting pulled in if `ngettext()` was missing but option `i18n` was not specified.
> The accepted `libgpgme` version was too old, leading to a missing `GPGME_KEYLIST_MODE_LOCATE` macro.
> 
> Signed-off-by: Ziemowit Łąski <zlaski@ziemas.net>
> ---
>   meson.build | 36 +++++++++++++++++++++---------------
>   1 file changed, 21 insertions(+), 15 deletions(-)
> 
> diff --git a/meson.build b/meson.build
> index c8ee42fd..a6c92276 100644
> --- a/meson.build
> +++ b/meson.build
> @@ -77,16 +77,21 @@ conf.set_quoted('CACHEDIR', join_paths(LOCALSTATEDIR, 'cache/pacman/pkg/'))
>   conf.set_quoted('HOOKDIR', join_paths(SYSCONFDIR, 'pacman.d/hooks/'))
>   conf.set_quoted('ROOTDIR', ROOTDIR)
>   
> -libintl = dependency('', required: false)
> -if get_option('i18n')
> -  if not cc.has_function('ngettext')
> -    libintl = cc.find_library('intl', required : false, static: get_option('buildstatic'))
> -    if not libintl.found()
> -      error('ngettext not found but NLS support requested')
> -    endif
> -  endif
> -  conf.set('ENABLE_NLS', 1)
> +# i18n / ngettext
> +has_ngettext = cc.has_function('ngettext')
> +libintl = dependency('libintl',
> +                     required: false,
> +                     static: get_option('buildstatic'))
> +if not libintl.found()
> +  libintl = cc.find_library('libintl',
> +                            required: false,
> +                            static: get_option('buildstatic'))
> +endif
> +conf.set('HAVE_LIBINTL', libintl.found())
> +if not libintl.found() and (get_option('i18n') or not has_ngettext)
> +  error('libintl not found even though ngettext() not found or NLS support requested')
>   endif
> +conf.set('ENABLE_NLS', 1)
>   
>   # dependencies
>   libarchive = dependency('libarchive',
> @@ -104,7 +109,7 @@ libcurl = dependency('libcurl',
>                        static : get_option('buildstatic'))
>   conf.set('HAVE_LIBCURL', libcurl.found())
>   
> -needed_gpgme_version = '>=1.3.0'
> +needed_gpgme_version = '>=1.12.0'
>   gpgme = dependency('gpgme',
>                      version : needed_gpgme_version,
>                      required : get_option('gpgme'),
> @@ -290,7 +295,7 @@ subdir('src/util')
>   subdir('scripts')
>   
>   # Internationalization
> -if get_option('i18n')
> +if conf.get('HAVE_LIBINTL')
>     i18n = import('i18n')
>     subdir('lib/libalpm/po')
>     subdir('src/pacman/po')
> @@ -350,7 +355,7 @@ pacman_bin = executable(
>     pacman_sources,
>     include_directories : includes,
>     link_with : [libalpm, libcommon],
> -  dependencies : [libarchive],
> +  dependencies : [libarchive, libintl],
>     install : true,
>   )
>   
> @@ -359,7 +364,7 @@ executable(
>     pacman_conf_sources,
>     include_directories : includes,
>     link_with : [libalpm, libcommon],
> -  dependencies : [libarchive],
> +  dependencies : [libarchive, libintl],
>     install : true,
>   )
>   
> @@ -368,7 +373,7 @@ executable(
>     testpkg_sources,
>     include_directories : includes,
>     link_with : [libalpm],
> -  dependencies : [libarchive],
> +  dependencies : [libarchive, libintl],
>     install : true,
>   )
>   
> @@ -420,7 +425,7 @@ foreach path : [
>   	join_paths(DATAROOTDIR, 'makepkg-template/'),
>   	join_paths(DATAROOTDIR, 'libalpm/hooks/'),
>   	]
> -	meson.add_install_script('sh', '-c', 'mkdir -p "$DESTDIR/@0@"'.format(path))
> +	meson.add_install_script('sh', '-c', 'mkdir -p "$DESTDIR@0@"'.format(path))
>   endforeach
>   
>   TEST_ENV = environment()
> @@ -464,6 +469,7 @@ message('\n    '.join([
>     '  Build docs               : @0@'.format(build_doc),
>     '  debug build              : @0@'.format(get_option('buildtype') == 'debug'),
>     '  Use libcurl              : @0@'.format(conf.get('HAVE_LIBCURL')),
> +  '  Use libintl              : @0@'.format(conf.get('HAVE_LIBINTL')),
>     '  Use GPGME                : @0@'.format(conf.get('HAVE_LIBGPGME')),
>     '  Use OpenSSL              : @0@'.format(conf.has('HAVE_LIBSSL') and
>                                               conf.get('HAVE_LIBSSL') == 1),
Ziemowit Laski Dec. 14, 2022, 6:57 a.m. UTC | #2
The existing implementation was broken: Even if the user selected the 'i18n' option, libintl would NOT be included if there was a pre-existing ngettext().  I rewrote it so that libintl would get pulled in if 'i18n' was specified OR ngettext() was missing.

However, I now realize that this can be simplified so that libintl is used IFF 'i18n' is present.  The ENABLE_NLS macro would then be set to 1 if libintl was found OR if ngettext() was found elsewhere.

I shall prepare a streamlined patch.

> -----Original Message-----
> From: Allan McRae <allan@archlinux.org>
> Sent: Monday, December 12, 2022 17:45
> I have no idea what change you are trying to make with ngettext and i18n.
> 
> Allan

Patch

diff --git a/meson.build b/meson.build
index c8ee42fd..a6c92276 100644
--- a/meson.build
+++ b/meson.build
@@ -77,16 +77,21 @@  conf.set_quoted('CACHEDIR', join_paths(LOCALSTATEDIR, 'cache/pacman/pkg/'))
 conf.set_quoted('HOOKDIR', join_paths(SYSCONFDIR, 'pacman.d/hooks/'))
 conf.set_quoted('ROOTDIR', ROOTDIR)
 
-libintl = dependency('', required: false)
-if get_option('i18n')
-  if not cc.has_function('ngettext')
-    libintl = cc.find_library('intl', required : false, static: get_option('buildstatic'))
-    if not libintl.found()
-      error('ngettext not found but NLS support requested')
-    endif
-  endif
-  conf.set('ENABLE_NLS', 1)
+# i18n / ngettext
+has_ngettext = cc.has_function('ngettext')
+libintl = dependency('libintl',
+                     required: false,
+                     static: get_option('buildstatic'))
+if not libintl.found()
+  libintl = cc.find_library('libintl',
+                            required: false,
+                            static: get_option('buildstatic'))
+endif                            
+conf.set('HAVE_LIBINTL', libintl.found())
+if not libintl.found() and (get_option('i18n') or not has_ngettext)
+  error('libintl not found even though ngettext() not found or NLS support requested')
 endif
+conf.set('ENABLE_NLS', 1)
 
 # dependencies
 libarchive = dependency('libarchive',
@@ -104,7 +109,7 @@  libcurl = dependency('libcurl',
                      static : get_option('buildstatic'))
 conf.set('HAVE_LIBCURL', libcurl.found())
 
-needed_gpgme_version = '>=1.3.0'
+needed_gpgme_version = '>=1.12.0'
 gpgme = dependency('gpgme',
                    version : needed_gpgme_version,
                    required : get_option('gpgme'),
@@ -290,7 +295,7 @@  subdir('src/util')
 subdir('scripts')
 
 # Internationalization
-if get_option('i18n')
+if conf.get('HAVE_LIBINTL')
   i18n = import('i18n')
   subdir('lib/libalpm/po')
   subdir('src/pacman/po')
@@ -350,7 +355,7 @@  pacman_bin = executable(
   pacman_sources,
   include_directories : includes,
   link_with : [libalpm, libcommon],
-  dependencies : [libarchive],
+  dependencies : [libarchive, libintl],
   install : true,
 )
 
@@ -359,7 +364,7 @@  executable(
   pacman_conf_sources,
   include_directories : includes,
   link_with : [libalpm, libcommon],
-  dependencies : [libarchive],
+  dependencies : [libarchive, libintl],
   install : true,
 )
 
@@ -368,7 +373,7 @@  executable(
   testpkg_sources,
   include_directories : includes,
   link_with : [libalpm],
-  dependencies : [libarchive],
+  dependencies : [libarchive, libintl],
   install : true,
 )
 
@@ -420,7 +425,7 @@  foreach path : [
 	join_paths(DATAROOTDIR, 'makepkg-template/'),
 	join_paths(DATAROOTDIR, 'libalpm/hooks/'),
 	]
-	meson.add_install_script('sh', '-c', 'mkdir -p "$DESTDIR/@0@"'.format(path))
+	meson.add_install_script('sh', '-c', 'mkdir -p "$DESTDIR@0@"'.format(path))
 endforeach
 
 TEST_ENV = environment()
@@ -464,6 +469,7 @@  message('\n    '.join([
   '  Build docs               : @0@'.format(build_doc),
   '  debug build              : @0@'.format(get_option('buildtype') == 'debug'),
   '  Use libcurl              : @0@'.format(conf.get('HAVE_LIBCURL')),
+  '  Use libintl              : @0@'.format(conf.get('HAVE_LIBINTL')),
   '  Use GPGME                : @0@'.format(conf.get('HAVE_LIBGPGME')),
   '  Use OpenSSL              : @0@'.format(conf.has('HAVE_LIBSSL') and
                                             conf.get('HAVE_LIBSSL') == 1),