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

enable kobuki Windows build #7

Closed
wants to merge 4 commits into from

Conversation

kejxu
Copy link

@kejxu kejxu commented Jul 17, 2019

this library needs to be updated in order to enable the kobuki code base to build on Windows

  • -std= is not supported on MSVC, use the compiler-agnostic CMAKE_CXX_STANDARD flag instead

this port points to the melodic release branch because it currently only targets ROS1.melodic

@stonier
Copy link
Owner

stonier commented Aug 18, 2019

I've bumped the logic for requiring and enabling (in a cross platform way) cxx14 on the devel branch in #9 and cx11 in #10. I can backport that logic to melodic, however, did you actually use it anyhwhere? I don't see it being used in stonier/ecl_lite#27 or stonier/ecl_core#87.

@kejxu
Copy link
Author

kejxu commented Aug 19, 2019

I've bumped the logic for requiring and enabling (in a cross platform way) cxx14 on the devel branch in #9 and cx11 in #10. I can backport that logic to melodic

Sounds great!

however, did you actually use it anyhwhere? I don't see it being used in stonier/ecl_lite#27 or stonier/ecl_core#87.

I think these macros are already being used in ecl_core (and there should be more instances like this). Since the original logic (ecl_check_for_cxx11_compiler) hard codes the -std= flag, this change was made to enable compiling with MSVC

@kejxu
Copy link
Author

kejxu commented Aug 19, 2019

for the other question, MSVC uses /std:c++ flags, but as you mentioned, anything cross-platform might be the desirable solution so that we could let CMake do the dirty work.

@stonier
Copy link
Owner

stonier commented Aug 20, 2019

Aha, I found the few references in the melodic branch enabling cxx11.

./ecl_geometry/CMakeLists.txt:49:ecl_enable_cxx11_compiler()
./ecl_core_apps/CMakeLists.txt:38:ecl_enable_cxx11_compiler()
./ecl_statistics/CMakeLists.txt:41:ecl_enable_cxx11_compiler()
./ecl_linear_algebra/CMakeLists.txt:48:ecl_enable_cxx11_compiler()
./ecl_time/CMakeLists.txt:40:ecl_enable_cxx11_compiler()
./ecl_devices/CMakeLists.txt:39:ecl_enable_cxx11_compiler()

I'll backport the new cross platform api and see how it handles it.

@stonier
Copy link
Owner

stonier commented Aug 21, 2019

Cross platform approach in 45edeb9. Released and tagged as v0.61.8 for melodic.

Closing this PR in favour of that, if you have any issues with this, please open a new issue.

Moving on to dealing with the other ecl repositories.

@stonier stonier closed this Aug 21, 2019
@kejxu
Copy link
Author

kejxu commented Aug 21, 2019

Cross platform approach in 45edeb9. Released and tagged as v0.61.8 for melodic.

Thanks for back-porting the change! Will test this one out. Also thanks for educating me about CMAKE_CXX_EXTENSIONS and CMAKE_CXX_STANDARD_REQUIRED, will check CMake documentation on these 2.

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.

2 participants