search

czaloj / bullet

Automatically exported from code.google.com/p/bullet
0 stars 0 forks source link

Shared library and OS X Framework support #129

Closed GoogleCodeExporter closed 9 years ago

GoogleCodeExporter commented 9 years ago 
Currently, building shared libraries fails due to unresolved symbols during 
linking.

This patch will tweak the CMakeList.txt files to list appropriate dependencies 
for linking.

One item of note is in Extras/LibXML/xmlmodule.h, I changed the #if 0 to 
#ifndef _WIN32, 
otherwise it would complain about the functions in that section being 
undefined.  According to the 
previous patch comments, the #if 0 was added to support compilation under 
MinGW, hopefully this 
will maintain that, but still provide compilation on other platforms.

Alternatively, perhaps cmake could be set to run 'configure' on libxml, so it 
will do the "right thing" 
on each platform instead of trying to use the same libxml/config.h for all 
platforms?

Tested on OS X and Linux

Original issue reported on code.google.com by ejtt...@gmail.com on 4 Nov 2008 at 5:26

Attachments:

GoogleCodeExporter commented 9 years ago 

Thanks a lot. So by default we stick with static libraries? How would the 
command-
line be to generate dynamic libs for CMake?

So far there has be zero complaints about compilation of libxml/COLLADA-DOM on 
any 
platform, so it would be good to keep it that way. Going the 'configure' route 
could 
open a can of (support) worms.

Is this #if 0 change only necessary for dynamic linkage? Building static 
libraries 
for Extras/libxml using the included "config.h" seem to compile on pretty much 
every 
platform. 

If so, can you replace the #if0/#ifndef _WIN32 by #ifdef 
BT_USING_SHARED_LINKAGE, and 
define thatby the CMake build system? Just to make sure we don't break anything?

Thanks again

Original comment by erwin.coumans on 4 Nov 2008 at 7:16

GoogleCodeExporter commented 9 years ago 
Yes, by default everything is the same, it builds a static library.

To enable a dynamic/shared library, run:
cmake -D BUILD_SHARED_LIBS=true
I verified the demos and such work too... they just load the .dylib/.so as you 
would expect, so HelloWorld is 
~80K instead of ~2.5MB.  This might be useful for developers working on bullet, 
can rebuild one of the bullet 
libraries and then test the demos without having to relink all of the 
executables...

Under OS X, you can generate frameworks instead of "naked" .dylib's with:
cmake -D BUILD_SHARED_LIBS=true -D FRAMEWORK=true

I may followup with another patch that will combine the separate frameworks 
into an umbrella framework for 
easier installation... first want to try using it myself ;)

Original comment by ejtt...@gmail.com on 4 Nov 2008 at 8:24

GoogleCodeExporter commented 9 years ago 

That sounds good. We are just about to prepare a 2.73 release, so we need to 
check 
if this dynamic flags goes into 2.73 or 2.74 (next month).

>>I changed the #if 0 to #ifndef _WIN32 

I'm more concerned about this change in in Extras/LibXML/xmlmodule.h. Can you 
replace the 
#ifndef _WIN32 (formerly #if 0)
into
#ifdef BT_BUILD_SHARED_LIBS
is this BUILD_SHARED_LIBS recognized by the compiler, or can you add some 
preprocessor define in the CMake builds only when creating dynamic libs, so 
that 
there is guaranteed no difference for static builds for this xml thing?

Thanks again,
Erwin

Original comment by erwin.coumans on 4 Nov 2008 at 8:43

GoogleCodeExporter commented 9 years ago 
Alternatively, if none of the extras/demos are using the xml "modules" feature 
we could just disable it and 
avoid the issue altogether...
(disable LIBXML_MODULES_ENABLED in Extras/LibXML/include/libxml/xmlversion.h)

I tried this, everything still builds, so it should be safe — if there was 
actually anything using the libxml 
modules feature, it would be complaining about the missing symbols.

I prefer this solution as it guarantees nothing tries to use the non-portable 
feature. (non-portable with a 
static config.h anyway)  Also imported demos/extras will get accurate feedback 
on the status of xmlmodule 
via the xmlversion definitions, so if any do make use of xmlmodule, they can at 
least check xmlversion and 
choose to disable features...

Original comment by ejtt...@gmail.com on 4 Nov 2008 at 9:14

Attachments:

GoogleCodeExporter commented 9 years ago 
We can disable LIBXML_MODULES_ENABLED.

The only remaining thing is: the new INSTALL feature requires CMake 2.6, and I 
want 
to keep Cmake 2.4 compatibility.

See this newly fixed issue:
http://code.google.com/p/bullet/issues/detail?id=130

Hopefully your patch is compatible with this.

Original comment by erwin.coumans on 5 Nov 2008 at 2:36

