elixir-cldr / cldr_collation

CLDR Collation
Other
4 stars 4 forks source link

Upgrading to 0.7.4 on MacOS causes Cldr.Collation not found #15

Closed tjchambers closed 1 month ago

tjchambers commented 1 month ago

After upgrading from 0.7.3 to 0.7.4 I am seeing:

Generating Sct.Cldr for 4 locales named [:en, :"en-GB", :es, :und] with a default locale named :en

10:32:38.739 [warning] The on_load function for module Elixir.Cldr.Collation returned:
{:error,
 {:load_failed,
  ~c"Failed to load NIF library: 'dlopen(/Users/tj/projects/sct_elixir/_build/test/lib/ex_cldr_collation/priv/ucol.so, 0x0002): symbol not found in flat namespace '_ucol_close_48''"}}

10:32:38.849 [warning] The on_load function for module Elixir.Cldr.Collation returned:
{:error,
 {:load_failed,
  ~c"Failed to load NIF library: 'dlopen(/Users/tj/projects/sct_elixir/_build/test/lib/ex_cldr_collation/priv/ucol.so, 0x0002): symbol not found in flat namespace '_ucol_close_48''"}}

10:32:38.850 [warning] Sct.Cldr: The CLDR provider module Cldr.Collation was not found

I tried deleting my _build and still same error.

am seeing ** (UndefinedFunctionError) function Cldr.Collation.nif_compare/3 is undefined (module Cldr.Collation is not available) in all tests involving collation code.

==> ex_cldr_collation
Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:24:24] [ds:24:24:10] [async-threads:1] [jit]

cc c_src/ucol.o -arch arm64 -flat_namespace -undefined suppress -shared -L/Users/tj/.asdf/installs/erlang/27.0/usr/lib -lei -lpthread -lm -licucore -lstdc++ -o ./priv/ucol.so
ld: warning: -undefined suppress is deprecated
ld: warning: -undefined suppress is deprecated

^ after downgrading to 0.7.3 (only change) seems to go back to working.

BTW am on Elixir 1.17.1 and MacOS Sonoma

kipcole9 commented 1 month ago

That's very unexpected! (I develop and test on MacOS). I created a new app, added the dependency and compiled and didn't see the same error. I'm wondering if this is OS version related? I'm still on Sonoma - perhaps you've upgraded already (or should I say I'm a bit slow to upgrade this machine). I tested on both M1 and Intel machines too.

% mix new my_app
* creating README.md
* creating .formatter.exs
* creating .gitignore
* creating mix.exs
* creating lib
* creating lib/my_app.ex
* creating test
* creating test/test_helper.exs
* creating test/my_app_test.exs

Your Mix project was created successfully.
You can use "mix" to compile it, test it, and more:

    cd my_app
    mix test

Run "mix help" for more commands.
% cd my_app
% mix deps.get
Resolving Hex dependencies...
Resolution completed in 0.027s
New:
  elixir_make 0.8.4
  ex_cldr_collation 0.7.4
* Getting ex_cldr_collation (Hex package)
* Getting elixir_make (Hex package)
% iex -S mix
Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit] [dtrace]

==> elixir_make
Compiling 8 files (.ex)
Generated elixir_make app
==> ex_cldr_collation
Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:10:10] [ds:10:10:10] [async-threads:1] [jit] [dtrace]

cc c_src/ucol.o -arch arm64 -flat_namespace -undefined suppress -shared -L/Users/kip/.asdf/installs/erlang/27.0/usr/lib -lei -lpthread -lm -licucore -lstdc++ -o ./priv/ucol.so
ld: warning: -undefined suppress is deprecated
ld: warning: -undefined suppress is deprecated
Compiling 3 files (.ex)
Generated ex_cldr_collation app
==> my_app
Compiling 1 file (.ex)
Generated my_app app
Interactive Elixir (1.17.2) - press Ctrl+C to exit (type h() ENTER for help)
iex(1)> Cldr.Collation.compare "a", "b"
:lt
tjchambers commented 1 month ago

As I indicated above I have NOT upgraded to Sequoia. AM wondering if I need to delete something to make this more virgin.

tjchambers commented 1 month ago

BTW I am on an M2 Ultra running 14.6.1.

kipcole9 commented 1 month ago

As I indicated above I have NOT upgraded to Sequoia. AM wondering if I need to delete something to make this more virgin.

Sorry @tjchambers, should have seen that, haven't had coffee yet.

If you delete ./deps/ex_cldr_collation/priv/ucol.so does that make any difference? Or given the compile errors, perhaps it's not even there? And yeah, that's not really a good place to put artefacts either, I need to work on that. Anyway, for now, that's the NIF module.

tjchambers commented 1 month ago

I have an inference about what may be different. I tried your reproduction on my machine, but did so by starting with fresh app but on 0.7.3. All worked as your example.

Then I bumped to 0.7.4 (as I had done on my full app) and it failed. But I suspect it is related to the change in parameters vs. my historical config.

Upgraded:
  ex_cldr_collation 0.7.3 => 0.7.4
* Updating ex_cldr_collation (Hex package)
~/projects/sct_elixir@Brie [14:31:19]> mix compile
==> ex_cldr_collation
Erlang/OTP 27 [erts-15.0] [source] [64-bit] [smp:24:24] [ds:24:24:10] [async-threads:1] [jit]

