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

Fix CMakeLists + package.xmls #548

Merged
merged 3 commits into from
Jul 31, 2017

Conversation

mintar
Copy link
Contributor

@mintar mintar commented Dec 1, 2016

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes #537 .

@spmaniato
Copy link

spmaniato commented Dec 1, 2016

@mintar Were there no complaints about move_base ? See this comment in #483

@mintar
Copy link
Contributor Author

mintar commented Dec 2, 2016

@spmaniato I didn't even run catkin_lint on move_base, since with this PR it compiles for me with rosjava installed. But one should definitely go through all the packages again and fix all catkin_lint errors.

Some more remarks (they may be obvious, but nevertheless):

  • The bug Indigo build fails when ROS java is installed #537 only occurs when rosjava is installed and navigation is checked out as a source package into the Catkin workspace you're building.
  • Even after applying this PR, I initially need to run catkin_make 2-3 times before it eventually succeeds due to some problems with dynamic_reconfigure. (Without this PR, it doesn't build at all.)

@mintar
Copy link
Contributor Author

mintar commented Dec 21, 2016

Ping

@caicaixia
Copy link

Hello, these changes haven't released yet, right? I just upgrade the ros-indigo-navigation, but the version is: 1.12.13-0trusty-20161128-101040-0800. And still have ros java problem :(

@mintar
Copy link
Contributor Author

mintar commented Jan 11, 2017

@caicaixia : No, I'm still waiting for this PR to be merged. But this bug only concerns source installs anyway, so it doesn't matter if this is released or not.

Copy link
Member

@DLu DLu left a comment

Choose a reason for hiding this comment

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

Overall, looks good. I merged things in the wrong order (see #549), so could you please merge the latest indigo-devel?

@@ -60,8 +61,6 @@ generate_dynamic_reconfigure_options(
catkin_package(
INCLUDE_DIRS
include
${EIGEN_INCLUDE_DIRS}
${PCL_INCLUDE_DIRS}
Copy link
Member

Choose a reason for hiding this comment

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

Why are these removed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Because there already is a catkin_package(DEPENDS PCL Eigen). This means that PCL + Eigen are already added as dependencies for any project that depends on costmap_2d, and it's Eigen's (resp. PCL's) job to export their own include dirs. The catkin_package(INCLUDE_DIRS) should only export our own include dirs.

Removing these two lines makes the following catkin_lint error go away:

costmap_2d: CMakeLists.txt(61): error: catkin_package() exports non-package include path
     * You listed one or more include paths in the INCLUDE_DIRS stanza
     * of your catkin_package() call which are not part of your
     * package. If you want to export include paths of other modules,
     * use find_package(), find_path(), and/or find_library() and add
     * the dependency to the DEPENDS stanza.

@mintar
Copy link
Contributor Author

mintar commented Jan 25, 2017

I've rebased this branch onto the latest indigo-devel, so it should be good to merge.

@mintar
Copy link
Contributor Author

mintar commented Jan 25, 2017

I've just added two more commits that fix 87 out of 90 catkin_lint errors (the rest are false positives).

@mikeferguson
Copy link
Contributor

@mintar -- could you rebase this on top of latest changes (there is a conflict) and then we can get this merged. Thanks!

mintar added 3 commits May 7, 2017 12:24
This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes ros-planning#537 .
This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
@mintar
Copy link
Contributor Author

mintar commented May 7, 2017

@mikeferguson Ok, rebased!

@mintar
Copy link
Contributor Author

mintar commented Jun 1, 2017

ping

@mikeferguson
Copy link
Contributor

mikeferguson commented Jul 31, 2017

Test failures are unrelated. I'm going to squash-merge to make it easier to forward port to K+L

@mikeferguson mikeferguson merged commit d443c44 into ros-planning:indigo-devel Jul 31, 2017
mikeferguson pushed a commit that referenced this pull request Jul 31, 2017
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes #537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
mikeferguson pushed a commit that referenced this pull request Jul 31, 2017
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes #537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
mikeferguson pushed a commit that referenced this pull request Jul 31, 2017
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes #537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
mikeferguson pushed a commit that referenced this pull request Jul 31, 2017
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes #537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
mikeferguson pushed a commit that referenced this pull request Jul 31, 2017
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes #537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
mikeferguson pushed a commit that referenced this pull request Jul 31, 2017
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes #537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
mikeferguson added a commit that referenced this pull request Aug 1, 2017
mikeferguson pushed a commit that referenced this pull request Aug 1, 2017
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes #537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
mikeferguson added a commit that referenced this pull request Aug 1, 2017
@mintar
Copy link
Contributor Author

mintar commented Aug 1, 2017

Hooray! :-)

@mintar mintar deleted the catkin_fixes branch October 29, 2017 13:26
gerkey pushed a commit to codebot/navigation that referenced this pull request Jan 19, 2018
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes ros-planning#537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
johaq pushed a commit to CentralLabFacilities/navigation that referenced this pull request Mar 30, 2018
* Fix CMakeLists + package.xmls

This fixes compilation errors when rosjava is installed on the system.
It also removes a lot of errors reported by catkin_lint.

Fixes ros-planning#537 .

* Fix more CMakeLists.txt + package.xmls

* amcl: disambiguate include path

This fixes the following catkin_lint warning:

    amcl: warning: include paths 'include/amcl/map' and 'include' are ambiguous
         * You have used two include paths where one is a parent of the
         * other. Thus the same headers can be included with two different
         * include paths which may confuse users. It is recommended that
         * you keep your include paths consistent.
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.

5 participants