find_clang.cr finds different versions of clang & llvm-config

[ZaWertun]

There maybe several clang & llvm versions installed on user computer.
For example I have both llvm/clang 6 and 10.
After running find_clang.cr I've got this variables (from Makefile.variables):

CLANG_BINARY := /bin/clang++-10
CLANG_INCLUDES := -I/usr/include -I/include/c++/10
CLANG_LIBS := -Wl,--start-group -lclang -lclang-cpp -Wl,--start-group -Wl,--start-group -lLLVM-10 -lLLVM-10.0.0 -lLLVM -Wl,--start-group
LLVM_CONFIG_BINARY := /bin/llvm-config-6.0-64
LLVM_VERSION := 6
LLVM_VERSION_FULL := 6.0.1
LLVM_CXX_FLAGS := -I/usr/lib64/llvm6.0/include -O2 -g -pipe -Wall -D__STDC_LIMIT_MACROS
LLVM_LD_FLAGS := -L/usr/lib64/llvm6.0/lib
LLVM_LIBS := -Wl,--start-group -lLLVM-10 -lLLVM-10.0.0 -lLLVM -Wl,--start-group

Clang 10 is linked against llvm-10 so I get strange errors when running crystal spec.
Also /bin/llvm-config points to the version 10 but it's not selected.

[docelic]

Interesting, gonna look into this. The llvm binary is detected by first looking into the paths discovered from examining clang, so I assumed it would always pick the right one. Will check.

[ZaWertun]

Maybe some check needed that clang version matches llvm version.

[docelic]

Yes, the finder supports this, so unless a more direct approach is found, restricting the matches to the same version would sure work.

[docelic]

@ZaWertun try now please. The clang-discovered paths were added to the beginning of the search list as expected, but I missed to replace "lib|include" in them with "bin". The last commit adds this back.

[ZaWertun]

There is not much change:

CLANG_BINARY := /usr/lib64/ccache/clang++
CLANG_INCLUDES := -I/usr/include -I/usr/include/c++/10
CLANG_LIBS := -Wl,--start-group -lclang -lclang-cpp -Wl,--start-group -Wl,--start-group -lLLVM-10 -lLLVM-10.0.0 -lLLVM -Wl,--start-group
LLVM_CONFIG_BINARY := /usr/bin/llvm-config-6.0-64
LLVM_VERSION := 6
LLVM_VERSION_FULL := 6.0.1
LLVM_CXX_FLAGS := -I/usr/lib64/llvm6.0/include -O2 -g -pipe -Wall -D__STDC_LIMIT_MACROS
LLVM_LD_FLAGS := -L/usr/lib64/llvm6.0/lib
LLVM_LIBS := -Wl,--start-group -lLLVM-10 -lLLVM-10.0.0 -lLLVM -Wl,--start-group

But replacing default OPTIONS[:llvm_config_pattern]="llvm-config" with "llvm-config" - helps.

[docelic]

Hm let's also add the version check then. Will have it in a couple minutes.
(Yes, doing a fixed lookup for "llvm-config" would work in your case, but sometimes the binary is named "llvm-config-7" etc.)

[ZaWertun]

Hm let's also add the version check then. Will have it in a couple minutes.
(Yes, doing a fixed lookup for "llvm-config" would work in your case, but sometimes the binary is named "llvm-config-7" etc.)

It's better to check both llvm-config and llvm-config*. Not only last.

[ZaWertun]

Actually it's strange that llvm-config is not found by pattern llvm-config*, because it must match.

[docelic]

In find_clang's find_llvm_config_binary, can you add a p paths.join " " so that we see which paths are being searched, and then possibly do in the shell:

find <those paths> -name 'llvm-config*'

From this we'd have all the info needed to determine why it is/isn't matching.

[ZaWertun]

Paths:

["/lib/gcc/x86_64-redhat-linux/10",
 "/lib64",
 "/usr/lib64",
 "/lib64",
 "/usr/lib64",
 "/bin",
 "/usr/bin",
 "/bin",
 "/usr/bin"]

All llvm-config executables & links are living in the /usr/bin (/bin now is linked to the /usr/bin):

$ ls -la /usr/bin/llvm-config*
lrwxrwxrwx 1 root root     29 апр  6 22:10 /usr/bin/llvm-config -> /etc/alternatives/llvm-config*
lrwxrwxrwx 1 root root     34 янв 29 16:18 /usr/bin/llvm-config-6.0-64 -> /usr/lib64/llvm6.0/bin/llvm-config*
-rwxr-xr-x 1 root root 129072 апр  2 11:15 /usr/bin/llvm-config-64*

[ZaWertun]

Looks like all links to the binary files was skipped somehow.

[ZaWertun]

Nah, maybe just llvm-config-6.0-64 picked first.

$ sudo find / -xdev -name 'llvm-config*' -executable
/etc/alternatives/llvm-config
/usr/lib64/llvm6.0/bin/llvm-config
/usr/bin/llvm-config-6.0-64
/usr/bin/llvm-config-64
/usr/bin/llvm-config

[docelic]

