Unable to locate rttr library when using conan. CMake files need to be refactored to wrap RTTR_DEBUG_POSTFIX usage

[ctin]

Hello! Right now i am using conan to build all libraries for all platforms (yes, I am using rttr for ios and android). If user will not pass build_type to conan, CMake will choose debug as default build_type in some cases (in some others - Release). Actually conan don't know nothing about choosen build_type so it searching for rttr_core, not for rttr_core_d. I found that _d is RTTR_DEBUG_POSTFIX value. But when I set it to an empty string - I've got a lot of CMake errors, because this variable is used directly in set_target_properties function. My proposal is to wrap all set_target_properties with if $RTTR_DEBUG_POSTFIX to have an ability to unset it. Thank you.

p.s. also it may be good idea to use RTTR_DEBUG_POSTFIX as option, not as variable: option(RTTR_DEBUG_POSTFIX "library debug postfix." "_d")

[acki-m]

@ctin I found no RTTR_DEBUG_POSTFIX variable in CMake. Or do you mean CMAKE_DEBUG_POSTFIX? I still don't understand why it is a problem for Conan to have this postfix option...

Btw. Here is a conan script: https://github.com/rttrorg/conan-rttr/blob/master/conanfile.py Doesn't it work without this changes?

[ctin]

RTTR_DEBUG_POSTFIX is declared here: https://github.com/rttrorg/rttr/blob/master/CMake/config.cmake#L89

Just looked at your conanfile.py, it can solve the problem as well, but we still have another one: if self.settings.build_type is not defined (by default it not defined), your conanfile.py will expect "rttr_core", while cmake will generate "rttr_core_d". Only one way to solve it correctly is to set CMAKE_DEBUG_POSTFIX from conan. But actually you already have variable I talked above. But this variable an not be empty, because you can not pass empty DEBUG_POSTFIX to cmake target properties, but you need to clear this variable from conan.

[acki-m]

I am not a conan expert and this script is not from me. However, I am unsure, it is the right way to fix it. The debug postsuffix is still needed for windows builds, so debug and release libraries are placed in the same folder. How will conan handle this?

You should also talk with @weatherhead99 He was also quite active using conan and rttr.

[ctin]

Conan will create different build folders for each build variant. I am not against postfix at all, I am proposing just to have an ability to clear it. This work is complex because you are using RTTR_DEBUG_POSTFIX at so many places, and every one of them need to be wrapped with if( ${RTTR_DEBUG_POSTFIX} ). I can make this pull-request if you agree.

UPD: googled cmake error, found advice: https://stackoverflow.com/questions/3221996/set-target-properties-called-with-incorrect-number-of-arguments Actually no need of if..., I can just add quotes for all set_target_properties call

[weatherhead99]

hi @ctin I'm afraid I haven't kept up with latest conan & rttr developments nor had time to get it properly into shape, but could you also test using my conan-rttr package, see if you have the same issue? https://github.com/weatherhead99/conan-rttr

there is a separate branch for 0.9.6 and 0.9.5, as is the style of bincrafters packages

[ctin]

@weatherhead99 hi! just run it . MacOS High Sierra, profile:

[settings]
# Target
os=Android
os.api_level=24
arch=x86_64
compiler=clang
compiler.libcxx=libc++
compiler.version=5.0
cppstd=14

# Host
os_build=Macos
arch_build=x86_64

[build_requires]
android_ndk_toolchain/r16b@ctin/stable

command conan create . ctin/experimental 0.9.5:

CMake Error at src/rttr/CMakeLists.txt:96 (message):
  Do now know how to to name the static library.

whoops, passing option -o shared = False;

test_package/conanfile.py: 'options.shared' doesn't exist

whoops, fixing option shared=True in conanfile.py default value also adding line print("!!!!!!!!", "_debug" if self.settings.build_type == "Debug" else "release") to config_options method to predict _d postfix.

!!!!!!!! release
rttr/0.9.5@ctin/experimental: WARN: Forced build from source

well, expecting release library [ 50%] Linking CXX shared library ../../lib/librttr_core_d.so hmmmm...

.../build/659df99f39280d6d92a7c010c9a31a201693e1bb/rttr-0.9.5-src/src/unit_tests/variant/variant_ctor_test.cpp:242:25: error: 
      no template named 'has_trivial_copy_constructor' in namespace 'std'; did you mean 'is_trivially_copy_constructible'?
    static_assert(!std::has_trivial_copy_constructor<self_aware>::value, "");
                   ~~~~~^~~~~~~~~~~~~~~~~~~~~~~~~~~~
                        is_trivially_copy_constructible
.../package/2276107abe7327a78ee525aca6c169b8e0e69607/lib/gcc/x86_64-linux-android/4.9.x/../../../../include/c++/4.9.x/type_traits:3433:50: note: 
      'is_trivially_copy_constructible' declared here
template <class _Tp> struct _LIBCPP_TEMPLATE_VIS is_trivially_copy_constructible
                                                 ^
1 error generated.

oh, did not compile.

0.9.6:

!!!!!!!! release
rttr/0.9.5@ctin/experimental: WARN: Forced build from source

well, expecting release library [ 50%] Linking CXX shared library ../../lib/librttr_core_d.so hmmmm, I saw it before.

