cindex.py & cmake builds of clang

Hi all,

When building clang using cmake, libclang is actually named
'liblibclang' to avoid a file collision on Windows.

It seems to me the cindex.py should be able to cope with both flavours,
as it can not know for sure how the libclang library was generated.
The attached patch teach cindex.py to attempt to load 'liblibclang' in
case the loading of 'libclang' failed.

Cheers,

cindex.patch (1.16 KB)

I have run into this problem as well: One cannot build libclang on Unix using cmake if one wants to use the libclang python bindings.

I raised bug 12620 where I documented the history of this liblibclang nonsense:
http://llvm.org/bugs/show_bug.cgi?id=12620

A quick summary: For the sake of Windows Visual Studio users, where a library (clang.dll) cannot share the same base name as an executable (clang.exe), the cmake build system was changed to build libclang.dll, which in turn caused the Unix builds to produce liblibclang.so instead of libclang.so.

In response to "Why can't we produce libclang.dll on Windows and libclang.so on Unix?", Óscar Fuentes replied "This is not a technical problem. We know how to implement any of the choices under consideration."
http://clang-developers.42468.n3.nabble.com/Cannot-run-clang-regression-tests-with-cmake-td2961911.html#d1305831334000-246

So it seems that somebody already knows how to fix this, but no further work was done.

I really want the python bindings to work, so I prefer Arnaud's patch over no solution at all; but it feels so filthy to add code to work around defects in the build system, instead of fixing the build system.

David Röthlisberger.

The question is whether we consider that a problem with the build system:
When you refer to libclang, you usually put the "lib" into the name.
If you consider the "lib" to be part of the name, and want the
standard unix pattern -l<name> to work, it liblibclang is actually the
correct file name for the library, and the proposed patch is the
correct solution (and the configure build should probably be fixed,
too :wink:

If we don't consider the name of the library to be libclang, and you
want to write -lclang, then we can easily fix the cmake file - the
right fix in that case would in my opinion be to either change the
library prefix in general, or just the name of the dll on windows,
depending on what windows devs find more reasonable.

Cheers,
/Manuel

The question is whether we consider that a problem with the build system:
When you refer to libclang, you usually put the "lib" into the name.
If you consider the "lib" to be part of the name, and want the
standard unix pattern -l<name> to work, it liblibclang is actually the
correct file name for the library.

My understanding is that nobody changed the build system *on purpose* to build "liblibclang". It was an unintended side-effect of a change made for Windows.

There are other libraries I refer to in normal speech with the "lib" prefix: "libc", "libcurl". None of these are stored as "liblibc.so" or "liblibcurl.so" on the filesystem. The only exception I can think of is glib (libglib.so) but I think we'd all agree that is a different case to libclang's (because glib's extra "lib" is at the end, not at the front).

Cheers
Dave.

This is not per-se a build system problem : the build system is tweaked to avoid a collision occuring on windows (only) on some msvc intermediate files.

In my opinion, the best fix would be to change the libclang dll name only on windows. cindex.py is already 'decorating' the library name depending on the platform (platform specific suffix, ...), and we can handle here the windows specific 'decoration'. This would have the advantage that on a given platform, the 'libclang' would always have the same name, irrespective of the build system used.

The attached patch implements this, or at least is a step in the right direction.

I have attempted to make the Windows/cygwin & Windows/msvc cmake builds give the same library name, to simplify the library finding on the windows platform. I have unfortunately no way to check it myself on Windows.

Cheers,

cindex_cmake.patch (1.5 KB)

This is not per-se a build system problem : the build system is tweaked to avoid a collision occuring on windows (only) on some msvc intermediate files.

In my opinion, the best fix would be to change the libclang dll name only on windows. cindex.py is already 'decorating' the library name depending on the platform (platform specific suffix, ...), and we can handle here the windows specific 'decoration'. This would have the advantage that on a given platform, the 'libclang' would always have the same name, irrespective of the build system used.

The attached patch implements this, or at least is a step in the right direction.

I'd expect now all platforms to load libclang.<suffix> (why the added
"lib" on windows?)

Also, I think it would be more consistent to call the target 'clang'
and change the output name for windows instead of the other way around
(if David's explanation accurately describes the history of the
change).

Cheers,
/Manuel

I'd expect now all platforms to load libclang.<suffix> (why the added
"lib" on windows?)

That's all the point of this discussion :slight_smile:
Because of a filename collision on windows, caused by intermediate msvc files, the library filename was renamed libclang.dll (svn commit #129239)

Also, I think it would be more consistent to call the target 'clang'
and change the output name for windows instead of the other way around

clang is already a target : that's the compiler driver (tools/driver/CMakelist.txt), so libclang sounds ok for describing the library target.

Cheers,

I'd expect now all platforms to load libclang.<suffix> (why the added
"lib" on windows?)