GoogleCodeExporter commented 9 years ago 
Yeah, the INSTALL thing is orthogonal.
I actually ran into the FILES_MATCHING issue myself because my linux machine 
only had CMake 2.4 (Fedora 
7)... had to compile CMake 2.6 from source since there wasn't a 2.6 package 
available :-/  So I can see your 
concern there.

But I think if you just comment out the FILES_MATCHING lines, it shouldn't 
affect the framework stuff.

However, I've discovered the framework patch isn't quite working at the 
moment... the generated frameworks 
are missing the header files, and the 'quick fix' copies them as a flat 
directory, which obviously breaks a 
bunch of the internal #includes... but I've figured out the workaround (not too 
ugly), so I'll be sending another 
patch.  Should be ready tomorrow.

Original comment by ejtt...@gmail.com on 5 Nov 2008 at 4:57

GoogleCodeExporter commented 9 years ago 
So, the basic solution to having header subdirectories is to list the headers 
for each directory separately, and 
then set the source properties for each group to go to the appropriate location.

I think this style of solution can also be used for the INSTALL commands, thus 
avoiding the FILES_MATCHING 
issue.

With this patch, I can now compile HelloWorld in a separate project, linking 
against the frameworks.  Ta-Da!

However, there is one final hangup, which should probably be addressed in a 
separate patch: I wind up having 
to manually rename the produced framework, the binary file within the 
framework, and the framework's 
symlink to the current binary.  The problem here is that the directory names 
are BulletCollision, 
BulletDynamics, etc., but the target names within CMake are LibBulletCollision, 
LibBulletDynamics, etc.  That 
extra "Lib" breaks the symmetry between produced file structures and the 
directory names.  It also means you 
get silly products like libLibLinearMath.so.

I would recommend globally dropping the "Lib" from the CMake target names.

Original comment by ejtt...@gmail.com on 5 Nov 2008 at 4:55

Attachments:

GoogleCodeExporter commented 9 years ago 
This patch conflicts with the latest trunk, can you try to create it again for 
the 
latest trunk?

The autoconf and jam generated libraries are libbulletmath, libbulletcollision 
and 
libbulletdynamics. They are not exactly matching with the directory names 
indeed.
So we should drop the Lib indeed, and remove capitals from the lib names. This 
has 
been suggested in this issue:

http://code.google.com/p/bullet/issues/detail?id=108

So using bulletmath, bulletcollision and bulletdynamics etc. in CMakeList.txt 
gives 
libbulletmath.a (or .lib) etc, which is consistent with Jam and autoconf/make.

Thanks,
Erwin

Original comment by erwin.coumans on 5 Nov 2008 at 10:03

GoogleCodeExporter commented 9 years ago 
Are you sure I'm conflicting with the trunk?  I'm at revision 1458:
bullet$ svn update
At revision 1458.
bullet$ svn diff > ~/Desktop/bullet-shared-noxmlmodule-3.diff
bullet$ diff ~/Desktop/bullet-shared-noxmlmodule-{2,3}.diff
<no output>

Original comment by ejtt...@gmail.com on 5 Nov 2008 at 10:17

GoogleCodeExporter commented 9 years ago 
> and remove capitals from the lib names
I would hesitate on this one... it wouldn't match the directory names, which 
would still cause some nuisance for 
frameworks.  For standardization, maybe caps can be added in Jam and autoconf...

I know caps are less common in unix-style libraries, but it's not entirely 
unheard of either... (e.g. the ICE libraries 
from ZeroC)  Keeping the caps would be a little easier for overall portability.

Original comment by ejtt...@gmail.com on 5 Nov 2008 at 10:24

GoogleCodeExporter commented 9 years ago 

Why do folder names need to match with library names for frameworks?

Original comment by erwin.coumans on 5 Nov 2008 at 10:46

GoogleCodeExporter commented 9 years ago 

my bad, forgot to svn update on this PC (too many different develop 
environments).

I'll apply the patch. So this 'HAVE_DLOPEN', do we still need to 'enable' it 
with #if 
1, or can we leave the #if 0, now we disabled LIBXML_MODULES_ENABLED?

We can remove the Lib prefix (keeping the capitalization), and  worry about the 
autoconf/jam/CMake consistency after the 2.73 release. Unless someone wants to 
tackle 
autoconf/Jam and add the capitalization?

Original comment by erwin.coumans on 5 Nov 2008 at 10:56

GoogleCodeExporter commented 9 years ago 
When a user does #include <BulletDynamics/foo.h>, this searches for either a 
"BulletDynamics" directory in a 
standard include directory, or a framework named "BulletDynamics".  So to use a 
framework portably, the 
name of the framework has to match the 'include' sub-directory which is used on 
other platforms.  And the 
name of the framework produced by CMake is determined by the name of the 
target.  Thus, life is good iff the 
target name matches the directory name... :-/

(perhaps there's a CMake command to specify a different name for a framework 
produced by a target?  Or 
alternatively to change the name of the library file to make that all 
lower-case?  I have been unable to find 
such a command/property...)