[100%] Running unit_tests
/bin/sh: ../../bin/unit_tests_d: cannot execute binary file

After i explored build folders, it really made "_d" library.

Actually, as I talked before, without line "build_type" in your profile you will get UB because CMake will not tell to conan the build type it choosed. The only nice way to solve this problem is to control postfixes and prefixes from conan via cmake definitions and options.

[weatherhead99]

@ctin I'm afraid I don't use OSX in any of my environments, but I did do some very basic testing on OSX when designing this recipe (specifically for 0.9.6). I agree in general that it would be much preferable to improve rttr's CMake build system, it in general is a bit idiosyncratic from the point of view of usual linux / OSX libraries. I'm not sure whether the _d (and indeed _s suffixes) that it uses are specifically needed for Windows purposes. I use this recipe for several different linux distros and windows 7 and windows 10 (both release and debug builds) in production though.

However, there are other options: we can manually (and quite annoyingly, considering e.g. RPATH issues) rename and re-patch things from conan without altering rttr source. We could also include another patch file in the recipe (as I currently to to set CMAKE_CXX_STANDARD, which is required to build the library properly on GCC 4.8 and up to 5.4).

I can try and have a look into this on an OSX machine but it will be very low priority. If you can find a way to make the recipe work on OSX (even by patching the source of rttr if you like!) then please do throw a PR at that repository.

[acki-m]

@weatherhead99 Maybe I should remove the _d suffix for linux builds. I build RTTR from gc 4.9 up to 7 in CI: https://travis-ci.org/rttrorg/rttr/jobs/420715774 no problems so far.

[ctin]

@acki-m My proposal is simple: instead of variable with _d postfix create an option. So this option default value will be _d, but user can define any other value. But you will get a lot of generation errors if you will set empty string instead of _d, and I printed above how to solve them (by adding quotes in set_target_properties)

[acki-m]

@ctin How does other libraries solve this? E.g. boost, aren't they working on a cmake solution for boost? We don't have to reinvent the wheel.

[weatherhead99]

from the point of view of conan at least it doesn't matter about library name collisions, as every separate combination of settings (including debug vs release build) will have its own package cache directory.

[acki-m]

The thing, is, when you bundle the libraries, e.g. you want to ship one installer, which container debug and release binaries, which are in one folder, you need on windows two libraries, with different names.

So the suffix should be there also, when using conan. My suggestion would be to set the suffix only for windows based builds. The rest, will not have it.

[ctin]

My libraries compiled in separate folders (like Debug and Release folders) even if they are not based on conan. Conan creates new folder for every configuration type (like debug, release, compiler version etc.). I saw _d postfix in other libraries, so this is a common situation too. @acki-m your suggestion is wrong, because someone will use conan on windows. I found where is this prefix set in RTTR cmake code and I know how to make it configurable. Should i sumbit pull request?

[weatherhead99]

@ctin the conan builds for windows all now pass on AppVeyor, including the package tests. If you are consuming the package via CMake, you can use either the find_package() or the target_link_libraries(lib ${CONAN_LIBS}) options perfectly well, in release and debug modes... (https://ci.appveyor.com/project/weatherhead99/conan-rttr). What I'm saying is, for my branch conan package at least, you don't need to worry about it on windows, I use this every day in both debug and release modes... I will PR my package against @acki-m once I get all the builds to pass (including OSX)... as for ios and android, probably these need more package changes in conanfile.py. I personally have no use building either of these platforms and don't have the spare energy to get it working, but would much appreciate any patches against the recipe you want to provide. The idea is that we don't change the upstream source in order to enable conan packaging. I don't think getting rid of CMAKE_DEBUG_POSTFIX is a good idea, because presumably @acki-m has many other users not using conan who now expect this behaviour to stay as it currently is...

If you have another way that this will be consumed in windows, please either give an example so I can cover this method in my package test...

I have started working on testing the package also on OSX, and it's nearly there but not quite working yet.

p.s. if your actual problem is that cmake doesn't "tell" conan what CMAKE_BUILD_TYPE is, that's true. And there is no way around it as conan is designed to ship pre-built binary packages. The official solution to this is if you want a debug build, you do, eg when installing the deps:

conan install .. -sbuild_type=Debug -gcmake and if you want release you do conan install .. -sbuild_type=Release -gcmake

and then call cmake .. -DCMAKE_BUILD_TYPE=Debug or release as appropriate. Conan will handle the prefix and suffix of the library, the package is designed that way. On windows at least (the same is not true for OSX and linux) one simply cannot link a debug library (e.g. using MDd runtime) against a release library (using MD runtime). So conan has to have separate packages for this. (The MT / MTd runtime is also supported in the windows package and works). Actually, in fact, if you install the deps with -sbuild_type=, then conan will indeed pass the correct -DCMAKE_BUILD_TYPE= flag to cmake on your own project anyway.

[acki-m]

The only rule I have is, the default names of the libraries should not be changed with another packagmanger. They should stay as they are, because other people will see that you deploy rrr_core.dll and think this is the release version, why instead it might be the debug one.

When I understand @weatherhead99 correctly, there is a way around the problem to fix it, without adding another option.