MyGUI / mygui

Fast, flexible and simple GUI.
http://mygui.info/
Other
726 stars 207 forks source link

Split out libraries as shared libraries #14

Closed psi29a closed 10 years ago

psi29a commented 10 years ago

Currently on Debian (and Ubuntu), we carry a patch around to split out the MyGUIEngine into separate shared libraries. In addition to this, we also correct the SOVERSION for these new shared libs.

Is it possible to make this the default behavior?

I haven't forked and asked for a PR because I wasn't sure how best this should be done or if needed to discuss it more.

Here is our patch:

Description: build the library as a shared library
Author: Scott Howard 

--- a/Platforms/Ogre/OgrePlatform/CMakeLists.txt
+++ b/Platforms/Ogre/OgrePlatform/CMakeLists.txt
@@ -8,11 +8,12 @@ include_directories(

 include(${PROJECTNAME}.list)

-add_library(${PROJECTNAME} ${HEADER_FILES} ${SOURCE_FILES})
+add_library(${PROJECTNAME} SHARED ${HEADER_FILES} ${SOURCE_FILES})
+set_target_properties(${PROJECTNAME} PROPERTIES VERSION 0debian1.0.0 SOVERSION 0debian1)

 add_dependencies(${PROJECTNAME} MyGUIEngine)

-target_link_libraries(${PROJECTNAME} ${OGRE_LIBRARIES})
+target_link_libraries(${PROJECTNAME} ${OGRE_LIBRARIES} MyGUIEngine boost_system)
 link_directories(${OGRE_LIB_DIR})

 install(FILES ${HEADER_FILES}
--- a/Plugins/CMakeLists.txt
+++ b/Plugins/CMakeLists.txt
@@ -1,7 +1,7 @@
 option(MYGUI_BUILD_HIKARI_PLUGIN "Build Plugin HikariWidget" FALSE)
 option(MYGUI_BUILD_BERKELIUM_PLUGIN "Build Plugin BerkeliumWidget" FALSE)

-add_subdirectory(Plugin_StrangeButton)
+#add_subdirectory(Plugin_StrangeButton)

 if (MYGUI_BUILD_HIKARI_PLUGIN)
    if (WIN32)
--- a/Platforms/OpenGL/OpenGLPlatform/CMakeLists.txt
+++ b/Platforms/OpenGL/OpenGLPlatform/CMakeLists.txt
@@ -9,14 +9,15 @@ include_directories(

 include(${PROJECTNAME}.list)

-add_definitions(-DGLEW_STATIC)
+#add_definitions(-DGLEW_STATIC)
 add_definitions(-DGL_GLEXT_PROTOTYPES)

-add_library(${PROJECTNAME} ${HEADER_FILES} ${SOURCE_FILES})
+add_library(${PROJECTNAME} SHARED ${HEADER_FILES} ${SOURCE_FILES})
+set_target_properties(${PROJECTNAME} PROPERTIES VERSION 0debian1.0.0 SOVERSION 0debian1)

 add_dependencies(${PROJECTNAME} MyGUIEngine)

-target_link_libraries(${PROJECTNAME} ${OPENGL_LIBRARIES} ${PNG_LIBRARIES} ${ZLIB_LIBRARIES})
+target_link_libraries(${PROJECTNAME} GL ${PNG_LIBRARIES} ${ZLIB_LIBRARIES} MyGUIEngine GLEW)
 link_directories(${OPENGL_LIB_DIR} ${PNG_LIBRARY})

 # installation rules
--- a/Platforms/OpenGL/OpenGLPlatform/MyGUI.OpenGLPlatform.list
+++ b/Platforms/OpenGL/OpenGLPlatform/MyGUI.OpenGLPlatform.list
@@ -8,7 +8,6 @@ set (HEADER_FILES
   include/MyGUI_OpenGLVertexBuffer.h
 )
 set (SOURCE_FILES
-  include/GL/glew.c
   src/MyGUI_OpenGLDataManager.cpp
   src/MyGUI_OpenGLPlatform.cpp
   src/MyGUI_OpenGLRTTexture.cpp
@@ -26,7 +25,6 @@ SOURCE_GROUP("Header Files" FILES
   include/MyGUI_OpenGLVertexBuffer.h
 )
 SOURCE_GROUP("Source Files" FILES
-  include/GL/glew.c
   src/MyGUI_OpenGLDataManager.cpp
   src/MyGUI_OpenGLPlatform.cpp
   src/MyGUI_OpenGLRTTexture.cpp
--- a/MyGUIEngine/CMakeLists.txt
+++ b/MyGUIEngine/CMakeLists.txt
@@ -44,7 +44,8 @@ endif ()

 # setup MyGUIEngine target
 add_library(${PROJECTNAME} ${MYGUI_LIB_TYPE} ${HEADER_FILES} ${SOURCE_FILES})
-set_target_properties(${PROJECTNAME} PROPERTIES VERSION ${MYGUI_VERSION} SOVERSION ${MYGUI_VERSION_MAJOR})
+set_target_properties(${PROJECTNAME} PROPERTIES VERSION ${MYGUI_VERSION} SOVERSION ${MYGUI_VERSION_MAJOR} LINK_INTERFACE_LIBRARIES "")
+target_link_libraries(${PROJECTNAME} dl)
 if (MYGUI_USE_FREETYPE)
    target_link_libraries(${PROJECTNAME}
        ${FREETYPE_LIBRARIES}
ghost commented 10 years ago

This patch is too big and should be broken down into multiple patches:

Other changes that I don't understand:

Altren commented 10 years ago

Use system installation of GLEW (looks reasonable, but should be optional)

Agree.

Build Platforms as shared libraries

Platforms are not supposed to be built as dynamic libraries. Those are rather examples of render implementation that can be freely changed or used as it is, but there are no dllexport or visibility attribute that are needed for shared libraries and I guess those won't even work if compiled that way.

maqifrnswa commented 10 years ago

you're right, this is actually a bunch of different fixes. See my comments below

You added MyGUIEngine to target_link_libraries in some cases, where MyGUIEngine has already been set up as a dependency: add_dependencies(${PROJECTNAME} MyGUIEngine). This shouldn't be needed!

A bunch of symbols are missing, the dependency may have been added but it wasn't linked. See below dpkg-shlibdeps: warning: symbol _ZN5MyGUI9SingletonINS_13RenderManagerEE10msInstanceE used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZTIN5MyGUI9ExceptionE used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI9SingletonINS_13RenderManagerEE14mClassTypeNameE used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI5TimerC1Ev used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI10LogManager3logERKSsNS_8LogLevelES2_PKci used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI9ExceptionC1ERKSsS2_PKcl used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZTIN5MyGUI13RenderManagerE used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZTVN5MyGUI13RenderManagerE used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI9ExceptionD1Ev used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI10LogManager11getInstanceEv used by debian/libmygui.ogreplatform0debian1/usr/lib/libMyGUI.OgrePlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: 7 other similar warnings have been skipped (use -v to see them all) dpkg-shlibdeps: warning: symbol _ZN5MyGUI10LogManagerD1Ev used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI7UStringD1Ev used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI9SingletonINS_11DataManagerEE14mClassTypeNameE used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI9ExceptionC1ERKSsS2_PKcl used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI9SingletonINS_13RenderManagerEE14mClassTypeNameE used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZNK5MyGUI7UString6asUTF8Ev used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI9ExceptionD1Ev used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI10LogManager3logERKSsNS_8LogLevelES2_PKci used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZN5MyGUI14DataFileStreamC1EPSt14basic_ifstreamIcSt11char_traitsIcEE used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: symbol _ZTVN5MyGUI13RenderManagerE used by debian/libmygui.openglplatform0debian1/usr/lib/libMyGUI.OpenGLPlatform.so.0debian1.0.0 found in none of the libraries dpkg-shlibdeps: warning: 19 other similar warnings have been skipped (use -v to see them all)

+target_link_libraries(${PROJECTNAME} dl) - Why? Doesn't seem to be needed on my end (Ubuntu)

same as above, libMyGUIEngine.so.3.2.1 needs dlclose, dlopen, dlsym dpkg-shlibdeps: warning: symbol dlclose used by debian/libmyguiengine3debian1/usr/lib/libMyGUIEngine.so.3.2.1 found in none of the libraries dpkg-shlibdeps: warning: symbol dlopen used by debian/libmyguiengine3debian1/usr/lib/libMyGUIEngine.so.3.2.1 found in none of the libraries dpkg-shlibdeps: warning: symbol dlsym used by debian/libmyguiengine3debian1/usr/lib/libMyGUIEngine.so.3.2.1 found in none of the libraries

Plugin_StrangeButton disabled?

Licensing problem: there is no license on those files, so Debian can't distribute

Changed ${OPENGL_LIBRARIES} to GL. Why?

OPENGL_LIBRARIES links to x11 and GLU, but not GL. The variable OPENGL_LIBRARIES means "link against the libraries opengl depends on" not "link against the opengl library." You don't need x11 nor GLU, but you do need GL. Without GL, symbols are missing. You also get to avoid an extra link to x11 and GLU which you didn't need anyways.

maqifrnswa commented 10 years ago

Platforms are not supposed to be built as dynamic libraries. Those are rather examples of render implementation that can be freely changed or used as it is, but there are no dllexport or visibility attribute that are needed for shared libraries and I guess those won't even work if compiled that way.

They work on linux at least. openmw has been distributing them for about a year in their PPA. Symbols are getting exported somehow, you can see the symbols in the openmw binary.

ghost commented 10 years ago

Plugin_StrangeButton disabled?

Licensing problem: there is no license on those files, so Debian can't distribute

I'm not sure what you mean. The COPYING.MIT file in the repository root states: "Unless otherwise indicated, Source Code is licensed under MIT license." In the files for Plugin_StrangeButton, no other license is mentioned, so it should be assumed that it is licensed under MIT.

A bunch of symbols are missing, the dependency may have been added but it wasn't linked. See below

That is only causing problems because you are trying to build a shared library. Static libraries don't need to be linked.

OPENGL_LIBRARIES links to x11 and GLU, but not GL. The variable OPENGL_LIBRARIES means "link against the libraries opengl depends on" not "link against the opengl library." You don't need x11 nor GLU, but you do need GL. Without GL, symbols are missing. You also get to avoid an extra link to x11 and GLU which you didn't need anyways.

Could you use OPENGL_gl_LIBRARY variable then perhaps?

They work on linux at least. openmw has been distributing them for about a year in their PPA. Symbols are getting exported somehow, you can see the symbols in the openmw binary.

That's because GCC does not hide symbols by default. To hide all non-exported symbols, one must use -fvisibility=hidden switch. MyGUI uses this switch for shared libraries, but doesn't use it for platforms, which were meant to be built statically. That means as is, your shared library change will just happen to work on GCC, but will break on other compilers using different defaults.

same as above, libMyGUIEngine.so.3.2.1 needs dlclose, dlopen, dlsym

I see. Still wondering why that isn't causing link errors, then. Perhaps something else (Ogre?) is pulling in this library?

Altren commented 10 years ago
  1. dl lib now linked when found in system (used Ogre's solution). Even though it worked fine on my debian machine without adding dl.
  2. Plugin_StrangeButton and almost everything else is licensed under MIT. If you need license header in source files then those do not exist in Platforms code as well, is that the problem?
  3. As I said platforms are not designed to be compiled as dynamic libraries. Is that really necessary for distribution? And scrawl is right, that this cause most linker errors.
  4. Could you make patch for optional GL linking? Add some boolean flag like MYGUI_USE_SYSTEM_OPENGL (false by default) and use it for linking system installed library.

Perhaps something else (Ogre?) is pulling in this library?

Just FYI MyGUIEngine does not use Ogre.

maqifrnswa commented 10 years ago

Thank you everyone - a lot of this might be old patches carried over from a while ago, hopefully I can clear some of this up too!

Licensing is fine - I honestly don't remember if there was another reason not to build or distribute strange button...

Thank you scrawl for figuring out that gcc thing. We can distribute static libraries, but depending packages should only use shared libraries unless there is a really good reason not to. That's why we made them shared. Shared libraries prevents us from having to rebuild every depending package if a bug is fixed in a library. It also allows depending packages to only pull in the platform it is going to use.

I'll check the dl linking again, but I just compiled 3.2.1, and dl wasn't linked even though it was present on the system.

We can add and option for system GLEW.

Thank you again!

maqifrnswa commented 10 years ago

Thank you scrawl for figuring out that gcc thing. We can distribute static libraries, but depending packages should only use shared libraries unless there is a really good reason not to. That's why we made them shared. Shared libraries prevents us from having to rebuild every depending package if a bug is fixed in a library. It also allows depending packages to only pull in the platform it is going to use.

Maybe there is a better solution: how can we denote platform specefic mygui libraries? Right now, I was trying to make a platform agnostic libMyGUIEngine.so and then each platform as a seperate library. Is it better to distribute two shared libraries of libMyGUIEngine.so: libMyGUIEngine_ogre.so and libMyGUIEngineopengl.so? In which case, should all libraries actually be named libMyGUIEngine${PLATFORM}.a libMyGUIEngine_${PLATFORM}.so since different platforms are not binary compatible with each other? Thank you again.

ghost commented 10 years ago

I have no clue what you mean. The MyGUIEngine library is not using any specific platform.

Altren commented 10 years ago

I don't get it. There is platform independent MyGUIEngine library and there are MyGUI.OgrePlatform and MyGUI.OpenGL platform, one use Ogre, another use OpenGL. Same libMyGUIEngine.so can be used for both Ogre and OpenGL platforms.

maqifrnswa commented 10 years ago

Sorry for the confusion. I'm trying to find a way to distribute the platform libraries if they are not intended to be shared libraries. From a distribution point of view, they must be shared libraries. I was trying to think of some way of get both of those requirements to align.

Is it worth thinking about how to make mygui a shared library (both the engine and the platform libraries)? Since you all are closer to the code, I'm interested in your opinions on how to do it. The above patch seems to be working, at least with openmw compiled with gcc on linux. What is the Right Way of doing this?

ghost commented 10 years ago

I wouldn't say there's a right or wrong way. Static and shared libraries have different advantages and drawbacks. IMO an option for building platforms as shared libraries could be added to MyGUI, but then it needs to work properly on all systems, i.e. adding visibility/export qualifiers where needed. Altren is right that platforms are typically customized by the user (we actually do this in OpenMW, the OgreRenderManager had to be changed to support shaders), which makes a shared library somewhat less useful. But I understand shared libraries are still preferable from a distribution standpoint.

I guess for now you can keep your shared library change as a downstream patch. It is unlikely to cause problems for you, because I am not aware of any compilers on linux that hide symbols by default.

ghost commented 10 years ago

Can we close this issue now?