Skip to content

Add tinyxml and Eigen3 as dependencies#34

Merged
Karsten1987 merged 10 commits intomasterfrom
orocos-kdl
Mar 22, 2017
Merged

Add tinyxml and Eigen3 as dependencies#34
Karsten1987 merged 10 commits intomasterfrom
orocos-kdl

Conversation

@Karsten1987
Copy link
Copy Markdown
Contributor

This installs libtinyxml-dev and libeigen3-dev as a dependency on the linux docker.
For mac, we have to install these packages via homebrew, for windows, we are currently in the process of writing choco packages for it.

@Karsten1987 Karsten1987 self-assigned this Mar 11, 2017
@Karsten1987 Karsten1987 added in progress Actively being worked on (Kanban column) in review Waiting for review (Kanban column) and removed in progress Actively being worked on (Kanban column) labels Mar 11, 2017
@mikaelarguedas
Copy link
Copy Markdown
Member

Should this wait for #32 and be added to the "only install the dependencies if appropriate" part ?

@Karsten1987
Copy link
Copy Markdown
Contributor Author

Karsten1987 commented Mar 21, 2017

windows: Build Status
linux: Build Status
The windows build are compiled with the choco packages for tinyxml and eigen3.

As discussed offline, I think these changes should go into the ros2.repos as I consider the robot_state_publisher a critical package which should be shipped to all platforms.

@dirk-thomas
Copy link
Copy Markdown
Member

Is there a reason not to use TinyXML2 (libtinyxml2-dev)? Newer versions of FastRTPS use version 2 so it would be good if we use it here too.

@Karsten1987
Copy link
Copy Markdown
Contributor Author

That would require a change in URDFdom / URDFdom_headers / orocos_kdl / robot_model. For now, we only have minor changes to these packages.

@dirk-thomas
Copy link
Copy Markdown
Member

dirk-thomas commented Mar 21, 2017

I am not sure if both versions can be used at the same time which would be necessary as soon as we update to the latest FastRTPS version again (if they are exposed transitively through the headers).

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Mar 21, 2017

TinyXML and TinyXML2 are completely separate. They can be used at the same time. We already have this case in ROS 1, where urdf uses tinyxml(1) and other stuff uses tinyxml2. On my machine, both are installed:

william@farl ~
% brew list tinyxml
/usr/local/Cellar/tinyxml/2.6.2/include/tinyxml.h
/usr/local/Cellar/tinyxml/2.6.2/lib/libtinyxml.dylib
/usr/local/Cellar/tinyxml/2.6.2/lib/pkgconfig/tinyxml.pc

william@farl ~
% brew list tinyxml2
/usr/local/Cellar/tinyxml2/4.0.1/include/tinyxml2.h
/usr/local/Cellar/tinyxml2/4.0.1/lib/libtinyxml2.4.0.1.dylib
/usr/local/Cellar/tinyxml2/4.0.1/lib/pkgconfig/tinyxml2.pc
/usr/local/Cellar/tinyxml2/4.0.1/lib/ (2 other files)

I believe it is the same on Linux.

@Karsten1987
Copy link
Copy Markdown
Contributor Author

So it compiles on all platforms, can anybody give feedback on that?
ros2/demos#118

@wjwwood
Copy link
Copy Markdown
Member

wjwwood commented Mar 22, 2017

I believe it is the same on Linux.

Can someone verify this that has a Xenial system handy?

Also, don't forget to update the installation (binary and source) guide on the wiki. Also for macOS and manually install these on the macOS Jenkins nodes.

@Karsten1987
Copy link
Copy Markdown
Contributor Author

didn't check for symbol collision, but so far:

karsten@karsten-mint ~ $ ll /usr/include/tinyxml*
-rw-r--r-- 1 root root 62897 Jul 30  2015 /usr/include/tinyxml2.h
-rw-r--r-- 1 root root 64887 Aug  5  2015 /usr/include/tinyxml.h

karsten@karsten-mint ~ $ ll /usr/lib/x86_64-linux-gnu/libtinyxml*
-rw-r--r-- 1 root root 139980 Jul 30  2015 /usr/lib/x86_64-linux-gnu/libtinyxml2.a
lrwxrwxrwx 1 root root     16 Jul 30  2015 /usr/lib/x86_64-linux-gnu/libtinyxml2.so -> libtinyxml2.so.2
lrwxrwxrwx 1 root root     20 Jul 30  2015 /usr/lib/x86_64-linux-gnu/libtinyxml2.so.2 -> libtinyxml2.so.2.2.0
-rw-r--r-- 1 root root  76296 Jul 30  2015 /usr/lib/x86_64-linux-gnu/libtinyxml2.so.2.2.0
-rw-r--r-- 1 root root 163194 Aug  5  2015 /usr/lib/x86_64-linux-gnu/libtinyxml.a
lrwxrwxrwx 1 root root     19 Aug  5  2015 /usr/lib/x86_64-linux-gnu/libtinyxml.so -> libtinyxml.so.2.6.2
-rw-r--r-- 1 root root  88768 Aug  5  2015 /usr/lib/x86_64-linux-gnu/libtinyxml.so.2.6.2

@mikaelarguedas
Copy link
Copy Markdown
Member

Yep, I know that several ROS1 packages depend on tinyxml2 while several low level packages in ROS depend on tinyxml. They are different packages and can coexist without problem

Copy link
Copy Markdown
Member

@wjwwood wjwwood left a comment

Choose a reason for hiding this comment

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

lgtm then

@Karsten1987 Karsten1987 merged commit 2846008 into master Mar 22, 2017
@Karsten1987 Karsten1987 removed the in review Waiting for review (Kanban column) label Mar 22, 2017
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

Successfully merging this pull request may close these issues.

4 participants