-
Notifications
You must be signed in to change notification settings - Fork 16
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 compiling S-Function with Matlab R2018b #36
Conversation
sources/Simulink/CMakeLists.txt
Outdated
@@ -25,6 +25,9 @@ if(CMAKE_CXX_COMPILER_ID MATCHES "GNU") | |||
target_compile_options(Simulink PRIVATE -Wno-format-overflow) | |||
endif() | |||
|
|||
# Fix https://github.com/robotology/blockfactory/issues/4 | |||
target_compile_options(Simulink PRIVATE "-fvisibility=default") | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This should be conditional on the specific compiler, right?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately it is not that easy, have a look at https://gitlab.kitware.com/cmake/cmake/blob/master/Modules/FindMatlab.cmake#L1021-1098.
I'm actually considering to include in this project a vendored version of FindMatlab.cmake
, what's your opinion?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm actually considering to include in this project a vendored version of
FindMatlab.cmake
, what's your opinion?
Ok for me.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately it is not that easy, have a look at https://gitlab.kitware.com/cmake/cmake/blob/master/Modules/FindMatlab.cmake#L1021-1098.
Ok, but do you agree that in the current form the PR is breaking MSVC builds?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes it does, but as usual CI cannot catch it since there's no Matlab / Simulink support.
It support the Simulink component that it adds automatically the compile definitions and include directories.
Updated PR with new |
This is a temporary fix of #4. A more stable fix should be found.
Closes #4.