-
-
Notifications
You must be signed in to change notification settings - Fork 1.1k
Fix windows build with icu tag #1017
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
Conversation
In mingw64, [icu][] comes with `libicuin` instead of `libicui18n` library. [icu]: https://packages.msys2.org/package/mingw-w64-x86_64-icu?repo=mingw64
Use variadic arguments in windows implementation of `CommandFromString`. Check for windows root directory while searching for notebooks, this fixes an infinite loop during startup notebook discovery. Fixes zk-org#139. Please note that `go build -tags "fts5 icu"` will require a patch from `go-sqlite3` [here][] with a msys2/mingw64 env. [here]: mattn/go-sqlite3#1017
Codecov Report
@@ Coverage Diff @@
## master #1017 +/- ##
==========================================
- Coverage 46.17% 46.04% -0.14%
==========================================
Files 11 11
Lines 1490 1490
==========================================
- Hits 688 686 -2
- Misses 666 667 +1
- Partials 136 137 +1
Continue to review full report at Codecov.
|
#cgo darwin LDFLAGS: -L/usr/local/opt/icu4c/lib | ||
#cgo openbsd LDFLAGS: -lsqlite3 | ||
#cgo darwin LDFLAGS: -L/usr/local/opt/icu4c/lib -licui18n | ||
#cgo openbsd LDFLAGS: -lsqlite3 -licui18n |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@mattn Do you know why we are using -lsqlite3
here? I would think that would cause issues since we'd be simultaneously dynamic linking against the system and statically compiling our own version.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Seems to be related on #595
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Hmm, I cannot test this since I don't have access to an openbsd system. And it's not clear if it was intentional. I will file a separate issue to investigate.
In mingw64, icu comes with
libicuin
instead oflibicui18n
library.