3

I am on Windows 10, Visual Studio 2015. Suppose I am building library A with CMakeLists looking like

cmake_minimum_required(VERSION 3.7)
project(A)

set(DLLIMPORT "__declspec(dllimport)")
set(DLLEXPORT "__declspec(dllexport)")

set(PROJECT_SRCS
${PROJECT_SOURCE_DIR}/src/TestA.cpp)

set(PROJECT_INCS
${PROJECT_SOURCE_DIR}/include/TestA.h)

add_library(${PROJECT_NAME} SHARED ${PROJECT_SRCS} ${PROJECT_INCS})

target_compile_definitions(${PROJECT_NAME} INTERFACE
                          WINDOWS_DLL_API=${DLLIMPORT})

target_compile_definitions(${PROJECT_NAME} PRIVATE
                          WINDOWS_DLL_API=${DLLEXPORT})

target_include_directories(${PROJECT_NAME} PUBLIC
                          $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
                          $<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/include>)

I am defining the macro WINDOWS_DLL_API as dllexport when it's building library A, and defining WINDOWS_DLL_API as dllimport for external applications that is linking library A. The problem is when I have another library B that is also linking A, I don't know how to overwrite WINDOWS_DLL_API back to dllexport. Below is my attempt of my CMakeLists for library B,

cmake_minimum_required(VERSION 3.7)
project(B)

set(DLLEXPORT "__declspec(dllexport)")

set(PROJECT_SRCS
${PROJECT_SOURCE_DIR}/src/TestB.cpp)

set(PROJECT_INCS
${PROJECT_SOURCE_DIR}/include/TestB.h)

add_library(${PROJECT_NAME} SHARED ${PROJECT_SRCS} ${PROJECT_INCS})

target_include_directories(${PROJECT_NAME} PUBLIC
    $<BUILD_INTERFACE:${PROJECT_SOURCE_DIR}/include>
    $<INSTALL_INTERFACE:${CMAKE_INSTALL_PREFIX}/include>)

target_link_libraries(${PROJECT_NAME} A)

# does not work
target_compile_definitions(${PROJECT_NAME} PRIVATE
                          WINDOWS_DLL_API=${DLLEXPORT})

What is the right way to do it?

user3667089
  • 2,996
  • 5
  • 30
  • 56
  • `... defining WINDOWS_DLL_API as dllimport for external applications that is linking library A.` - `B` is "external" library, which links with `A`, so it obtains this definition. If this definition isn't needed for `B`, simply do not define it for `A` as *INTERFACE*. – Tsyvarev Jan 26 '17 at 04:33
  • @Tsyvarev If I don't define it, then if I have an application (`add_executable`) that links to A and includes `TestA.h` , `WINDOWS_DLL_API ` will be undefined. – user3667089 Jan 26 '17 at 05:56
  • This might be useful [GenerateExportHeader](https://cmake.org/cmake/help/v3.0/module/GenerateExportHeader.html). It works with `_dllexport/__dllimport`, and it also handles GCC builds with `-fvisibility=hidden` – Nazar554 Jan 27 '17 at 10:59
  • @Nazar554 No it doesn't work with dllimport and you are forced to dllexport everything in the header. See https://blog.kitware.com/create-dlls-on-windows-without-declspec-using-new-cmake-export-all-feature/. Unfortunately dll macros are still needed – user3667089 Jan 27 '17 at 15:55

2 Answers2

4

Concept of INTERFACE option for command target_compile_definitions (and for other target_* CMake commands) is to enforce something for all users of the library, both executables and libraries.

Intention to clear enforcement for at least single library's user means that the conception is used in a wrong way. And other approaches should be used instead.

In given case, you need to use different macro names for libraries A and B. And it is better to remove INTERFACE option completely, so even non-CMake users of your library will be happy.

TestA.h:

#ifdef BUILD_A
#define WINDOWS_DLL_API_A __declspec(dllexport)
#else
#define WINDOWS_DLL_API_A __declspec(dllimport)
#endif

...
WINDOWS_DLL_API_A void foo(void);
...

TestB.h:

#ifdef BUILD_B
#define WINDOWS_DLL_API_B __declspec(dllexport)
#else
#define WINDOWS_DLL_API_B __declspec(dllimport)
#endif

// Assume usage of A is here.
#include <TestA.h>
...
WINDOWS_DLL_API_B void bar(void);

A/CMakeLists.txt:

cmake_minimum_required(VERSION 3.7)
project(A)

...    

add_library(${PROJECT_NAME} SHARED ...)

target_compile_definitions(${PROJECT_NAME} PRIVATE "BUILD_${PROJECT_NAME}=1")

B/CMakeLists.txt:

cmake_minimum_required(VERSION 3.7)
project(B)

...    

add_library(${PROJECT_NAME} SHARED ...)

target_compile_definitions(${PROJECT_NAME} PRIVATE "BUILD_${PROJECT_NAME}=1")

target_link_libraries(${PROJECT_NAME} A)

See also this answer, which provides more detailed header, which works on Windows platforms too.


Note, that when the library B includes header from A, it treats foo() as imported, and this is correct: the function is defined in A, not in B. With your approach (even if you would manage to redefine WINDOWS_DLL_API for B), library B would incorrectly treat foo() as exported.

This is an advantage of the conception: intention to overcome a conception signals that you do something wrong.

Community
  • 1
  • 1
Tsyvarev
  • 60,011
  • 17
  • 110
  • 153
  • wow, this works but it's more messier then I expected, not to mention that if I want my code to compile on linux I have to add a few more conditions to make `WINDOWS_DLL_API` equals to nothing. – user3667089 Jan 26 '17 at 15:50
  • Yes, header files become more complex if need to support several platforms(OSes). See e.g. [this answer](http://stackoverflow.com/a/39231971/3440745) (I have added this reference into my post too). – Tsyvarev Jan 26 '17 at 17:41
1

Just wanted to add my piece of code I'm using (compatible with CMake versions prior to 2.8.12).

In my root CMakeLists.txt file I have:

if (MSVC)
    add_definitions(-DWINDOWS_DLL_API=__declspec\(dllexport\))
else()
    add_definitions(-DWINDOWS_DLL_API=)
endif()

In the (sub-)project's CMakeLists.txt using the DLL I've put:

if (MSVC)
    remove_definitions(-DWINDOWS_DLL_API=__declspec\(dllexport\))
    add_definitions(-DWINDOWS_DLL_API=__declspec\(dllimport\))
endif()

The MSVC checks are necessary in my case because I also cross-compile.

Reference

Community
  • 1
  • 1
Florian
  • 39,996
  • 9
  • 133
  • 149
  • Just want to point out that the reason that the new `target_compile_definitions` is preferred is because the old `add_definitions` way would not work when exporting targets – user3667089 Jan 27 '17 at 15:48