cc c_src/ucol.o -arch arm64 -flat_namespace -undefined suppress -shared -L/Users/tj/.asdf/installs/erlang/27.0/usr/lib -lei -L/usr/local/lib -licuio -licui18n -licuuc -licudata -o ./priv/ucol.so
ld: warning: -undefined suppress is deprecated
ld: warning: -undefined suppress is deprecated
ld: warning: ignoring file '/usr/local/lib/libicuio.48.1.dylib': found architecture 'x86_64', required architecture 'arm64'
ld: warning: ignoring file '/usr/local/lib/libicudata.48.1.dylib': found architecture 'x86_64', required architecture 'arm64'
ld: warning: ignoring file '/usr/local/lib/libicui18n.48.1.dylib': found architecture 'x86_64', required architecture 'arm64'
ld: warning: ignoring file '/usr/local/lib/libicuuc.48.1.dylib': found architecture 'x86_64', required architecture 'arm64'
Compiling 3 files (.ex)

14:31:28.840 [warning] The on_load function for module Elixir.Cldr.Collation returned:
{:error,
 {:load_failed,
  ~c"Failed to load NIF library: 'dlopen(/Users/tj/projects/sct_elixir/_build/dev/lib/ex_cldr_collation/priv/ucol.so, 0x0002): symbol not found in flat namespace '_ucol_close_48''"}}

Generated ex_cldr_collation app

My OS was originally on an Intel based machine and has been migrated and updated as I went to Mac Studio. Note the icu files above are not arm64 (yet). My inference is the change in location/usage of the icu files is now failing because I still have x86 versions in place and the arch required arm64 versions of these (which is part of the reason for this change).

I am going to try and see how I can bring these forward as arm64 modules to see if I can normalize this discrepancy.

Note the compile I posted in original was from 0.7.3 (that may not have been clear). the `-L/usr/local/lib" in the args is not present as it is in 0.7.4.

This is above my understanding, but it is a noticed difference. Does that help explain anything?

kipcole9 commented 1 month ago

Ahhh, that might help explain it.

The change in 0.7.4 is to basically prioritise using libicu if it is installed. That part of the Makefile looks like this:

ICU_CFLAGS = $(shell pkg-config --cflags icu-uc icu-io --silence-errors)
ICU_LIBS = $(shell pkg-config --libs icu-uc icu-io --silence-errors)

ifeq ($(ICU_LIBS),)
  ifeq ($(UNAME_SYS), Darwin)
        ICU_CFLAGS = -I$(BASEDIR)/c_src/platform/osx/icu
        ICU_LIBS = -lpthread -lm -licucore -lstdc++
    endif
endif

So if libicu is installed, then it is used. If not installed, it will use the version embedded in ex_cldr_collation.

It looks therefore like the libicu you have installed is still the x86 version, not the ARM version. Which I think is what you are saying?

tjchambers commented 1 month ago

Yes - that is what I am saying. My inference is that assuming on a Mac that the one installed IS definitely the current arch of the machine may be a bit of a stretch. Especially given Apple's propensity to have older versions. SO that makes me ambivalent about this. I am trying to get an arm64 version installed - which I do have 74.2 of icu4c installed via brew, but I am not successful in getting it to be the default (probably due to symlinking). Given that building 0.7.3 worked and also that it specified arch arm64 is it reasonable to assume that 0.7.3 build args used a arm64 version? If that is a reasonable assumption, then this seems like a bit of a risky change. Not saying I should not have a arm64 version - just that historical paths may cause this to recur for others???

tjchambers commented 1 month ago

Thanks @kipcole9 for patience while I worked through this.

I did a brew reinstall icu4c and then brew link icu4c --force (it is keg-only) and was able to get it to work successfully as the symlink is now pointing to a 74.2 arm64 version(s).

So for me problem is solved.

kipcole9 commented 1 month ago

OK, I did the following:

% brew reinstall icu4c
% brew link icu4c --force
% pkg-config --cflags icu-uc icu-io
-I/opt/homebrew/Cellar/icu4c/74.2/include
% mix deps.compile ex_cldr_collation
==> ex_cldr_collation
cc c_src/ucol.o -arch arm64 -flat_namespace -undefined suppress -shared -L/Users/kip/.asdf/installs/erlang/27.0/usr/lib -lei -L/opt/homebrew/Cellar/icu4c/74.2/lib -licuio -licui18n -licuuc -licudata -o ./priv/ucol.so
ld: warning: -undefined suppress is deprecated
ld: warning: -undefined suppress is deprecated`

So basically operating as expected using the brew installed icu4c - not the MacOS native version.

Given your situation, may I suggest trying the following:

% brew unlink icu4c
% rm ./deps/ex_cldr_collation/priv/ucol.so
% mix deps.compile ex_cldr_collation

And see if that works (I'm optimistic). Those steps are intended to take icu4c out of the build loop and fall back to using the MacOS native library.

kipcole9 commented 1 month ago

Aha, we overlapped our messages but arrived at the same place. I'll update the documentation to make this clearer - specially about how to influence which icu to use.

tjchambers commented 1 month ago

I tried the unlink and it fell back to the "X86_64" version of icu4c which failed the test. So yes it did go back to the native library - which in this case is not gonna work for me. May work on Sequoia if it is upgraded. So I am relinking for arm64 and going on.

Thanks again for patience in letting me work through this.

kipcole9 commented 1 month ago

Got it, most peculiar but happy you have a way forward. I'll update the docs and close the issue for now. Please re-open if you spot anything else unexpected.