allow meson to find ICU in non-standard localtion

Started by Jeff Davisalmost 3 years ago8 messages
#1Jeff Davis
pgsql@j-davis.com
1 attachment(s)

I attached a simple patch that allows meson to find ICU in a non-
standard location if if you specify -Dextra_lib_dirs and
-Dextra_include_dirs.

I'm not sure it's the right thing to do though. One downside is that it
doesn't output the version that it finds, it only outputs "YES".

Attachments:

v1-0001-Allow-meson-to-find-ICU-in-non-standard-locations.patchtext/x-patch; charset=UTF-8; name=v1-0001-Allow-meson-to-find-ICU-in-non-standard-locations.patchDownload
From 8fd8d42a6ef9a4589776260e539f10d730e1c3f1 Mon Sep 17 00:00:00 2001
From: Jeff Davis <jeff@j-davis.com>
Date: Wed, 22 Feb 2023 10:24:52 -0800
Subject: [PATCH v1] Allow meson to find ICU in non-standard locations.

---
 meson.build | 7 +++++--
 1 file changed, 5 insertions(+), 2 deletions(-)

diff --git a/meson.build b/meson.build
index f534704452..250840b281 100644
--- a/meson.build
+++ b/meson.build
@@ -721,8 +721,11 @@ endif
 
 icuopt = get_option('icu')
 if not icuopt.disabled()
-  icu = dependency('icu-uc', required: icuopt.enabled())
-  icu_i18n = dependency('icu-i18n', required: icuopt.enabled())
+
+  icu = cc.find_library('icuuc', required: icuopt.enabled(),
+     dirs: postgres_lib_d, header_include_directories: postgres_inc)
+  icu_i18n = cc.find_library('icui18n', required: icuopt.enabled(),
+    dirs: postgres_lib_d, header_include_directories: postgres_inc)
 
   if icu.found()
     cdata.set('USE_ICU', 1)
-- 
2.34.1

#2Nazir Bilal Yavuz
byavuz81@gmail.com
In reply to: Jeff Davis (#1)
Re: allow meson to find ICU in non-standard localtion

Hi,

Thanks for the patch.

On Wed, 22 Feb 2023 at 21:26, Jeff Davis <pgsql@j-davis.com> wrote:

I'm not sure it's the right thing to do though. One downside is that it
doesn't output the version that it finds, it only outputs "YES".

- icu = dependency('icu-uc', required: icuopt.enabled())
- icu_i18n = dependency('icu-i18n', required: icuopt.enabled())

I think you can do dependency checks with 'required: false' first and
if they weren't found by dependency checks; then you can do
cc.find_library() checks. This also solves only the outputting "YES"
problem if they were found by dependency checks.

Regards,
Nazir Bilal Yavuz
Microsoft

#3Andres Freund
andres@anarazel.de
In reply to: Jeff Davis (#1)
Re: allow meson to find ICU in non-standard localtion

Hi,

On 2023-02-22 10:26:23 -0800, Jeff Davis wrote:

I attached a simple patch that allows meson to find ICU in a non-
standard location if if you specify -Dextra_lib_dirs and
-Dextra_include_dirs.

If you tell meson where to find the pkg-config file in those directories it'd
also work. -Dpkg_config_path=...

Does that suffice?

Greetings,

Andres Freund

#4Jeff Davis
pgsql@j-davis.com
In reply to: Andres Freund (#3)
Re: allow meson to find ICU in non-standard localtion

On Sun, 2023-02-26 at 09:57 -0800, Andres Freund wrote:

If you tell meson where to find the pkg-config file in those
directories it'd
also work. -Dpkg_config_path=...

Setup is able to find it, which is good, but it seems like it's not
adding it to RPATH so it's not working.

I think we need some doc updates to clarify which features are affected
by -Dextra_lib_dirs/-Dpkg_config_path.

Regards,
Jeff Davis

#5Jacob Champion
jchampion@timescale.com
In reply to: Jeff Davis (#4)
Re: allow meson to find ICU in non-standard localtion

On Sun, Feb 26, 2023 at 7:36 PM Jeff Davis <pgsql@j-davis.com> wrote:

On Sun, 2023-02-26 at 09:57 -0800, Andres Freund wrote:

If you tell meson where to find the pkg-config file in those
directories it'd
also work. -Dpkg_config_path=...

Setup is able to find it, which is good, but it seems like it's not
adding it to RPATH so it's not working.

For my custom OpenSSL setups using -Dpkg_config_path, meson initially
adds the correct RPATH during build, then accidentally(?) strips it
during the `ninja install` step. This has been complained about [1]https://github.com/mesonbuild/meson/issues/6541,
and it seems like maybe they intended to fix it back in 0.55, but I'm
not convinced they did. :)

I work around it by manually setting -Dextra_lib_dirs. I just tried
doing that with ICU 72, and it worked without a patch. Hopefully that
helps some?

--Jacob

[1]: https://github.com/mesonbuild/meson/issues/6541

#6Jeff Davis
pgsql@j-davis.com
In reply to: Jacob Champion (#5)
Re: allow meson to find ICU in non-standard localtion

On Wed, 2023-03-01 at 11:43 -0800, Jacob Champion wrote:

I work around it by manually setting -Dextra_lib_dirs. I just tried
doing that with ICU 72, and it worked without a patch. Hopefully that
helps some?

Yes, works, thank you.

Obviously we'd like a little better solution so that others don't get
confused, but it's not really a problem for me any more.

Also there's the issue that libxml2.so pulls in the system's ICU
regardless. I don't think that causes a major problem, but I thought
I'd mention it.

Regards,
Jeff Davis

#7Peter Eisentraut
peter.eisentraut@enterprisedb.com
In reply to: Jeff Davis (#6)
Re: allow meson to find ICU in non-standard localtion

On 01.03.23 21:30, Jeff Davis wrote:

On Wed, 2023-03-01 at 11:43 -0800, Jacob Champion wrote:

I work around it by manually setting -Dextra_lib_dirs. I just tried
doing that with ICU 72, and it worked without a patch. Hopefully that
helps some?

Yes, works, thank you.

Obviously we'd like a little better solution so that others don't get
confused, but it's not really a problem for me any more.

So should we withdraw the patch from the commit fest?

#8Jeff Davis
pgsql@j-davis.com
In reply to: Peter Eisentraut (#7)
Re: allow meson to find ICU in non-standard localtion

On Wed, 2023-03-08 at 17:30 +0100, Peter Eisentraut wrote:

So should we withdraw the patch from the commit fest?

Withdrawn. If someone else is interested we can still pursue some
improvements.

Regards,
Jeff Davis