[cfe-dev] cindex.py & cmake builds of clang

Tue May 15 00:38:57 PDT 2012

Hi Manuel,

I do not have commit access, so feel free to commit whenever you believe
we got sufficient feedback.

Cheers,
--
Arnaud de Grandmaison

On 05/08/2012 09:42 PM, Manuel Klimek wrote:
> On Tue, May 8, 2012 at 8:20 PM, Arnaud ALLARD DE GRANDMAISON
> <arnaud.allarddegrandmaison at parrot.com> wrote:
>> 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 :)
>
>> Thanks again for taking time to clarify the situation on windows with facts.
> No problem.
>
> Cheers,
> /Manuel
>
>> Cheers,
>> --
>> Arnaud
>> ________________________________________
>> From: Manuel Klimek [klimek at google.com]
>> Sent: Tuesday, May 08, 2012 7:56 PM
>> To: Arnaud ALLARD DE GRANDMAISON
>> Cc: David Röthlisberger; cfe-dev at cs.uiuc.edu
>> Subject: Re: [cfe-dev] cindex.py & cmake builds of clang
>>
>> On Tue, May 8, 2012 at 5:55 PM, Arnaud ALLARD DE GRANDMAISON
>> <arnaud.allarddegrandmaison at parrot.com> wrote:
>>> 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 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,
>>> --
>>> Arnaud
>>> ________________________________________
>>> From: Manuel Klimek [klimek at google.com]
>>> Sent: Tuesday, May 08, 2012 5:37 PM
>>> To: Arnaud ALLARD DE GRANDMAISON
>>> Cc: David Röthlisberger; cfe-dev at cs.uiuc.edu
>>> Subject: Re: [cfe-dev] cindex.py & cmake builds of clang
>>>
>>> On Tue, May 8, 2012 at 5:27 PM, Arnaud ALLARD DE GRANDMAISON
>>> <arnaud.allarddegrandmaison at parrot.com> wrote:
>>>> 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
>>>
>>>> Cheers,
>>>> --
>>>> Arnaud
>>>> ________________________________________
>>>> From: Manuel Klimek [klimek at google.com]
>>>> Sent: Tuesday, May 08, 2012 5:06 PM
>>>> To: Arnaud ALLARD DE GRANDMAISON
>>>> Cc: David Röthlisberger; cfe-dev at cs.uiuc.edu
>>>> Subject: Re: [cfe-dev] cindex.py & cmake builds of clang
>>>>
>>>> On Tue, May 8, 2012 at 5:00 PM, Arnaud ALLARD DE GRANDMAISON
>>>> <arnaud.allarddegrandmaison at parrot.com> wrote:
>>>>>> I'd expect now all platforms to load libclang.<suffix> (why the added
>>>>>> "lib" on windows?)
>>>>> That's all the point of this discussion :)
>>>>> 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
>>>>
>>>>> Cheers,
>>>>> --
>>>>> Arnaud
>>>>> ________________________________________
>>>>> From: Manuel Klimek [klimek at google.com]
>>>>> Sent: Tuesday, May 08, 2012 4:28 PM
>>>>> To: Arnaud ALLARD DE GRANDMAISON
>>>>> Cc: David Röthlisberger; cfe-dev at cs.uiuc.edu
>>>>> Subject: Re: [cfe-dev] cindex.py & cmake builds of clang
>>>>>
>>>>> On Tue, May 8, 2012 at 4:13 PM, Arnaud ALLARD DE GRANDMAISON
>>>>> <arnaud.allarddegrandmaison at parrot.com> wrote:
>>>>>> 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 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,
>>>>>> --
>>>>>> Arnaud
>>>>>> ________________________________________
>>>>>> From: Manuel Klimek [klimek at google.com]
>>>>>> Sent: Tuesday, May 08, 2012 12:11 PM
>>>>>> To: David Röthlisberger
>>>>>> Cc: Arnaud ALLARD DE GRANDMAISON; cfe-dev at cs.uiuc.edu
>>>>>> Subject: Re: [cfe-dev] cindex.py & cmake builds of clang
>>>>>>
>>>>>> On Mon, May 7, 2012 at 3:06 PM, David Röthlisberger <david at rothlis.net> wrote:
>>>>>>> On 7 May 2012, at 12:30, Arnaud de Grandmaison wrote:
>>>>>>>> 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.
>>>>>>> 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.
>>>>>> 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 ;)
>>>>>>
>>>>>> 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

-- 
Arnaud de Grandmaison
Senior CPU engineer
Business Unit Digital Tuner

Parrot S.A.
174, quai de Jemmapes
75010 Paris - France
Phone: +33 1 48 03 84 59