Bug in CMake for building under Windows #739

OgreTransporter · 2020-05-25T11:09:14Z

cmake -G "Visual Studio 15" -A x64 -D CMAKE_INSTALL_PREFIX:PATH=D:\test4\install -D CMAKE_DEBUG_POSTFIX:STRING=d -D OPENEXR_BUILD_BOTH_STATIC_SHARED:BOOL=ON -D BUILD_SHARED_LIBS:BOOL=OFF -D ILMBASE_BUILD_BOTH_STATIC_SHARED:BOOL=ON -D BUILD_TESTING:BOOL=OFF -D PYILMBASE_ENABLE:BOOL=OFF -D ZLIB_INCLUDE_DIR:PATH=D:\test4\deps\zlib\install\include -D ZLIB_LIBRARY_DEBUG:FILEPATH=D:\test4\deps\zlib\install\lib\zlibstaticd.lib -D ZLIB_LIBRARY_RELEASE:FILEPATH=D:\test4\deps\zlib\install\lib\zlibstatic.lib D:\test4\src

All projects of the static libraries are created with the switch -DOPENEXR_DLL However, this switch is only intended for the shared libraries and therefore generates build errors under Windows. Example:

2>D:\test4\src\IlmBase\Imath\ImathVec.cpp(138): error C2491: "Imath_2_5::Vec2<short>::length": Definition von Funktion für dllimport nicht zulässig

Also, creating symbolic links during installation does not work on Windows. Example:

EXEC : CMake error : failed to create symbolic link 'Imath.dll': operation not permitted

It is useless anyway, because when distributing the DLLs to other systems the links do not work anymore. The links have to be recreated on each system. This must then be integrated into the installation program.

The text was updated successfully, but these errors were encountered:

OgreTransporter · 2020-05-25T11:10:40Z

Bugfix:

From ca131e6dcf0ad1ace0107e39ac69ed9939c68344 Mon Sep 17 00:00:00 2001
From: Transporter <[email protected]>
Date: Mon, 25 May 2020 13:08:44 +0200
Subject: [PATCH] Fix problems with -DOPENEXR_DLL on static libraries

---
 IlmBase/config/LibraryDefine.cmake    | 7 +++----
 OpenEXR/IlmImfExamples/CMakeLists.txt | 3 +++
 OpenEXR/config/LibraryDefine.cmake    | 7 +++----
 OpenEXR/exr2aces/CMakeLists.txt       | 3 +++
 OpenEXR/exrenvmap/CMakeLists.txt      | 3 +++
 OpenEXR/exrheader/CMakeLists.txt      | 3 +++
 OpenEXR/exrmakepreview/CMakeLists.txt | 3 +++
 OpenEXR/exrmaketiled/CMakeLists.txt   | 3 +++
 OpenEXR/exrmultipart/CMakeLists.txt   | 3 +++
 OpenEXR/exrmultiview/CMakeLists.txt   | 3 +++
 OpenEXR/exrstdattr/CMakeLists.txt     | 3 +++
 11 files changed, 33 insertions(+), 8 deletions(-)

diff --git a/IlmBase/config/LibraryDefine.cmake b/IlmBase/config/LibraryDefine.cmake
index 44254a7..cc7feb7 100644
--- a/IlmBase/config/LibraryDefine.cmake
+++ b/IlmBase/config/LibraryDefine.cmake
@@ -12,8 +12,6 @@ function(ILMBASE_DEFINE_LIBRARY libname)
   # only do the object library mechanism in a few cases:
   # - xcode doesn't handle "empty" targets (i.e. add_library with
   #   an object lib only)
-  # - under windows, we don't want the static library targets to
-  #   have the export tags
   # - if we're not compiling both, don't add the extra layer to prevent
   #   extra compiles since we aren't doing that anyway
   if(ILMBASE_BUILD_BOTH_STATIC_SHARED AND NOT (APPLE OR WIN32))
