-
Notifications
You must be signed in to change notification settings - Fork 128
COMP: Explicitly disable BUILD_SHARED_LIBS #184
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
Conversation
|
|
||
| if( BUILD_SHARED_LIBS ) | ||
| message(FATAL_ERROR "Elastix no longer supports BUILD_SHARED_LIBS") | ||
| endif() |
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.
"elastix does not support BUILD_SHARED_LIBS"
of
"elastix does not support BUILD_SHARED_LIBS ON"
The "no longer" can go, as we never suported it
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.
except for the exact wording, this looks good to me. So go ahead with a merge
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.
@mstaring Are you sure elastix never supported BUILD_SHARED_LIBS? In elastix-4.9.0-manual.pdf it says:
If elastix is supplied as a shared library (DLL), you need to define the preprocessor symbol _ELASTIX_USE_SHARED_LIBRARY
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.
quite sure that it never worked. Can you also modify this sentence in the manual?
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.
@mstaring I just adjusted manual.tex and elxMacro.h accordingly. Please check again!
Attempts to build shared Elastix libraries have led to various problems, including a core dump recently reported by Luisa Sanchez Brea. Pull request #145 "Make BUILD_SHARED_LIBS do its job" by Harmen Stoppels (@haampie) was abandoned because of lack of resources at our side. This commit prevents users from accidentally switching on the CMake option `BUILD_SHARED_LIBS` for Elastix. Moreover, it drops support for the macro `_ELASTIX_USE_SHARED_LIBRARY`. The manual (TeX file) is adjusted accordingly.
0cc822e to
4f4d85e
Compare
|
@coertmetz Could you perhaps check if this does not interfere with quantib needs? |
|
We only use static linking. Anyways we use a fork, so there will not be any direct influence anyways. |
|
thanks Coert. @N-Dekker then go ahead with the merge |
Aims to fix the following CMake Warning from https://open.cdash.org/build/6758579/configure Build: InsightSoftwareConsortium/ITKElastix-macos-10.15--refs/pull/60/merge > CMake Warning (dev) at /Users/runner/work/ITKElastix/build/_deps/elx-src/CMakeLists.txt:37 (option): > Policy CMP0077 is not set: option() honors normal variables. Run "cmake > --help-policy CMP0077" for policy details. Use the cmake_policy command to > set the policy and suppress this warning. > > For compatibility with older versions of CMake, option is clearing the > normal variable 'BUILD_SHARED_LIBS'. Follow-up to commit bb902f7, "COMP: Explicitly disable BUILD_SHARED_LIBS", September 11, 2019. Related to: - pull request #184 "COMP: Explicitly disable BUILD_SHARED_LIBS" - pull request #145 "Make BUILD_SHARED_LIBS do its job" - issue #202 "Elastix 5.0 does not support shared libaray?"
Aims to fix the following CMake Warning from https://open.cdash.org/build/6758579/configure Build: InsightSoftwareConsortium/ITKElastix-macos-10.15--refs/pull/60/merge > CMake Warning (dev) at /Users/runner/work/ITKElastix/build/_deps/elx-src/CMakeLists.txt:37 (option): > Policy CMP0077 is not set: option() honors normal variables. Run "cmake > --help-policy CMP0077" for policy details. Use the cmake_policy command to > set the policy and suppress this warning. > > For compatibility with older versions of CMake, option is clearing the > normal variable 'BUILD_SHARED_LIBS'. Follow-up to commit bb902f7, "COMP: Explicitly disable BUILD_SHARED_LIBS", September 11, 2019. Related to: - pull request #184 "COMP: Explicitly disable BUILD_SHARED_LIBS" - pull request #145 "Make BUILD_SHARED_LIBS do its job" - issue #202 "Elastix 5.0 does not support shared libaray?"
Attempts to build shared Elastix libraries have led to various problems, including a core dump recently reported by Luisa Sanchez Brea.
Pull request #145 "Make BUILD_SHARED_LIBS do its job" by Harmen Stoppels (@haampie) was abandoned because of lack of resources at our side.
This commit prevents users from accidentally switching on the CMake option
BUILD_SHARED_LIBSfor Elastix.Moreover, it drops support for the macro
_ELASTIX_USE_SHARED_LIBRARY.The manual (TeX file) is adjusted accordingly.