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

Don't bundle TinyXML #29

Closed
volter opened this issue Sep 11, 2011 · 4 comments
Closed

Don't bundle TinyXML #29

volter opened this issue Sep 11, 2011 · 4 comments

Comments

@volter
Copy link

volter commented Sep 11, 2011

Please don't bundle TinyXML.

I threw together a Cmake script and patched Cmakelists.txt:

http://www.geofrogger.net/review/osgEarth-2.0-FindTinyXML.cmake
http://www.geofrogger.net/review/osgEarth-2.0-tinyxml.patch

Note that the patch was written for version 2.0.0.

@jasonbeverage
Copy link
Collaborator

Hi Volter,

The whole point of us moving to TinyXML instead of the old Expat based
XML code was to avoid having to have yet another external dependency
to build against.

If you could send in a patch that could optionally allow us to build
against the embedded TinyXML or an external one if it was found I
think that would be more appropriate than removing the embedded
TinyXML completely.

Thanks,

Jason

On Sun, Sep 11, 2011 at 8:51 AM, volter
reply@reply.github.com
wrote:

Please don't bundle TinyXML.

I threw together a Cmake script and patched Cmakelists.txt:

http://www.geofrogger.net/review/osgEarth-2.0-FindTinyXML.cmake
http://www.geofrogger.net/review/osgEarth-2.0-tinyxml.patch

Note that the patch was written for version 2.0.0.

Reply to this email directly or view it on GitHub:
#29

@volter
Copy link
Author

volter commented Oct 12, 2011

I hope this works. I can't really test it because osgearth trunk doesn't build for me -- probably because of my OpenSceneGraph version.

The patch modifies CMakeLists.txt and src/osgearth/CMakeLists.txt and adds a find script in CMakeModules.

-D WITH_EXTERNAL_TINYXML causes the find script to run.

The find scripts has TINYXML_INCLUDE_DIR and TINYXML_LIBRARY. If both have values, TINYXML_FOUND is true.

If TINYXML_FOUND is true in src/osgearth/CMakeLists.txt, the bundled TinyXML files are not compiled. Therefore TINYXML_INCLUDE_DIR and TINYXML_LIBRARY are turned to use.

http://www.geofrogger.net/review/osgearth-external-tinyxml.patch

@volter volter closed this as completed Oct 12, 2011
@volter volter reopened this Oct 12, 2011
@jasonbeverage
Copy link
Collaborator

Hi Volter,

I've pushed this change, thanks!

Jason

On Wed, Oct 12, 2011 at 6:20 PM, volter
reply@reply.github.com
wrote:

I hope this works. I can't really test it because osgearth trunk doesn't build for me -- probably because of my OpenSceneGraph version.

The patch modifies CMakeLists.txt and src/osgearth/CMakeLists.txt and adds a find script in CMakeModules.

-D WITH_EXTERNAL_TINYXML causes the find script to run.

The find scripts has TINYXML_INCLUDE_DIR and TINYXML_LIBRARY. If both have values, TINYXML_FOUND is true.

If TINYXML_FOUND is true in src/osgearth/CMakeLists.txt, the bundled TinyXML files are not compiled. Therefore TINYXML_INCLUDE_DIR and TINYXML_LIBRARY are turned to use.

http://www.geofrogger.net/review/osgearth-external-tinyxml.patch

Reply to this email directly or view it on GitHub:
#29 (comment)

@jasonbeverage
Copy link
Collaborator

This is in the trunk now.

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

No branches or pull requests

2 participants