That's all the point of this discussion :slight_smile:
Because of a filename collision on windows, caused by intermediate msvc files, the library filename was renamed libclang.dll (svn commit #129239)

--- bindings/python/clang/cindex.py (revision 156318)
+++ bindings/python/clang/cindex.py (working copy)
@@ -74,7 +74,7 @@
     if name == 'Darwin':
         return cdll.LoadLibrary('libclang.dylib')
     elif name == 'Windows':
- return cdll.LoadLibrary('libclang.dll')
+ return cdll.LoadLibrary('liblibclang.dll')
     else:
         return cdll.LoadLibrary('libclang.so')

Why would the library be called "liblibclang.dll" on Windows after the change?
Am I somehow misreading this patch?

Also, I think it would be more consistent to call the target 'clang'
and change the output name for windows instead of the other way around

clang is already a target : that's the compiler driver (tools/driver/CMakelist.txt), so libclang sounds ok for describing the library target.

Good point. This is definitely still confusing, but I don't have a
better proposal ...

Cheers,
/Manuel

CMake is prefixing the library output name with 'lib'. By default, in cmake, the output name is the target name.
The default is overriden with the set_target_properties to avoid this double 'lib' prefix on all platforms but windows --- because of the collision.
Confusing, isn't it ?

Cheers,

CMake is prefixing the library output name with 'lib'. By default, in cmake, the output name is the target name.
The default is overriden with the set_target_properties to avoid this double 'lib' prefix on all platforms but windows --- because of the collision.
Confusing, isn't it ?

I thought the problem was initially the collision of clang.exe and
clang.dll / clang.lib (for the static lib or dll import lib) on
windows, which by default does not have a library prefix. Thus, I
would expect having a target name 'libclang' on windows to lead to
'libclang.dll' and 'libclang.lib' on windows.

Cheers,
/Manuel

If we step back from the cmake build details, the situation we have today is :
- configure build :
    - supports : Darwin + Linux
    - produces : libclang.platform_suffix
- cmake build :
    - supports : Darwin + Linux + Windows + IDEs
    - produces : liblibclang.platform_suffix

I both cases, the builds are fine in the sense that all executables are finding the right library --- after all that's the purpose of a build system.

The only issue is with cindex.py which should cope with both build flavours, especially since it can be shipped with an application separately from the libclang package (the vim plugin 'clang_complete' is such an example).

My first patch was an attempt to accept both build flavours in the state they are today. The second patch is trying to have the cmake build behave like the configure build on all platforms but windows --- which needs a special treatment because of the collision.

I hope it clarify things.

Cheers,

Hi Arnaud

Can you please confirm whether your 2nd patch causes cmake to build "libclang.dll" on Windows and "libclang.so" (or .dylib etc) on Unix-like platforms. (In other words, *not* "liblibclang" on *any* platform).

Thanks,
Dave.

David,

I can only test the patch on linux. I can thus only confirm the behaviour on linux. The intended behaviour is :
- Darwin + linux : libclang.suffix (i.e. configure and cmake name the library the same way)
- Windows : liblibclang.dll Given the history and my cmake-foo, I do not think we can avoid this.

Although not ideal, it provides a consistent naming on a given platform, irrespective of the build system. cindex.py can then handle the platform specific bits, as it already does today.

Cheers,

If we step back from the cmake build details, the situation we have today is :
- configure build :
- supports : Darwin + Linux
- produces : libclang.platform_suffix
- cmake build :
- supports : Darwin + Linux + Windows + IDEs
- produces : liblibclang.platform_suffix

I think this is incorrect. I just installed VS & cmake and built clang
trunk, and I find a libclang.dll, libclang.ilk and libclang.pdb in my
bin directory. This is also what I expected: by default cmake does not
prepend a library prefix on the windows platform.

Cheers,
/Manuel

I may be wrong on the windows aspect. I can not test on it, but thanks for taking some time to do it.
I tried to guess --- wrongly it seems --- from the commit history what was happening.

However, on linux, with a top of tree checkout of llvm+clang+compiler_rt :

cmake -DCMAKE_BUILD_TYPE:STRING=Release -DLLVM_TARGETS_TO_BUILD:STRING=X86 -DLLVM_BUILD_EXAMPLES:BOOL=0 -DBUILD_SHARED_LIBS:BOOL=1 ../llvm
make

ls -l lib/
...
lrwxrwxrwx 1 arnaud users 18 May 8 20:09 liblibclang.so -> liblibclang.so.3.2
-rwxr-xr-x 1 arnaud users 692601 May 8 20:09 liblibclang.so.3.2
...

ldd bin/c-index-test
...
      liblibclang.so.3.2 => ..../build.tmp/lib/liblibclang.so.3.2 (0x00007f8ec4184000)

So my 2nd patch should be OK if we drop the cindex.py part : I tried to not modify the cmake behaviour on windows.

Thanks again for taking time to clarify the situation on windows with facts.

Cheers,

I may be wrong on the windows aspect. I can not test on it, but thanks for taking some time to do it.
I tried to guess --- wrongly it seems --- from the commit history what was happening.

However, on linux, with a top of tree checkout of llvm+clang+compiler_rt :

cmake -DCMAKE_BUILD_TYPE:STRING=Release -DLLVM_TARGETS_TO_BUILD:STRING=X86 -DLLVM_BUILD_EXAMPLES:BOOL=0 -DBUILD_SHARED_LIBS:BOOL=1 ../llvm
make

ls -l lib/
...
lrwxrwxrwx 1 arnaud users 18 May 8 20:09 liblibclang.so -> liblibclang.so.3.2
-rwxr-xr-x 1 arnaud users 692601 May 8 20:09 liblibclang.so.3.2
...

ldd bin/c-index-test
...
liblibclang.so.3.2 => ..../build.tmp/lib/liblibclang.so.3.2 (0x00007f8ec4184000)

So my 2nd patch should be OK if we drop the cindex.py part : I tried to not modify the cmake behaviour on windows.

Yep, I tested it on windows and it seems to work as expected. We might
still want somebody who's more of a windows dev to give feedback :slight_smile:

Thanks again for taking time to clarify the situation on windows with facts.

No problem.

Cheers,
/Manuel

I not sure I fully understand all the details of this issue, but I
think a way to solve the clang.dll collission issue would be to simply
rename libclang.

IMHO the 'libclang.so' name isn't very descriptive, to me it more
sounds like the full C++ clang api. The lib contains the Index.h C
api, so why couldn't it be named libcindex.so (or cindex.dll) or
libclangindex.so or something similar. (libclang-c.so if other parts
than the index is expected to be added).

The problem with renaming would ofcourse be existing integrations
linking to it, but for darwin/linux a symlink from libclang.so ->
libcindex.so could be created.

Just my 0.02

anders

We can always consider this at a later date but right now we are not going to rename libclang because of a build system bug that we already know how to fix.

Here is a summary of my understanding of the matter:

1. libclang python bindings don't work on Unixes if libclang was build with cmake.
2. Arnaud submitted a patch to the cmake build system (his second patch in this thread, sent on 2012-05-08). Arnaud tested it on Linux.
3. Manuel tested it on Windows and confirmed that it worked, but he is not a heavy Windows user.
4. WE ARE WAITING FOR WINDOWS USERS TO TEST THE PATCH -- PLEASE HELP. We need you to confirm that with this patch it still builds libclang.dll on Windows (instead of clang.dll, which causes a conflict with clang.exe).

This is an issue that has caused repeated confusion over the last year.† An entire year ago people were claiming to have a solution but no concrete patch was ever submitted. Now Arnaud has submitted a patch and it would be nice to give this the final push so we can get it committed once and for all.

† See the mailing list threads linked to in http://llvm.org/bugs/show_bug.cgi?id=12620#c0

Thanks,
Dave.

When building clang using cmake, libclang is actually named
'liblibclang' to avoid a file collision on Windows.

I not sure I fully understand all the details of this issue, but I
think a way to solve the clang.dll collission issue would be to simply
rename libclang.

We can always consider this at a later date but right now we are not going to rename libclang because of a build system bug that we already know how to fix.

Here is a summary of my understanding of the matter:

1. libclang python bindings don't work on Unixes if libclang was build with cmake.
2. Arnaud submitted a patch to the cmake build system (his second patch in this thread, sent on 2012-05-08). Arnaud tested it on Linux.
3. Manuel tested it on Windows and confirmed that it worked, but he is not a heavy Windows user.
4. WE ARE WAITING FOR WINDOWS USERS TO TEST THE PATCH -- PLEASE HELP. We need you to confirm that with this patch it still builds libclang.dll on Windows (instead of clang.dll, which causes a conflict with clang.exe).

I have verified that it builds libclang.dll on Windows - I want the
opinion of a windows dev (cc'ed Michael) because it looks to me like
in the thread that you cite there were concerns raised about how
windows devs integrate libclang into their projects - so it was more
about the change in the CMakeLists.txt then about any possible side
effects.

Cheers,
/Manuel

When building clang using cmake, libclang is actually named
'liblibclang' to avoid a file collision on Windows.

I not sure I fully understand all the details of this issue, but I
think a way to solve the clang.dll collission issue would be to simply
rename libclang.

We can always consider this at a later date but right now we are not going to rename libclang because of a build system bug that we already know how to fix.

Here is a summary of my understanding of the matter:

1. libclang python bindings don't work on Unixes if libclang was build with cmake.
2. Arnaud submitted a patch to the cmake build system (his second patch in this thread, sent on 2012-05-08). Arnaud tested it on Linux.
3. Manuel tested it on Windows and confirmed that it worked, but he is not a heavy Windows user.
4. WE ARE WAITING FOR WINDOWS USERS TO TEST THE PATCH -- PLEASE HELP. We need you to confirm that with this patch it still builds libclang.dll on Windows (instead of clang.dll, which causes a conflict with clang.exe).

I have verified that it builds libclang.dll on Windows - I want the
opinion of a windows dev (cc'ed Michael) because it looks to me like
in the thread that you cite there were concerns raised about how
windows devs integrate libclang into their projects - so it was more
about the change in the CMakeLists.txt then about any possible side
effects.

Cheers,
/Manuel

I have not tested this patch, nor am I familiar with libclang. But as
a CMake user that links to external libraries, as long as
find_package(clang) works on all platforms, it's good.

- Michael Spencer