Eh yes, right, so it didn't find any llvm binaries in the specific directories, and then when it got to the global/common ones like /usr/bin where it picked the first one >= 6.0.0.

Yes, in this case we'll need to add a version check. For now it will look for the exact version as clang reports. Will have it in a couple minutes. (Will also re-check what the old version of the code was doing.)

But I am unsure if this is the right thing to do in general. If someone knows why the version could be different, please chip in. Thanks.

[docelic]

Looking at the old code, it didn't have this case handled either. It incidentally worked because it was only looking for llvm-config (without *) and in most cases this was the exact/good version.

Will try to implement the min version check. If that fails will resort to just checking for llvm-config.

[kalinon]

On OSX we are able to have multiple llvm installs as well using brew:

$ brew list | grep llvm
llvm
llvm@6
llvm@8

This is why i am more in favor of strictly using what ever is passed in as --clang or found with a llvm-config call. Then using the llvm-config to define everything from there. Infact i would almost say we should be using llvm-config to define the clang binary path as well.

$ llvm-config --bindir
/usr/local/Cellar/llvm/10.0.0_1/bin

Which gives us /usr/local/Cellar/llvm/10.0.0_1/bin/clang++. Can be on the user to ensure the correct llvm-config is defined and/or passed.

[docelic]

Hm @kalinon indeed, ok, that brings us closer to that idea of forcing the good value in path, and figuring out everything from there. Will implement, thanks.

[docelic]

@ZaWertun Hey please try now. The updated find_clang.cr still does not search by version, but among other changes now it uses both the clang++ and llvm-config binaries which are detected by CMake itself (when run through make).

[ZaWertun]

Still /bin/llvm-config-6.0-64 is selected first.
But /usr/bin/llvm-config tried first, version is OK, but somehow it's not accepted.

[ZaWertun]

$ BINDGEN_DYNAMIC=1 crystal clang/find_clang.cr
Searching for binary `llvm-config` or `llvm-config-*` in PATH. Minimum version 6.0.0
Using llvm-config binary in "/bin/llvm-config-6.0-64".
Searching for binary clang++ or clang++-* in /usr/lib64/llvm6.0/bin. Minimum version 6.0.0
You're missing the LLVM and/or Clang executables or development libraries.

If you've installed the binaries in a non-standard location:
  1) Make sure that `llvm-config` or `llvm-config-*` is set with --llvm_config FILE or is in PATH. The first binary found which satisfies version will be used.
  2) In rare cases if clang++ isn't found or is incorrect, you can also specify it with --clang FILE.

If your distro does not support static libraries like openSUSE then set env var BINDGEN_DYNAMIC=1.
This will use .so instead of .a libraries during linking.

If you are missing the packages, please install them:
  ArchLinux: pacman -S llvm clang gc libyaml
  Ubuntu: apt install clang-4.0 libclang-4.0-dev zlib1g-dev libncurses-dev libgc-dev llvm-4.0-dev libpcre3-dev
  CentOS: yum install crystal libyaml-devel gc-devel pcre-devel zlib-devel clang-devel
  openSUSE: zypper install llvm clang libyaml-devel gc-devel pcre-devel zlib-devel clang-devel ncurses-devel
  Mac OS: brew install crystal bdw-gc gmp libevent libxml2 libyaml llvm

And cmake finds correct versions:

$ BINDGEN_DYNAMIC=1 cmake .
...
-- Found LLVM: 10.0.0
-- Using LLVMConfig.cmake in: /usr/lib64/cmake/llvm
-- LLVM Major version: 10
-- Setting std version: c++14
-- LLVM Tools Dir: /usr/bin
-- Using clang++ exec: /usr/lib64/ccache/clang++
-- Using llvm-config exec: /usr/bin/llvm-config
...

[ZaWertun]

Oh man...

      def sorted_candidates : Array({String, String})
        list = @candidates.sort_by!(&.first)

        if @config.prefer.lowest?
          res = list
        else
          res = list.reverse
        end

        pp res
        res
      end

[{"6.0.1", "/bin/llvm-config-6.0-64"},
 {"6.0.1", "/usr/bin/llvm-config-6.0-64"},
 {"10.0.0", "/bin/llvm-config-64"},
 {"10.0.0", "/usr/bin/llvm-config-64"},
 {"10.0.0", "/bin/llvm-config"},
 {"10.0.0", "/usr/bin/llvm-config"}]

It just takes lowest version. Why????

I think it will be better to iterate versions from lower to highest and check corresponding clang++ for it.
Or better I'll remove those pesky llvm-6.0.

[docelic]

Yes, but this is a great test while you have the old llvm. Can you run this in the usual way via cmake .; make and not via find_clang directly?

When called through make, make will supply to find_clang.cr the exact llvm-config and clang++ binaries to use. So in that case no autodetection should be happening in find_clang, and it should use the same binaries that CMake found.

[kalinon]

a git clean --ffxd will wipe all the caches (and any other non-tracked files). helpful in cleaning before cmake .

[ZaWertun]

Works fine after cleaning all things up and running BINDGEN_DYNAMIC=1 cmake . && make.

[docelic]

Closing as it seems resolved. Please reopen if anything remains.