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 cmake issues with external libraries #74

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

ruffsl
Copy link
Contributor

@ruffsl ruffsl commented May 13, 2020

This fixes several cmake issues when using external libraries

  • Don't assume opengv headers are installed system wide
  • Include missing DBoW2 headers

@@ -134,6 +134,8 @@ target_include_directories(${PROJECT_NAME}
${GFLAGS_INCLUDE_DIRS}
${GLOG_INCLUDE_DIRS}
${GTSAM_INCLUDE_DIR}
${OPENGV_INCLUDE_DIR}
${DBoW2_INCLUDE_DIRS}
Copy link
Collaborator

Choose a reason for hiding this comment

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

@ruffsl I don't see why you need to put DBoW2 include dirs here, this should be already done when calling DBoW2::DBoW2 in target_link_libraries (are you using DBoW2 headers through Kimera?).

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I don't think that interface is working as you expect. My hunch is that the project was relying on and on finding the headers via the system global paths given the sketchy *sudo* make install

# Install DBoW2
RUN git clone https://github.com/dorian3d/DBoW2.git
RUN cd DBoW2 && \
mkdir build && \
cd build && \
cmake .. && \
make -j$(nproc) install

or depended on a erroneous limitation in legacy catkin for non-isolated devel space builds:

https://answers.ros.org/question/320613/catkin_make-vs-catkin_make_isolated-which-is-preferred/?answer=320706#post-id-320706

When using current build tools, each package must explicitly declare it's install steps given the use of sanitary/isolated builds. Thus every downstream package must similarly explicitly import the resources they need, as no cmake context it implicitly shared across workspace packages.

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.

None yet

2 participants