Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Improve compatibility with OS X #5

Open
wants to merge 7 commits into
base: master
Choose a base branch
from
Open

Improve compatibility with OS X #5

wants to merge 7 commits into from

Conversation

D-Alex
Copy link
Member

@D-Alex D-Alex commented Sep 6, 2014

No description provided.

@@ -222,6 +227,9 @@ endmacro()
macro (rock_find_qt4)
find_package(Qt4 REQUIRED QtCore QtGui QtOpenGl ${ARGN})
include_directories(${QT_HEADERS_DIR})
include_directories(${QT_QTCORE_INCLUDE_DIR})
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@D-Alex: These should already be included in the foreach() just below.

@doudou
Copy link
Member

doudou commented Jan 29, 2015

@D-Alex: Ping. Could you have a look at this Qt thing ?

Alexander Duda added 2 commits January 29, 2015 22:53
@D-Alex
Copy link
Member Author

D-Alex commented Jan 29, 2015

sounds fair - I updated the pull request

@doudou
Copy link
Member

doudou commented Feb 12, 2015

Sorry I am taking so long with this one. It breaks compilation on non-OSX targets because one cannot link a module to another module.

This is a major problem with the qt stuff, as we need to compile with MODULE to allow loading through the qt plugin system, and with SHARED to use a widget directly. We'll probably have to compile the plugin separately from the shared library.

@goldhoorn
Copy link
Contributor

One general question which came me just in mind.
Why we definning SHARED for all our libraries manually in the rock-cmakes.
By doing so we cannot control it from external anymore.

The "Normal" way (then we can influence the build-type from autobuild/autoproj) could be by seeting (if needed packagewise):

SET(BUILD_SHARED_LIBS ON)

@doudou
Copy link
Member

doudou commented Feb 12, 2015

The "Normal" way (then we can influence the build-type from autobuild/autoproj) could be by seeting (if needed packagewise):

I would not mind changing this. Now, switching to static libs usually does not work because of PIC issues, linking order, ... In other words, if the goal is to support building Rock with static libs, we would have to IMO be serious about it.

@D-Alex
Copy link
Member Author

D-Alex commented Feb 12, 2015

You are right. But linking against libraries in the install/qt/desginer folder is very ugly anyway. Do you think changing it would introduce to much trouble? Every one has probably to recompile its installation.

Loadable modules don't need a pkg-config file (actually, the
file is basically useless). In the end, only rock_library needs
to generate the file..

This commit moves the call to rock_prepare_pkgconfig to rock_library.
It also makes the call strict (you have to mean it if you call it),
and promotes the warning about a missing pkg-config file (now in
rock_library) to WARNING
@doudou
Copy link
Member

doudou commented Feb 12, 2015

Wait ...

The one thing is this SHARED stuff ... That I would leave it to people that have more experience than me about linking with static libs (my experience being "it really does not work out of the box")

The other one is this module vs. shared lib thing. I am preparing a patch to split the vizkit plugins (widget & vizkit3d) into a shared lib part in lib/ and a module part installed wherever it is expected. The problem, however, is that it requires to rename the plugin lib (I currently append -plugin) because they're both built within the same folder (having two targets with the same output name in the same folder is something cmake does not support). Which means that we'll either have to do quite a bit more work, or have to update vizkit to search files with this plugin. While we're at it, we should probably move the vizkit3d plugins into their own subfolder of lib/

Thoughts ?

"Recent" cmake (post-2.2) will refuse to link to a module. Since
MacOSX requires to make the distinction between modules and
shared libs, we have to split the Qt plugins between the module
part and the shared lib part.

This introduces a backward compatible mode, where we basically
build the shared lib twice. However, if the rock_vizkit_widget
stanza is updated to use the new QTPLUGIN <sourcefile> flag, it
then links the module to the shared lib, only adding the qtplugin
file to the module.

To make this work, I had to start installing the vizkit3d plugins
from lib/ to lib/vizkit3d - to avoid name clashes between the
pure shared library and the plugin
@doudou
Copy link
Member

doudou commented Feb 12, 2015

Related pull request:

https://github.com/D-Alex/base-cmake/pull/1

D-Alex and others added 2 commits February 21, 2015 23:08
@D-Alex
Copy link
Member Author

D-Alex commented Feb 21, 2015

works fine on mac os - thx
👍

@doudou
Copy link
Member

doudou commented Mar 3, 2015

Actually, this is broken. On cmake 2.8 (Ubuntu 14.10) CMake randomly picks either of the two libraries (lib or module) and installs it in both places.

We'll either have to find a workaround, or install the plugin with a suffix. I would prefer a workaround, but I don't really know what to try ...

@goldhoorn
Copy link
Contributor

Is this PR still needed after:
#14
and
https://github.com/rock-core/base-cmake/pull/12/files

or can it be closed?

@D-Alex, @malter

@doudou
Copy link
Member

doudou commented May 5, 2015

Yes. This is unrelated. The main issue here is that vizkit3d plugins (qt plugins) cannot be used as pure shared libraries on MacOSX, which is troublesome. We would have to create both a module and a library, but cmake has a bug that makes it fail. So, now, someone would have to test a patched vizkit where the Qt plugins would have a suffix (e.g. -module). Or find a better way.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants