Skip to content
This repository has been archived by the owner on Jul 4, 2023. It is now read-only.

yaz vs ICU #31830

Closed
ssp opened this issue Aug 23, 2014 · 15 comments
Closed

yaz vs ICU #31830

ssp opened this issue Aug 23, 2014 · 15 comments

Comments

@ssp
Copy link
Contributor

ssp commented Aug 23, 2014

There seem to have been issues about yaz vs ICU on Homebrew before (see #15377) and I’d like to bring the topic back up.

Essentially the yaz built by homebrew does not seem to support ICU properly. This can be seen by running yaz-icu and getting the result:

ICU not available on your system.
Please install libicu-dev and icu-doc or similar, re-configure and re-compile

This also affects other tools (e.g. pazpar2) which rely on yaz’ ICU capabilities.

I can fix the issue by adding icu4c as a dependency to the formula, but I am not sure whether this is a good idea or should be necessary.

What’s also interesting: yaz as built by Homebrew does link to OS X’s libicucore:

✗ otool -L /usr/local/bin/yaz-icu
/usr/local/bin/yaz-icu:
    /usr/local/Cellar/yaz/5.4.1/lib/libyaz_icu.5.dylib (compatibility version 6.0.0, current version 6.0.0)
    /usr/local/Cellar/yaz/5.4.1/lib/libyaz.5.dylib (compatibility version 6.0.0, current version 6.0.0)
    /usr/lib/libexslt.0.dylib (compatibility version 9.0.0, current version 9.15.0)
    /usr/lib/libxslt.1.dylib (compatibility version 3.0.0, current version 3.26.0)
    /usr/lib/libz.1.dylib (compatibility version 1.0.0, current version 1.2.5)
    /usr/lib/libicucore.A.dylib (compatibility version 1.0.0, current version 51.1.0)
    /usr/lib/libSystem.B.dylib (compatibility version 1.0.0, current version 1197.1.1)
    /usr/lib/libxml2.2.dylib (compatibility version 10.0.0, current version 10.9.0)
    /usr/lib/libiconv.2.dylib (compatibility version 7.0.0, current version 7.0.0)

So it seems to find some kind of ICU related thing.

Unfortunately my understanding of build, Makefile and library finding magic is insufficient to understand what exactly is going wrong here and how it should be resolved (or whether yaz’ Makefile may be part of the problem), so I hope someone with more knowledge about these can understand what’s going on here.

Also pinging @asparagui.

@asparagui
Copy link
Contributor

Can reproduce. Messed around with this a bit, the only semi-relevant data I came across is that Mavericks supposedly tweaked the libicu header slightly, so it might work correctly with prior versions. Are you on 10.9, @ssp?

I sent a message upstream to indexdata as well. They are generally extremely responsive.

@ssp
Copy link
Contributor Author

ssp commented Aug 23, 2014

@asparagui: yes I am on 10.9 (but I am also seeing the same issue on my 10.7 machine with the current bottled yaz version)

Yes Index Data are exceptionally responsive. I also tried asking about an issue touching this on yazlist earlier this year http://lists.indexdata.dk/pipermail/yazlist/2014-February/003863.html but my impression is that the Mac / Homebrew setup may simply not be their main focus as typical yaz deployments will probably on Linux systems anyway. (But it’s so convenient to be able to develop locally on the Mac :)

@adamdickmeiss
Copy link

Guess that YAZ' configure does not find ICU on homebrew. Try this
./configure --with-icu
It will fail hard if ICU is not found (instead of just happy proceed if it does not find ICU).
Let us know, if it fails, what the output was and config.log .

Info: YAZ' configure uses icu-config in PATH and use that as basis for linking with ICU.

@ssp
Copy link
Contributor Author

ssp commented Aug 24, 2014

Hi @adamdickmeiss, thanks for chiming in!

Indeed, ./configure fails (on OS X 10.9) when used with the –with-icu option:

➜  yaz git:(master) ./configure --with-icu
checking for a BSD-compatible install... /usr/bin/install -c
[…]
checking for icu-config... no
configure: error: libicu development libraries not found.

OS X does not seem to come with icu-config at all. So that may be what misleads the configure script?

In fact, the only ICU related files I find on a current system are:

➜ find /usr | grep icu | grep -v local
/usr/lib/libicucore.A.dylib
/usr/lib/libicucore.dylib
/usr/share/icu
/usr/share/icu/icudt51l.dat

where my understanding is that libicucore.dylib is the ICU library on OS X (which even seems to linked into yaz when it is built – can anybody figure out why?).

Would these files even be sufficient for yaz to build properly (no headers?) or does that mean that we really do need the icu4c package which seems to provide more standard files. If that is the case, perhaps someone more familiar with Homebrew can offer advice on how we best deal with that.

@adamdickmeiss
Copy link

You need ICU development libraries (case for all systems).

@asparagui
Copy link
Contributor

To be lazy (pragmatic), it might be easiest just to add a --with-icu option that adds the icu4c dependency and a caveat about what is going on to the end-user.

@MikeMcQuaid
Copy link
Member

Yes, that seems reasonable.

ssp added a commit to ssp/homebrew that referenced this issue Aug 25, 2014
as discussed in Homebrew#31830

* it would be better to make this the default rather than optional
* add test for yaz-icu
@ssp
Copy link
Contributor Author

ssp commented Aug 25, 2014

@asparagui That may be a pragmatic first step and I tried to improve the formula with this:

https://github.com/ssp/homebrew/tree/yaz-icu

However, I still see a number of issues:

  • it seems odd to not not use ICU by default, especially as yaz does use ICU
  • no matter what I do, yaz always seems to link to /usr/lib/libicucore.A.dylib. Should this be the case? Why is this happening?
  • I still do not understand what the exact issue/problem with icu4c is. Especially regarding the now-closed issue YAZ won’t install #167 which is referred to when installing icu4c (see also: YAZ won’t install #167 (comment)). Hence I am not sure what kind of note to write to that caveat.

@ssp
Copy link
Contributor Author

ssp commented Sep 2, 2014

Just to ping you again for the answers to my open questions and advice on how to proceed with this – in particular whether defaulting to using ICU support or not makes sense or not (and if so, what it breaks).

My preferred solution would be to build with ICU support by default. Especially as yaz’ ICU support will be inherited by software depending on yaz (metaproxy, pazpar2, zebra). In my experience, it’s pretty much a given that you want to use ICU support in pazpar2 at least. So ensuring it’s fully functional by default seems like a good idea to me.

That said: I have not seen, nor understood, the issues people seem to have had with icu4c in the past. I am happy to change my preference if these still exist and cause inconvenience for users.

@MikeMcQuaid
Copy link
Member

@ssp Stupid question but: what does ICU support give you in those applications? The answer to that question will mean my answer will vary from "use icu4c as a recommended dependency" or "use icu4c as an optional dependency"

@ssp
Copy link
Contributor Author

ssp commented Sep 3, 2014

In pazpar2 (bibliographic metasearch software) the crucial use of ICU is to normalise the terms used for faceting, deduplication or sorting (search for icu_chain in the manpage). You end up with a lot of duplicate facets and missed opportunities for deduplication without this feature due to differences in capitalisation, Unicode Normalisation Form or other technical differences between the records.

In zebra (bibliographic index server), ICU can be used to clean and take into account language features when indexing and sorting. manual

I am not sure how metaproxy uses ICU features.

I also tried to find out how Index Data distribute their software in their Ubuntu packages. Things seem to be a little different there as they offer more fine-grained packages than we have in Homebrew. Yet, they always list libicu as a dependency for libyaz which is used by the packages for pazpar2, metaproxy and zebra.

@MikeMcQuaid
Copy link
Member

Ok, seems fair enough to be on by default.

ssp added a commit to ssp/homebrew that referenced this issue Sep 3, 2014
as discussed in Homebrew#31830

* add test for yaz-icu when building with ICU
@ssp
Copy link
Contributor Author

ssp commented Sep 3, 2014

Great. I adjusted the yaz formula to:

  • build with icu4c by default
  • have a --without-icu option (or should I just mark the icu4c dependency as :recommeded and have the more technical name?)
  • test the yaz-icu utility when building with ICU
  • have the latest version

Is there anything else that should be done? Do we need to ensure the formulae depending on yaz are re-built when this happens to reduce the chance of inconsistent results?

ssp added a commit to ssp/homebrew that referenced this issue Sep 12, 2014
as discussed in Homebrew#31830

* add test for yaz-icu when building with ICU
ssp added a commit to ssp/homebrew that referenced this issue Sep 12, 2014
as discussed in Homebrew#31830

* add test for yaz-icu when building with ICU
@ssp ssp mentioned this issue Sep 12, 2014
@ssp
Copy link
Contributor Author

ssp commented Sep 12, 2014

As there were no remarks on my questions, I assume that going forward with this is OK and submitted the changes as pull request #32261

ssp added a commit to ssp/homebrew that referenced this issue Sep 12, 2014
as discussed in Homebrew#31830

Also:

* add test for yaz-icu when building with ICU
* do not use File.open for writing test configuration files
ssp added a commit to ssp/homebrew that referenced this issue Sep 12, 2014
as discussed in Homebrew#31830

Also:

* add test for yaz-icu when building with ICU
* do not use File.open for writing test configuration files
ssp added a commit to ssp/homebrew that referenced this issue Sep 12, 2014
as discussed in Homebrew#31830

Also:

* add test for yaz-icu when building with ICU
* do not use File.open for writing test configuration files
ssp added a commit to ssp/homebrew that referenced this issue Sep 13, 2014
as discussed in Homebrew#31830

Also:

* add test for yaz-icu when building with ICU
* do not use File.open for writing test configuration files
MikeMcQuaid pushed a commit that referenced this issue Sep 15, 2014
as discussed in #31830

Also:

* add test for yaz-icu when building with ICU
* do not use File.open for writing test configuration files
@ssp
Copy link
Contributor Author

ssp commented Sep 15, 2014

Thanks for everybody’s help and guidance!

I am closing this issue now that #32261 has been merged.

@ssp ssp closed this as completed Sep 15, 2014
@Homebrew Homebrew locked and limited conversation to collaborators Feb 17, 2016
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants