gazebosim / gazebo-classic

Gazebo classic. For the latest version, see https://github.com/gazebosim/gz-sim
http://classic.gazebosim.org/
Other
1.19k stars 480 forks source link

Installed ode headers have wrong path #892

Closed osrf-migration closed 8 years ago

osrf-migration commented 11 years ago

Original report (archived issue) by John Hoare (Bitbucket: jhoare).


When gazebo 2.0's headers get installed, they're installed into /usr/include/gazebo-2.0/gazebo, but, the ode header files include "ode/filename.h" which is not in the header search path, since they are in the "ode" folder inside the "gazebo" folder. The ode headers should include other ode headers using the path: "gazebo/ode/filename.h" for them to be found.

osrf-migration commented 11 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Perhaps the ode headers should not be installed to the /usr/include/gazebo-2.0/gazebo/ prefix, but rather to /usr/include/gazebo-2.0/. This installation is currently done in deps/opende/CMakeLists.txt using the gz_install_includes macro.

osrf-migration commented 11 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


In the meantime, we could patch the gazebo_ode.in pkgconfig template to have the gazebo folder in the include path. Did you use pkgconfig to search for the gazebo_ode library?

osrf-migration commented 11 years ago

Original comment by Steve Peters (Bitbucket: Steven Peters, GitHub: scpeters).


Here's a patch that changes the header install location:

diff -r da99b2dca3657b9406ed71656b8ff6f7d85b1c7e deps/opende/CMakeLists.txt
--- a/deps/opende/CMakeLists.txt    Wed Oct 09 23:47:55 2013 -0700
+++ b/deps/opende/CMakeLists.txt    Sun Oct 13 14:04:57 2013 -0700
@@ -254,4 +254,4 @@
 add_dependencies(gazebo_ode gazebo_opende_ou)

 gz_install_library(gazebo_ode)
-gz_install_includes("ode" ${headers})
+gz_install_includes("../ode" ${headers})
diff -r da99b2dca3657b9406ed71656b8ff6f7d85b1c7e deps/opende/GIMPACT/CMakeLists.txt
--- a/deps/opende/GIMPACT/CMakeLists.txt    Wed Oct 09 23:47:55 2013 -0700
+++ b/deps/opende/GIMPACT/CMakeLists.txt    Sun Oct 13 14:04:57 2013 -0700
@@ -39,5 +39,5 @@

 gz_add_library(gazebo_gimpact ${sources})
 gz_install_library(gazebo_gimpact)
-gz_install_includes("GIMPACT" ${headers})
+gz_install_includes("../GIMPACT" ${headers})

diff -r da99b2dca3657b9406ed71656b8ff6f7d85b1c7e deps/opende/OPCODE/CMakeLists.txt
--- a/deps/opende/OPCODE/CMakeLists.txt Wed Oct 09 23:47:55 2013 -0700
+++ b/deps/opende/OPCODE/CMakeLists.txt Sun Oct 13 14:04:57 2013 -0700
@@ -85,4 +85,4 @@

 gz_add_library(gazebo_opcode ${sources})
 gz_install_library(gazebo_opcode)
-gz_install_includes("opcode" ${headers})
+gz_install_includes("../opcode" ${headers})
osrf-migration commented 11 years ago

Original comment by John Hoare (Bitbucket: jhoare).


I used find_package to find gazebo. Also, the gazebo find_package component does not report its version, so if I request version 2.0 it fails because the reported version is "unknown"

osrf-migration commented 11 years ago

Original comment by John Hoare (Bitbucket: jhoare).


If gazebo's ode is different than vanilla ode then I would argue that gazebo's ode should be included more explicitly (e.g. by #include<gazebo/ode/filename.hh>) but thats just my opinion.

osrf-migration commented 11 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


ODE headers are installed into /usr/include/gazebo-2.0/gazebo/ode, which I believe is the correct location.

We need to change the ODE includes to use gazebo/ode/*.h instead of ode/*.h

osrf-migration commented 11 years ago

Original comment by John Hoare (Bitbucket: jhoare).


Thanks Nate. As Steve alluded to, I believe the issue is with other libraries used by gazebo too, I only encountered it with ode because it happened to be what I was including at the time.

osrf-migration commented 8 years ago

Original comment by Martin Pecka (Bitbucket: peci1).


Guys, this has still been a pretty annoying issue for me in Gazebo 7. Do you have any ideas on how to solve it? I agree the above patch is not ideal, because it could spoil someone else's code. However, users should be given the possibility to include Gazebo's version of headers, ideally by #include <gazebo/ode/ode.h>, which unfortunately does not work.

Until this is fixed in upstream, I use a workaround like this one:

#!c++
list(GET GAZEBO_INCLUDE_DIRS 1 GAZEBO_INCLUDE_HACK)
include_directories(${Boost_INCLUDE_DIR} ${GAZEBO_INCLUDE_DIRS} ${GAZEBO_INCLUDE_HACK}/gazebo)

This allows me to specifically ask to use Gazebo's ODE headers for projects that need them, and leave other projects "just using Gazebo" without the possibly unwanted ODE path setting. I don't know if this is better for Gazebo than patching all ODE files.

If you like this idea, we might provide variables like GAZEBO_ODE_INCLUDE_DIRS for everybody to use when he likes it.

osrf-migration commented 8 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


Working on a solution in the ode_includes branch.

The solution will have ODE includes use #include <gazebo/ode/filename.h>

Gazebo's ODE header files will be installed into /usr/include/gazebo-8/gazebo/ode.

We won't be able to put this change into gazebo7.

osrf-migration commented 8 years ago

Original comment by Martin Pecka (Bitbucket: peci1).


Okay, sounds like the cleanest way of solving this issue. -- Martin Pecka

osrf-migration commented 8 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


See pull request #2186

osrf-migration commented 8 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


Resolved in pull request #2186

osrf-migration commented 8 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).


osrf-migration commented 8 years ago

Original comment by Nate Koenig (Bitbucket: Nathan Koenig).