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_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?
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.
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_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.
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.
