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

csm_eigen branch doesn't build #23

Open
efernandez opened this issue May 24, 2017 · 2 comments
Open

csm_eigen branch doesn't build #23

efernandez opened this issue May 24, 2017 · 2 comments

Comments

@efernandez
Copy link

I couldn't build the csm_eigen branch using catkin build. Indeed, the top CMakeLists.txt doesn't include (subdirs) the one in src, so the build passes but it doesn't do anything at all. See:
https://github.com/AndreaCensi/csm/blob/csm_eigen/CMakeLists.txt

Therefore, I've decided to fork and catkinize it. See this compare diff:
https://github.com/AndreaCensi/csm/compare/csm_eigen...clearpathrobotics:catkinize_csm_eigen?expand=1

@csprunk @RainerKuemmerle If you are OK with a change like that, I can open a PR.

@130s
Copy link
Collaborator

130s commented Jul 15, 2022

FYI, unsure if related though, #34 (and a few other PRs) are targeted to csm_eigen branch so worth looking into them.

Also more in general about branching I started a discussion. Please chime into #33 (comment)

@cst0
Copy link

cst0 commented Jul 15, 2022

I believe this is because both this branch and the master branch are configured as a 'pure' cmake package, causing cmake to produce a non-homogenous workspace. It does build successfully if manually triggered using cmake or catkin_make_isolated, and should work in a workspace with no other packages. That said, migrating to a catkin-based workspace seems sensible to me: it's already configured as a ROS package so there's no additional overhead but it does make things a bit more ROS-standard.

However: I do observe a conflict between these proposed changes and @malban's more recent ongoing work in #34 (in the root CMakeLists and the package.xml). As far as I can tell, though, #34 does everything the diff proposes on the condition that the buildtool is marked as being catkin (instead of cmake). So, my recommendation here would be to make that change over there and then close this issue once that's merged.

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

3 participants