[llvm] r316064 - Fix the incorrect detection of ICONV_LIBRARY_PATH

[Hans Wennborg via llvm-commits]

I've reverted this in r317517 as it (silently) broke our builds, i.e.
it was no longer setting LLVM_LIBXML2_ENABLED.

Questions inline.

On Tue, Oct 17, 2017 at 11:26 PM, Serguei Katkov via llvm-commits
<llvm-commits at lists.llvm.org> wrote:
> Author: skatkov
> Date: Tue Oct 17 23:26:39 2017
> New Revision: 316064
>
> URL: http://llvm.org/viewvc/llvm-project?rev=316064&view=rev
> Log:
> Fix the incorrect detection of ICONV_LIBRARY_PATH
>
> This is introduced in rL308711.
> Check for c library is incorrect here just because libc will be found always
> and it does not mean that iconv is presented.
>
> Thank to Andrew Krasny for narrowing down the root cause.

What this was the root cause of? I couldn't find any bug, and no
discussion on the code review or mailing list.

> Reviewers: ecbeckmann
> Reviewed By: ecbeckmann
> Subscribers: mgorny, llvm-commits
> Differential Revision: https://reviews.llvm.org/D38875
>
> Modified:
>     llvm/trunk/cmake/config-ix.cmake
>
> Modified: llvm/trunk/cmake/config-ix.cmake
> URL: http://llvm.org/viewvc/llvm-project/llvm/trunk/cmake/config-ix.cmake?rev=316064&r1=316063&r2=316064&view=diff
> ==============================================================================
> --- llvm/trunk/cmake/config-ix.cmake (original)
> +++ llvm/trunk/cmake/config-ix.cmake Tue Oct 17 23:26:39 2017
> @@ -157,7 +157,7 @@ if( NOT PURE_WINDOWS AND NOT LLVM_USE_SA
>      set(HAVE_TERMINFO 0)
>    endif()
>
> -  find_library(ICONV_LIBRARY_PATH NAMES iconv libiconv libiconv-2 c)
> +  find_library(ICONV_LIBRARY_PATH NAMES iconv libiconv libiconv-2)

On Linux, I believe libc usually provides iconv() which is why the
check was written that way.

>    set(LLVM_LIBXML2_ENABLED 0)
>    set(LIBXML2_FOUND 0)
>    if((LLVM_ENABLE_LIBXML2) AND ((CMAKE_SYSTEM_NAME MATCHES "Linux") AND (ICONV_LIBRARY_PATH) OR APPLE))

But this raises lots of questions.

Why do we need to check for iconv here? Isn't it enough that we find
libxml2; i.e., if we find it, can't we assume that it's usable?

And why is the iconv() check specific to Linux? Is it not needed on
Mac, and what if the user is on FreeBSD or something else -- no
libxml2 for them?

Does anyone know what's going on here?

Thanks,
Hans