Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Include icuio lib #36

Merged
merged 1 commit into from
Jan 27, 2022
Merged

Include icuio lib #36

merged 1 commit into from
Jan 27, 2022

Conversation

LukaHorvat
Copy link
Contributor

This seems to fix issues like haskell/lsp#56 and haskell/lsp#41

@gzh
Copy link

gzh commented Nov 14, 2018

Not only this seems to fix those issues. This seems to be the only correct link options on MinGW.

Because cbits/text_icu.c references ucnv_getMaxCharSize, and the latter is exported from libicuio.dll.

$ grep -r ucnv_getMaxCharSize /mingw32/bin
Binary file /mingw32/bin/libicuio62.dll matches
Binary file /mingw32/bin/libicuuc62.dll matches

$ nm /mingw32/bin/libicuio62.dll| grep ucnv_getMaxCharSize
6a391310 I __imp__ucnv_getMaxCharSize_62

$ nm /mingw32/bin/libicuuc62.dll| grep ucnv_getMaxCharSize

$

I can confirm this patch does work. Invented quite the same fix by myself and was going to submit an identical PR, but then discovered this one. I've tested this on MinGW 32-bit (just updated with pacman -Syu), ICU 6.2, GHC 8.4.3.

@daniel-chambers
Copy link

I'd love to see this PR merged! 😃 Without this fix, this library causes some builds to fail on Windows. See my experience over at commercialhaskell/stack#4415, where they kindly pointed me to this fix.

@vshabanov vshabanov merged commit f84dfb7 into haskell:master Jan 27, 2022
@vshabanov
Copy link
Collaborator

Could you please test whether the latest sources are building fine on Windows? Many PRs were merged, but I have no Windows machine to test how the library works there.

And please run cabal test to see whether all tests are passing.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants