cmake: detect and use libc-provided iconv#4777
Conversation
While most systems provide a separate iconv library against which
applications can link, musl based systems do not provide such a library.
Instead, iconv functions are directly included in the C library. As our
current CMake module to locate the iconv library only checks whether a
library exists somewhere in the typical library directories, we will
never build libgit2 with libiconv support on such systems.
Extend the iconv module to also search whether libc provides iconv
functions, which we do by checking whether the `iconv_open` function
exists inside of libc. If this is the case, we will default to use the
libc provided one instead of trying to use a separate libiconv. While
this changes which iconv we use on systems where both libc and an
external libiconv exist, to the best of my knowledge common systems only
provide either one or the other.
Note that libiconv support in musl is held kind of basic. To quote musl
libc's page on functional differences from glibc [1]:
The iconv implementation musl is very small and oriented towards
being unobtrusive to static link. Its character set/encoding
coverage is very strong for its size, but not comprehensive like
glibc’s.
As we assume iconv to be a lot more capable than what musl provides,
some of our tests will fail if using iconv on musl-based platforms.
[1]: https://wiki.musl-libc.org/functional-differences-from-glibc.html
|
Seems reasonable to me. My only concern is that if we start running the iconv tests against various, possibly inferior, iconv implementations that we'll start getting reports of test failures with them. (You mentioned that musl's iconv is intentionally incomplete.) It would be a disappointment to get these reports when we don't actually even use iconv on those platforms. In our (non-test) production code, iconv is only used for filename translation on macOS, IIRC. (Am I misremembering something? I may be.) I think that I remember hearing suggestions about doing more with charset translations (and thus, iconv) at the last Git Contributor Summit, so I think that this makes sense but let's make sure that if we start failing then we don't spend time fixing tests that don't actually matter. |
|
I share your concerns. I decided to just go ahead though because
So, yeah. It's basically a "Let's see what happens. And if something happens, some things cannot be blamed on us and the others can trivially be fixed." |
👍 |
While most systems provide a separate iconv library against which
applications can link, musl based systems do not provide such a library.
Instead, iconv functions are directly included in the C library. As our
current CMake module to locate the iconv library only checks whether a
library exists somewhere in the typical library directories, we will
never build libgit2 with libiconv support on such systems.
Extend the iconv module to also search whether libc provides iconv
functions, which we do by checking whether the
iconv_openfunctionexists inside of libc. If this is the case, we will default to use the
libc provided one instead of trying to use a separate libiconv. While
this changes which iconv we use on systems where both libc and an
external libiconv exist, to the best of my knowledge common systems only
provide either one or the other.
Note that libiconv support in musl is held kind of basic. To quote musl
libc's page on functional differences from glibc 1:
As we assume iconv to be a lot more capable than what musl provides,
some of our tests will fail if using iconv on musl-based platforms.