@@ -39,7 +37,7 @@ function(ILMBASE_DEFINE_LIBRARY libname)
   target_compile_features(${objlib} PUBLIC cxx_std_${OPENEXR_CXX_STANDARD})
   if(ILMBASE_CURLIB_PRIV_EXPORT AND BUILD_SHARED_LIBS)
     target_compile_definitions(${objlib} PRIVATE ${ILMBASE_CURLIB_PRIV_EXPORT})
-    if(WIN32)
+    if(WIN32 AND NOT ILMBASE_BUILD_BOTH_STATIC_SHARED)
       target_compile_definitions(${objlib} PUBLIC OPENEXR_DLL)
     endif()
   endif()
@@ -98,7 +96,7 @@ function(ILMBASE_DEFINE_LIBRARY libname)
     PUBLIC_HEADER
       DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/${ILMBASE_OUTPUT_SUBDIR}
   )
-  if(BUILD_SHARED_LIBS AND (NOT "${ILMBASE_LIB_SUFFIX}" STREQUAL ""))
+  if(BUILD_SHARED_LIBS AND (NOT "${ILMBASE_LIB_SUFFIX}" STREQUAL "") AND NOT WIN32)
     set(verlibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${ILMBASE_LIB_SUFFIX}${CMAKE_SHARED_LIBRARY_SUFFIX})
     set(baselibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${CMAKE_SHARED_LIBRARY_SUFFIX})
     if(WIN32)
@@ -118,6 +116,7 @@ function(ILMBASE_DEFINE_LIBRARY libname)
       target_link_libraries(${libname}_static INTERFACE ${objlib})
     else()
       # have to build multiple times... but have different flags anyway (i.e. no dll)
+      target_compile_definitions(${libname} PRIVATE OPENEXR_DLL)
       set(curlib ${libname}_static)
       add_library(${curlib} STATIC ${ILMBASE_CURLIB_SOURCES})
       target_compile_features(${curlib} PUBLIC cxx_std_${OPENEXR_CXX_STANDARD})
