SOCI / soci

Official repository of the SOCI - The C++ Database Access Library
http://soci.sourceforge.net/
Boost Software License 1.0
1.42k stars 478 forks source link

CMAKE_RUNTIME_OUTPUT_DIRECTORY Windows compatibility #1159

Open yihuajack opened 3 months ago

yihuajack commented 3 months ago

In CMakeLists.txt,

set(CMAKE_LIBRARY_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_ARCHIVE_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/lib)
set(CMAKE_RUNTIME_OUTPUT_DIRECTORY ${CMAKE_BINARY_DIR}/bin)

which will put socicore and soci DLLs under ${CMAKE_BINARY_DIR}/bin. However, on Windows, the executable is just under ${CMAKE_BINARY_DIR} rather than ${CMAKE_BINARY_DIR}/bin. If putting DLLs under ${CMAKE_BINARY_DIR} then the executable would fail to find the soci DLLs.

vadz commented 2 months ago

Sorry, which executable do you mean? I.e. what exactly is the problem and when does it happen?

yihuajack commented 2 months ago

I mean the executable of the program that uses the soci library, which is expected to be located under ${CMAKE_BINARY_DIR} on Windows. The problem is that if adding soci as a thirdparty library for a program on Windows, the program will not find any dlls under ${CMAKE_BINARY_DIR}/bin but ${CMAKE_BINARY_DIR}, so that there will be a dll not found error. One way to solve this is to manually copy soci's dlls from ${CMAKE_BINARY_DIR}/bin to ${CMAKE_BINARY_DIR}, but under some circumstances it is hard to do this copy for compatibility issues. I'm not sure about what's the role of the three settings in soci's CMakeLists.txt (if removing these settings or changes the settings to ${CMAKE_BINARY_DIR} then no problem exists).

vadz commented 2 months ago

Unfortunately I'm not sure about them neither. There is the #1118 (WIP) which aims to streamline all this, but I don't know if it addresses this.

Does your top-level project define CMAKE_RUNTIME_OUTPUT_DIRECTORY? If it does, we could skip setting it if it's already set which should be totally safe, I think.

Krzmbrzl commented 2 months ago

I have built applications on top of #1118 and I don't see any issues even on Windows.

The issue here is likely in the order in which the top-level project includes SOCI. If you first include SOCI, then the runtime output property propagates to your top-level project such that all your target inherit the same property (unless you set it explicitly to something else). However, any targets declared before including SOCI will not have this property and also won't get it once SOCI is included as the setting merely acts as a default for the given target's property.

Since the output directory property merely serves a cosmetic issue, I would vote for just not explicitly setting these things in SOCI. Then cmake's default will do something that just works on all platforms. And if any given user has special desires as to where the build artifacts should end up, they can easily set these variables when calling cmake from the command line. That will then also ensure that the result is consistent, avoiding issues such as the one presented here.


@vadz as a side-note: feel free to ping me in cmake-related issues. Especially due to my work with the cmake refactoring I a) know a lot about SOCI's existing cmake implementation and b) have an interest in getting to know all issues that people face with the current setup to make sure the new one fixes it :)

yihuajack commented 2 months ago

Thanks for your instruction. Your PR #1118 can fix this issue.