Juergen Ributzka
2014-Feb-13 02:47 UTC
[LLVMdev] [llvm] r201072 - [CMake] Introduce llvm_add_library().
No, it has the wrong value. I tried it with cmake 2.8.9 and 2.8.12.2. Both of them set the variable to “.so”. On Feb 12, 2014, at 5:29 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:> Juergen, > > Thanks to let me know. I guess r200762 (and r200763) might affect. > > Although I won't check this on darwin box, I suspect the line in > HandleLLVMOptions.cmake; > > set(LLVM_PLUGIN_EXT ${CMAKE_SHARED_MODULE_SUFFIX}) > > Does it have expected value? > > 2014-02-13 10:11 GMT+09:00 Juergen Ributzka <juergen at apple.com>: >> Hi Takumi, >> >> I am not sure if it this change, but recently we started to build >> LLVMHello.so and BugpointPasses.so on OS X. A few bugpoint tests are >> failing, because they are looking for a dylib that doesn't exist. >> >> Could you please take a look? >> >> Thanks >> >> -Juergen >> >> On Feb 10, 2014, at 2:34 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote: >> >> Steve, excuse me to respond you partially. >> >> 2014-02-10 18:54 GMT+09:00 Stephen Kelly <steveire at gmail.com>: >> >> NAKAMURA Takumi wrote: >> >> [CMake] Introduce llvm_add_library(). >> >> >> I recommend moving away from wrappers like this. They indicate that either >> CMake is not providing the interfaces needed, or not propagating them, or >> that they exist but are not used. >> >> Such wrappers don't parse the arguments in the same way as the wrapped >> command etc. Wrappers are not good API proxies. Additionally, you put people >> who know cmake already at a disadvantage because you make the code look like >> 'not cmake code' by using llvm_add_library instead of add_library, the cmake >> command. Making that wrapper macro also invoke target_link_libraries looks >> odd to someone familiar with cmake. >> >> I don't fully know what this wrapper is needed for, but I looked at it a >> bit. >> >> if(ARG_MODULE) >> set_property(TARGET ${name} PROPERTY SUFFIX ${LLVM_PLUGIN_EXT}) >> endif() >> >> if(ARG_SHARED) >> if (MSVC) >> set_target_properties(${name} >> PROPERTIES >> IMPORT_SUFFIX ".imp") >> endif () >> endif() >> >> >> CMake provides CMAKE_SHARED_MODULE_SUFFIX and CMAKE_IMPORT_LIBRARY_SUFFIX. >> Can you set those at directory scope instead of setting the target property >> in the macro? >> >> >> Reasonable. They could be put into common configurator, HandleLLVMOptions. >> For now in r201072, I simply gather common logics around there. >> I'd be happy if llvm_add_library became simpler, and I will work on. >> >> Anyway, just a note to consider moving instead in a direction of fewer >> wrappers, rather than more. I'm not familiar enough with the llvm >> buildsystem to know why you're doing some of the things you're doing, but if >> you tell us about the gaps in the cmake offering we can try to fill them. >> >> Thanks, >> >> Steve. >> >> _______________________________________________ >> >> LLVM Developers mailing list >> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >> >>
NAKAMURA Takumi
2014-Feb-13 16:00 UTC
[LLVMdev] [llvm] r201072 - [CMake] Introduce llvm_add_library().
Juergen, Tweaked in r201316. I expect it would help you. ...Takumi 2014-02-13 11:47 GMT+09:00 Juergen Ributzka <juergen at apple.com>:> No, it has the wrong value. I tried it with cmake 2.8.9 and 2.8.12.2. Both of them set the variable to ".so". > > On Feb 12, 2014, at 5:29 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote: > >> Juergen, >> >> Thanks to let me know. I guess r200762 (and r200763) might affect. >> >> Although I won't check this on darwin box, I suspect the line in >> HandleLLVMOptions.cmake; >> >> set(LLVM_PLUGIN_EXT ${CMAKE_SHARED_MODULE_SUFFIX}) >> >> Does it have expected value? >> >> 2014-02-13 10:11 GMT+09:00 Juergen Ributzka <juergen at apple.com>: >>> Hi Takumi, >>> >>> I am not sure if it this change, but recently we started to build >>> LLVMHello.so and BugpointPasses.so on OS X. A few bugpoint tests are >>> failing, because they are looking for a dylib that doesn't exist. >>> >>> Could you please take a look? >>> >>> Thanks >>> >>> -Juergen >>> >>> On Feb 10, 2014, at 2:34 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote: >>> >>> Steve, excuse me to respond you partially. >>> >>> 2014-02-10 18:54 GMT+09:00 Stephen Kelly <steveire at gmail.com>: >>> >>> NAKAMURA Takumi wrote: >>> >>> [CMake] Introduce llvm_add_library(). >>> >>> >>> I recommend moving away from wrappers like this. They indicate that either >>> CMake is not providing the interfaces needed, or not propagating them, or >>> that they exist but are not used. >>> >>> Such wrappers don't parse the arguments in the same way as the wrapped >>> command etc. Wrappers are not good API proxies. Additionally, you put people >>> who know cmake already at a disadvantage because you make the code look like >>> 'not cmake code' by using llvm_add_library instead of add_library, the cmake >>> command. Making that wrapper macro also invoke target_link_libraries looks >>> odd to someone familiar with cmake. >>> >>> I don't fully know what this wrapper is needed for, but I looked at it a >>> bit. >>> >>> if(ARG_MODULE) >>> set_property(TARGET ${name} PROPERTY SUFFIX ${LLVM_PLUGIN_EXT}) >>> endif() >>> >>> if(ARG_SHARED) >>> if (MSVC) >>> set_target_properties(${name} >>> PROPERTIES >>> IMPORT_SUFFIX ".imp") >>> endif () >>> endif() >>> >>> >>> CMake provides CMAKE_SHARED_MODULE_SUFFIX and CMAKE_IMPORT_LIBRARY_SUFFIX. >>> Can you set those at directory scope instead of setting the target property >>> in the macro? >>> >>> >>> Reasonable. They could be put into common configurator, HandleLLVMOptions. >>> For now in r201072, I simply gather common logics around there. >>> I'd be happy if llvm_add_library became simpler, and I will work on. >>> >>> Anyway, just a note to consider moving instead in a direction of fewer >>> wrappers, rather than more. I'm not familiar enough with the llvm >>> buildsystem to know why you're doing some of the things you're doing, but if >>> you tell us about the gaps in the cmake offering we can try to fill them. >>> >>> Thanks, >>> >>> Steve. >>> >>> _______________________________________________ >>> >>> LLVM Developers mailing list >>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >>> >>> >
Juergen Ributzka
2014-Feb-13 19:19 UTC
[LLVMdev] [llvm] r201072 - [CMake] Introduce llvm_add_library().
Thanks Takumi - It works now :D On Feb 13, 2014, at 8:00 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote:> Juergen, > > Tweaked in r201316. I expect it would help you. > > ...Takumi > > 2014-02-13 11:47 GMT+09:00 Juergen Ributzka <juergen at apple.com>: >> No, it has the wrong value. I tried it with cmake 2.8.9 and 2.8.12.2. Both of them set the variable to ".so". >> >> On Feb 12, 2014, at 5:29 PM, NAKAMURA Takumi <geek4civic at gmail.com> wrote: >> >>> Juergen, >>> >>> Thanks to let me know. I guess r200762 (and r200763) might affect. >>> >>> Although I won't check this on darwin box, I suspect the line in >>> HandleLLVMOptions.cmake; >>> >>> set(LLVM_PLUGIN_EXT ${CMAKE_SHARED_MODULE_SUFFIX}) >>> >>> Does it have expected value? >>> >>> 2014-02-13 10:11 GMT+09:00 Juergen Ributzka <juergen at apple.com>: >>>> Hi Takumi, >>>> >>>> I am not sure if it this change, but recently we started to build >>>> LLVMHello.so and BugpointPasses.so on OS X. A few bugpoint tests are >>>> failing, because they are looking for a dylib that doesn't exist. >>>> >>>> Could you please take a look? >>>> >>>> Thanks >>>> >>>> -Juergen >>>> >>>> On Feb 10, 2014, at 2:34 AM, NAKAMURA Takumi <geek4civic at gmail.com> wrote: >>>> >>>> Steve, excuse me to respond you partially. >>>> >>>> 2014-02-10 18:54 GMT+09:00 Stephen Kelly <steveire at gmail.com>: >>>> >>>> NAKAMURA Takumi wrote: >>>> >>>> [CMake] Introduce llvm_add_library(). >>>> >>>> >>>> I recommend moving away from wrappers like this. They indicate that either >>>> CMake is not providing the interfaces needed, or not propagating them, or >>>> that they exist but are not used. >>>> >>>> Such wrappers don't parse the arguments in the same way as the wrapped >>>> command etc. Wrappers are not good API proxies. Additionally, you put people >>>> who know cmake already at a disadvantage because you make the code look like >>>> 'not cmake code' by using llvm_add_library instead of add_library, the cmake >>>> command. Making that wrapper macro also invoke target_link_libraries looks >>>> odd to someone familiar with cmake. >>>> >>>> I don't fully know what this wrapper is needed for, but I looked at it a >>>> bit. >>>> >>>> if(ARG_MODULE) >>>> set_property(TARGET ${name} PROPERTY SUFFIX ${LLVM_PLUGIN_EXT}) >>>> endif() >>>> >>>> if(ARG_SHARED) >>>> if (MSVC) >>>> set_target_properties(${name} >>>> PROPERTIES >>>> IMPORT_SUFFIX ".imp") >>>> endif () >>>> endif() >>>> >>>> >>>> CMake provides CMAKE_SHARED_MODULE_SUFFIX and CMAKE_IMPORT_LIBRARY_SUFFIX. >>>> Can you set those at directory scope instead of setting the target property >>>> in the macro? >>>> >>>> >>>> Reasonable. They could be put into common configurator, HandleLLVMOptions. >>>> For now in r201072, I simply gather common logics around there. >>>> I'd be happy if llvm_add_library became simpler, and I will work on. >>>> >>>> Anyway, just a note to consider moving instead in a direction of fewer >>>> wrappers, rather than more. I'm not familiar enough with the llvm >>>> buildsystem to know why you're doing some of the things you're doing, but if >>>> you tell us about the gaps in the cmake offering we can try to fill them. >>>> >>>> Thanks, >>>> >>>> Steve. >>>> >>>> _______________________________________________ >>>> >>>> LLVM Developers mailing list >>>> LLVMdev at cs.uiuc.edu http://llvm.cs.uiuc.edu >>>> http://lists.cs.uiuc.edu/mailman/listinfo/llvmdev >>>> >>>> >>