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

configure.ac: test for iconv_open with #include <iconv.h> #3942

Merged
merged 1 commit into from
Feb 16, 2020

Conversation

stapelberg
Copy link
Member

The previously used AC_SEARCH_LIBS uses AC_LANG_CALL, which is discouraged
because it intentionally violates the C type system: it has no way of specifying
function parameters, so it does not include the required headers to suppress
type warnings:
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Generating-Sources.html

The new code does what AC_SEARCH_LIBS does, but uses AC_LANG_PROGRAM instead of
AC_LANG_CALL. It explicitly includes iconv.h and calls iconv_open with the
correct number and type of arguments.

This fixes compilation on systems where libiconv’s iconv.h is discovered first,
but glibc also provides iconv. Previously, this would result in configure
discovering glibc’s iconv, and make failing to compile because -liconv wasn’t
specified, but iconv.h pulled in libiconv.

@Airblader
Copy link
Member

@stapelberg Do we want to put this in before the release?

The previously used AC_SEARCH_LIBS uses AC_LANG_CALL, which is discouraged
because it intentionally violates the C type system: it has no way of specifying
function parameters, so it does not include the required headers to suppress
type warnings:
https://www.gnu.org/savannah-checkouts/gnu/autoconf/manual/autoconf-2.69/html_node/Generating-Sources.html

The new code does what AC_SEARCH_LIBS does, but uses AC_LANG_PROGRAM instead of
AC_LANG_CALL. It explicitly includes iconv.h and calls iconv_open with the
correct number and type of arguments.

This fixes compilation on systems where libiconv’s iconv.h is discovered first,
but glibc also provides iconv. Previously, this would result in configure
discovering glibc’s iconv, and make failing to compile because -liconv wasn’t
specified, but iconv.h pulled in libiconv.
@stapelberg
Copy link
Member Author

@stapelberg Do we want to put this in before the release?

Yes please

@Airblader Airblader merged commit 2914c7f into i3:next Feb 16, 2020
@stapelberg stapelberg deleted the iconv branch February 19, 2020 19:55
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.

None yet

2 participants