diff --git a/OpenEXR/IlmImfExamples/CMakeLists.txt b/OpenEXR/IlmImfExamples/CMakeLists.txt
index 3346911..cb3a65c 100644
--- a/OpenEXR/IlmImfExamples/CMakeLists.txt
+++ b/OpenEXR/IlmImfExamples/CMakeLists.txt
@@ -12,6 +12,9 @@ add_executable(IlmImfExamples
   rgbaInterfaceTiledExamples.cpp
 )
 target_link_libraries(IlmImfExamples OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(IlmImfExamples PRIVATE OPENEXR_DLL)
+endif()
 
 # Examples
 install(
diff --git a/OpenEXR/config/LibraryDefine.cmake b/OpenEXR/config/LibraryDefine.cmake
index a9561dc..8b4ae84 100644
--- a/OpenEXR/config/LibraryDefine.cmake
+++ b/OpenEXR/config/LibraryDefine.cmake
@@ -12,8 +12,6 @@ function(OPENEXR_DEFINE_LIBRARY libname)
   # only do the object library mechanism in a few cases:
   # - xcode doesn't handle "empty" targets (i.e. add_library with
   #   an object lib only)
-  # - under windows, we don't want the static library targets to
-  #   have the export tags
   # - if we're not compiling both, don't add the extra layer to prevent
   #   extra compiles since we aren't doing that anyway
   if(OPENEXR_BUILD_BOTH_STATIC_SHARED AND NOT (APPLE OR WIN32))
@@ -37,7 +35,7 @@ function(OPENEXR_DEFINE_LIBRARY libname)
   target_compile_features(${objlib} PUBLIC cxx_std_${OPENEXR_CXX_STANDARD})
   if(OPENEXR_CURLIB_PRIV_EXPORT AND BUILD_SHARED_LIBS)
     target_compile_definitions(${objlib} PRIVATE ${OPENEXR_CURLIB_PRIV_EXPORT})
-    if(WIN32)
+    if(WIN32 AND NOT OPENEXR_BUILD_BOTH_STATIC_SHARED)
       target_compile_definitions(${objlib} PUBLIC OPENEXR_DLL)
     endif()
   endif()
@@ -92,7 +90,7 @@ function(OPENEXR_DEFINE_LIBRARY libname)
     PUBLIC_HEADER
       DESTINATION ${CMAKE_INSTALL_INCLUDEDIR}/${OPENEXR_OUTPUT_SUBDIR}
   )
-  if(BUILD_SHARED_LIBS AND (NOT "${OPENEXR_LIB_SUFFIX}" STREQUAL ""))
+  if(BUILD_SHARED_LIBS AND (NOT "${OPENEXR_LIB_SUFFIX}" STREQUAL "") AND NOT WIN32)
     set(verlibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${OPENEXR_LIB_SUFFIX}${CMAKE_SHARED_LIBRARY_SUFFIX})
     set(baselibname ${CMAKE_SHARED_LIBRARY_PREFIX}${libname}${CMAKE_SHARED_LIBRARY_SUFFIX})
     if(WIN32)
@@ -112,6 +110,7 @@ function(OPENEXR_DEFINE_LIBRARY libname)
       target_link_libraries(${libname}_static INTERFACE ${objlib})
     else()
       # have to build multiple times... but have different flags anyway (i.e. no dll)
+      target_compile_definitions(${libname} PRIVATE OPENEXR_DLL)
       set(curlib ${libname}_static)
       add_library(${curlib} STATIC ${OPENEXR_CURLIB_SOURCES})
       target_compile_features(${curlib} PUBLIC cxx_std_${OPENEXR_CXX_STANDARD})
diff --git a/OpenEXR/exr2aces/CMakeLists.txt b/OpenEXR/exr2aces/CMakeLists.txt
index 74d9811..5885532 100644
--- a/OpenEXR/exr2aces/CMakeLists.txt
+++ b/OpenEXR/exr2aces/CMakeLists.txt
@@ -3,6 +3,9 @@
 
 add_executable(exr2aces main.cpp)
 target_link_libraries(exr2aces OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(exr2aces PRIVATE OPENEXR_DLL)
+endif()
 set_target_properties(exr2aces PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
 )
diff --git a/OpenEXR/exrenvmap/CMakeLists.txt b/OpenEXR/exrenvmap/CMakeLists.txt
index beb0225..eca3d0f 100644
--- a/OpenEXR/exrenvmap/CMakeLists.txt
+++ b/OpenEXR/exrenvmap/CMakeLists.txt
@@ -12,6 +12,9 @@ add_executable( exrenvmap
 )
 
 target_link_libraries(exrenvmap OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(exrenvmap PRIVATE OPENEXR_DLL)
+endif()
 set_target_properties(exrenvmap PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
 )
diff --git a/OpenEXR/exrheader/CMakeLists.txt b/OpenEXR/exrheader/CMakeLists.txt
index 5df8904..10ad405 100644
--- a/OpenEXR/exrheader/CMakeLists.txt
+++ b/OpenEXR/exrheader/CMakeLists.txt
@@ -3,6 +3,9 @@
 
 add_executable(exrheader main.cpp)
 target_link_libraries(exrheader OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(exrheader PRIVATE OPENEXR_DLL)
+endif()
 set_target_properties(exrheader PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
 )
diff --git a/OpenEXR/exrmakepreview/CMakeLists.txt b/OpenEXR/exrmakepreview/CMakeLists.txt
index cfc9e94..5184483 100644
--- a/OpenEXR/exrmakepreview/CMakeLists.txt
+++ b/OpenEXR/exrmakepreview/CMakeLists.txt
@@ -6,6 +6,9 @@ add_executable(exrmakepreview
   makePreview.cpp
 )
 target_link_libraries(exrmakepreview OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(exrmakepreview PRIVATE OPENEXR_DLL)
+endif()
 set_target_properties(exrmakepreview PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
 )
diff --git a/OpenEXR/exrmaketiled/CMakeLists.txt b/OpenEXR/exrmaketiled/CMakeLists.txt
index 8b0443a..5797dcf 100644
--- a/OpenEXR/exrmaketiled/CMakeLists.txt
+++ b/OpenEXR/exrmaketiled/CMakeLists.txt
@@ -7,6 +7,9 @@ add_executable(exrmaketiled
   Image.cpp
 )
 target_link_libraries(exrmaketiled OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(exrmaketiled PRIVATE OPENEXR_DLL)
+endif()
 set_target_properties(exrmaketiled PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
 )
diff --git a/OpenEXR/exrmultipart/CMakeLists.txt b/OpenEXR/exrmultipart/CMakeLists.txt
index f997e66..ccd3c5a 100644
--- a/OpenEXR/exrmultipart/CMakeLists.txt
+++ b/OpenEXR/exrmultipart/CMakeLists.txt
@@ -3,6 +3,9 @@
 
 add_executable(exrmultipart exrmultipart.cpp)
 target_link_libraries(exrmultipart OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(exrmultipart PRIVATE OPENEXR_DLL)
+endif()
 set_target_properties(exrmultipart PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
 )
diff --git a/OpenEXR/exrmultiview/CMakeLists.txt b/OpenEXR/exrmultiview/CMakeLists.txt
index 6446e22..ac76afc 100644
--- a/OpenEXR/exrmultiview/CMakeLists.txt
+++ b/OpenEXR/exrmultiview/CMakeLists.txt
@@ -7,6 +7,9 @@ add_executable(exrmultiview
   Image.cpp
 )
 target_link_libraries(exrmultiview OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(exrmultiview PRIVATE OPENEXR_DLL)
+endif()
 set_target_properties(exrmultiview PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
 )
diff --git a/OpenEXR/exrstdattr/CMakeLists.txt b/OpenEXR/exrstdattr/CMakeLists.txt
index c8c2158..7427538 100644
--- a/OpenEXR/exrstdattr/CMakeLists.txt
+++ b/OpenEXR/exrstdattr/CMakeLists.txt
@@ -3,6 +3,9 @@
 
 add_executable(exrstdattr main.cpp)
 target_link_libraries(exrstdattr OpenEXR::IlmImf)
+if(WIN32 AND (BUILD_SHARED_LIBS OR OPENEXR_BUILD_BOTH_STATIC_SHARED))
+  target_compile_definitions(exrstdattr PRIVATE OPENEXR_DLL)
+endif()
 set_target_properties(exrstdattr PROPERTIES
   RUNTIME_OUTPUT_DIRECTORY "${CMAKE_BINARY_DIR}/bin"
 )
-- 
2.17.1.windows.2

meshula · 2020-05-25T18:33:27Z

Thanks for the analysis, there's a few things to unpack here, so please bear with me :)

Symlinks on Windows are generally bad news. Thanks for finding that. Could you open a separate PR or issue about that one, as it is a problem orthogonal to issues of static and dynamic linking.
The prepocessor directive is intended only for the dynamic build, and to restate the issue you are reporting, when building both static and dynamic simultaneously, it is erroneously passed to the static build.
The command line you are using contains contradictory directives. It seems like a design flaw in our directives that it is possible to set up contradictory directives. What is your intent? It's hard to know what good fixes are without knowing that. I've broken the command line apart below to make it easier to discuss, with the contradictory lines separated out. You've specified BOTH_STATIC_SHARED_ON, and BUILD_SHARED_LIBS off. That's the part that I am confused about.

cmake -G "Visual Studio 15" -A x64 -D CMAKE_INSTALL_PREFIX:PATH=D:\test4\install -D CMAKE_DEBUG_POSTFIX:STRING=d 

-D OPENEXR_BUILD_BOTH_STATIC_SHARED:BOOL=ON 
-D BUILD_SHARED_LIBS:BOOL=OFF 
-D ILMBASE_BUILD_BOTH_STATIC_SHARED:BOOL=ON 

-D BUILD_TESTING:BOOL=OFF -D PYILMBASE_ENABLE:BOOL=OFF -D ZLIB_INCLUDE_DIR:PATH=D:\test4\deps\zlib\install\include -D ZLIB_LIBRARY_DEBUG:FILEPATH=D:\test4\deps\zlib\install\lib\zlibstaticd.lib -D ZLIB_LIBRARY_RELEASE:FILEPATH=D:\test4\deps\zlib\install\lib\zlibstatic.lib D:\test4\src

OgreTransporter · 2020-05-26T07:45:15Z

Symlinks on Windows are generally bad news. Thanks for finding that. Could you open a separate PR or issue about that one, as it is a problem orthogonal to issues of static and dynamic linking.

See issue #741 and PR #742

The prepocessor directive is intended only for the dynamic build, and to restate the issue you are reporting, when building both static and dynamic simultaneously, it is erroneously passed to the static build.

Yeah, that's what the patch does. OPENEXR_DLL is now only applied to the distributed libraries. Currently OPENEXR_DLL is applied to all projects. It is even listed as a bug in the CMakeFile:

under windows, we don't want the static library targets to have the export tags

See PR #743

The command line you are using contains contradictory directives. It seems like a design flaw in our directives that it is possible to set up contradictory directives. What is your intent? It's hard to know what good fixes are without knowing that. I've broken the command line apart below to make it easier to discuss, with the contradictory lines separated out. You've specified BOTH_STATIC_SHARED_ON, and BUILD_SHARED_LIBS off. That's the part that I am confused about.

I don't think this kind of configuration with BUILD_SHARED_LIBS, OPENEXR_BUILD_BOTH_STATIC_SHARED and ILMBASE_BUILD_BOTH_STATIC_SHARED is very smart either. It would be better to either always build static and shared libraries or use BUILD_SHARED_LIBS to build static or shared libraries. It's no problem to run CMake twice if needed, once with BUILD_SHARED_LIBS=ON and once with BUILD_SHARED_LIBS=OFF. BUILD_SHARED_LIBS has no effect at all if OPENEXR_BUILD_BOTH_STATIC_SHARED and ILMBASE_BUILD_BOTH_STATIC_SHARED are set, which I noticed later. Therefore I did not have to set BUILD_SHARED_LIBS. It was my first attempt to disable the global problem with OPENEXR_DLL.

meshula · 2020-05-27T06:47:25Z

Thanks for breaking it up into two pieces, it makes it a lot easier for me (at least ;) ) to reason about the changes.

cary-ilm · 2020-07-16T01:05:47Z

Looking into this, can we close this issue now that #742 and #743 have been merged?

OgreTransporter · 2020-07-26T14:22:00Z

Looking into this, can we close this issue now that #742 and #743 have been merged?

Yes, the problems have been fixed.

This was referenced May 26, 2020

[CMake] Symlinks under Windows #741

Closed

Removes symlink creation under Windows #742

Merged

Applies OPENEXR_DLL only to shared libraries and no longer to static libraries #743

Merged

cary-ilm added the Awaiting Feedback The issue is waiting for someone to respond. label Jul 16, 2020

cary-ilm closed this as completed Jul 27, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Bug in CMake for building under Windows #739

Bug in CMake for building under Windows #739

OgreTransporter commented May 25, 2020

OgreTransporter commented May 25, 2020

meshula commented May 25, 2020

OgreTransporter commented May 26, 2020

meshula commented May 27, 2020

cary-ilm commented Jul 16, 2020

OgreTransporter commented Jul 26, 2020 •

edited

Loading

Bug in CMake for building under Windows #739

Bug in CMake for building under Windows #739

Comments

OgreTransporter commented May 25, 2020

OgreTransporter commented May 25, 2020

meshula commented May 25, 2020

OgreTransporter commented May 26, 2020

meshula commented May 27, 2020

cary-ilm commented Jul 16, 2020

OgreTransporter commented Jul 26, 2020 • edited Loading

OgreTransporter commented Jul 26, 2020 •

edited

Loading