Skip to content

Commit

Permalink
update windows.cmake to fix common build issues on Windows (#984)
Browse files Browse the repository at this point in the history
* fix merge conflict, separate helper macro addition from this change

* add NO_STRICT from catkin level for Windows build.

* add Q_NOWINSTRICT

* add more global helper.

* add  WIN32_LEAN_AND_MEAN

*  Avoid using set_target_properties on ALIAS targets (#26)

* Avoid using set_target_properties on ALIAS targets

This change is to fix the error like below:
2018-11-30T10:40:39.7610068Z CMake Error at C:/opt/ros/melodic/x64/share/catkin/cmake/platform/windows.cmake:47 (set_target_properties):
2018-11-30T10:40:39.7610963Z   set_target_properties can not be used on an ALIAS target.
2018-11-30T10:40:39.7611263Z Call Stack (most recent call first):
2018-11-30T10:40:39.7611903Z   abseil_cpp/CMake/AbseilHelpers.cmake:67 (add_library)
2018-11-30T10:40:39.7612185Z   abseil_cpp/absl/base/CMakeLists.txt:74 (absl_library)

* nit

* Globally enable CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS for Windows build. (#29)

Walking through the projects we touched for ROS, many of them simply are just missing the dllimport/dllexport declarative, so no imported library is generated and breaks the build for downstream projects. Some are fixed by defining dllexport/dllimport, by defining WINDOWS_EXPORT_ALL_SYMBOLS, or by switching them to STATIC library (STATIC lib sometimes even creates more problems since the author assumes it is a shared lib at run-time).

In order to quickly enabling more untouched ROS packages, I am suggesting to enable CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS globally to quickly enabling them, but we should still encourage the package owners to explicitly use dllexport in our cookbook.

* update comments, reorder definitions in alphabetical order (#37)

* spelling
  • Loading branch information
kejxu authored and dirk-thomas committed Jan 28, 2019
1 parent b2f73e0 commit 3c62f99
Showing 1 changed file with 36 additions and 2 deletions.
38 changes: 36 additions & 2 deletions cmake/platform/windows.cmake
Original file line number Diff line number Diff line change
Expand Up @@ -41,9 +41,10 @@ if(BUILD_SHARED_LIBS)
if(WIN32)
function(add_library library)
# Check if its an external, imported library (e.g. boost libs via cmake module definition)
list(FIND ARGN "IMPORTED" FIND_POS)
list(FIND ARGN "IMPORTED" FIND_IMPORTED)
list(FIND ARGN "ALIAS" FIND_ALIAS)
_add_library(${ARGV0} ${ARGN})
if(${FIND_POS} EQUAL -1)
if(${FIND_IMPORTED} EQUAL -1 AND ${FIND_ALIAS} EQUAL -1)
set_target_properties(${ARGV0}
PROPERTIES
RUNTIME_OUTPUT_DIRECTORY ${CATKIN_DEVEL_PREFIX}/${CATKIN_GLOBAL_BIN_DESTINATION}
Expand All @@ -54,3 +55,36 @@ if(BUILD_SHARED_LIBS)
endfunction()
endif()
endif()

# It is encouraged to follow this guide to enable exports for dll's in a cross-platform way:
# http://wiki.ros.org/win_ros/Contributing/Dll%20Exports
# however, since not every project has implemented import/export macros, enable CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS as a workaround
# https://blog.kitware.com/create-dlls-on-windows-without-declspec-using-new-cmake-export-all-feature/
if(BUILD_SHARED_LIBS)
if(WIN32)
set(CMAKE_WINDOWS_EXPORT_ALL_SYMBOLS ON)
endif()
endif()

# For Windows, add definitions to exclude defining common names macros that cause name collision
if(WIN32)
# enable Math Constants (https://docs.microsoft.com/en-us/cpp/c-runtime-library/math-constants)
add_definitions(-D_USE_MATH_DEFINES)

# do not define STRICT macros (minwindef.h or boost\winapi\basic_types.hpp)
add_definitions(-DNO_STRICT)

# do not define min/max macros
add_definitions(-DNOMINMAX)

# do not define STRICT macros (qtgui\qwindowdefs_win.h)
add_definitions(-DQ_NOWINSTRICT)

# keep minimum windows headers inclusion
add_definitions(-DWIN32_LEAN_AND_MEAN)
endif()

if(MSVC)
# https://blogs.msdn.microsoft.com/vcblog/2018/04/09/msvc-now-correctly-reports-__cplusplus/
add_compile_options(/Zc:__cplusplus)
endif()

0 comments on commit 3c62f99

Please sign in to comment.