Original comment by ejtt...@gmail.com on 5 Nov 2008 at 11:02

GoogleCodeExporter commented 9 years ago 

Why is there this lines?
SET(Root_HDRS
    ../btBulletDynamicsCommon.h
    ../btBulletCollisionCommon.h
)

Those headerfiles should already been installed in the Bullet/src/CMakeList.txt.

Probably we can remove the 2.6 condition for this one (in 
Bullet/src/CMakeList.txt)

#INSTALL of other files requires CMake 2.6^M
IF (${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION} GREATER 2.6)
        INSTALL(FILES btBulletCollisionCommon.h btBulletDynamicsCommon.h Bullet-C-
Api.h DESTINATION include)
ENDIF (${CMAKE_MAJOR_VERSION}.${CMAKE_MINOR_VERSION} GREATER 2.6)

Original comment by erwin.coumans on 5 Nov 2008 at 11:05

GoogleCodeExporter commented 9 years ago 
> I'll apply the patch. So this 'HAVE_DLOPEN', do we still need to 'enable' it 
with #if 
I think the #if 0 / #if 1 thing was added by Bullet (err, /you/ actually), so 
I'd say just to drop it altogether to go 
back closer to the libxml original... make life easier if you want to upgrade 
the embedded libxml source 
someday.

The LIBXML_MODULES_ENABLED flag is disabled now, so the whole file is skipped 
due to the #ifdef at the 
beginning of xmlmodule.c.  So really you can do whatever you want in there ;)

Original comment by ejtt...@gmail.com on 5 Nov 2008 at 11:08

GoogleCodeExporter commented 9 years ago 
When the Lib prefix is removed, it looks very cluttered in a MSVC solution: you 
can't  
distinguish libraries from demos anymore. Should we prefix demos with "App", 
like 
"AppBasicDemo"?

Original comment by erwin.coumans on 5 Nov 2008 at 11:13

GoogleCodeExporter commented 9 years ago 
> Those headerfiles should already been installed in the 
Bullet/src/CMakeList.txt.
Umm, I guess you could do it there too.  Just depends if you want to keep the 
target's file list all in one place 
(e.g. the BulletDynamics/CMakeList.txt file) vs. keeping all references to a 
file in the peer-level CMakeList.txt.

(i.e. should targets pull their files, or should the files push to the targets) 
 Whatev :)

Original comment by ejtt...@gmail.com on 5 Nov 2008 at 11:16

GoogleCodeExporter commented 9 years ago 
> Probably we can remove the 2.6 condition for this one (in 
Bullet/src/CMakeList.txt)
> [...]
> Those headerfiles should already been installed in the 
Bullet/src/CMakeList.txt.

Oh, I just put together what you're talking about here... I don't think the 
install() stuff applies to framework 
construction, so we would need to add some
    SET_TARGET_PROPERTIES(<target> PROPERTIES PUBLIC_HEADER "<files>")
lines in the src/CMakeList.txt.  The corresponding lines could then be removed 
from the sub-CMakeList files.

If you want to move them there, I could submit another patch.

But either way, the src/CMakeList.txt file looks 2.4 safe, so don't really need 
the 2.6 check...

Original comment by ejtt...@gmail.com on 5 Nov 2008 at 11:25

GoogleCodeExporter commented 9 years ago 
The patch has been applied, but some more work is needed, to remove Lib prefix, 
and 
root headerfiles are installed twice now (needs cleanup)

http://code.google.com/p/bullet/source/detail?r=1459

Thanks for the contribution.

Can you add some wiki docs on how to use those Cmake features?

Original comment by erwin.coumans on 6 Nov 2008 at 6:05

GoogleCodeExporter commented 9 years ago 

Lib prefix has been removed. One remaining inconsistency is that LibXML folder 
includes the 'Lib'.

Please check:
http://code.google.com/p/bullet/source/detail?r=1460

Original comment by erwin.coumans on 6 Nov 2008 at 6:56

GoogleCodeExporter commented 9 years ago 
> Can you add some wiki docs on how to use those Cmake features?
I updated the wiki.
http://www.continuousphysics.com/mediawiki-1.5.8/index.php?title=Installation
Should probably link to this from the doxygen main page where there's currently 
some installation info.

I was going to add something about installing, but I can't figure out to 
trigger the installation.
'make install' always complains about "No rule to make target `install'", and 
'make preinstall; cmake -P cmake_install.cmake' doesn't actually do anything.  
I've tried this on both OS X (disabling frameworks too) and 
linux... I must be missing something.

Original comment by ejtt...@gmail.com on 6 Nov 2008 at 5:38

GoogleCodeExporter commented 9 years ago 
Thanks, let's close the issue and continue discussion on the forums.

Original comment by erwin.coumans on 6 Nov 2008 at 7:54