Skip to content

Conversation

@haampie
Copy link
Contributor

@haampie haampie commented Apr 29, 2019

Not 100% confident if this is correct, but it's an attempt to fix #81.

@mstaring
Copy link
Member

this should be carefully reviewed, as it may break code downstream, for example for the companies using it.

@haampie
Copy link
Contributor Author

haampie commented Apr 30, 2019

It's also failing on Windows:

definition of dllimport function not allowed

I'm not very familiar with Windows / dll's, but it seems tricky.

@N-Dekker
Copy link
Member

Thank you Harmen @haampie for looking into this issue. Unfortunately we don't really have the resources (both man power and computer resources) to support both shared and static, for all platforms. As I discussed with Marius @mstaring earlier today. So unless there is a use case that appears very important to us, we won't actively support building shared elastix libs.

Technical considerations:

  1. A static library can potentially yield better performance than a shared library, because static linking allows the linker to do extra optimizations (function inlining, etc).
  2. elastix was never designed for shared libraries, and as it consists of a collection of many small subprojects, it may produce a lot of small shared libraries (if we would support building shared libs), which may be cumbersome.

Hope it's OK to you!

@mstaring
Copy link
Member

mstaring commented May 2, 2019

There are many intricacies involved for supporting different types of libs, upstream and downstream.

I'm going to close this PR as there are currently no good test cases, and no resources to investigate this carefully.

@mstaring mstaring closed this May 2, 2019
N-Dekker added a commit that referenced this pull request Sep 11, 2019
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 `BUILD_SHARED_LIBS` for Elastix.
N-Dekker added a commit that referenced this pull request Sep 16, 2019
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.
N-Dekker added a commit that referenced this pull request Sep 16, 2019
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.
N-Dekker added a commit that referenced this pull request Sep 12, 2020
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?"
N-Dekker added a commit that referenced this pull request Sep 12, 2020
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?"
@mboisson mboisson mentioned this pull request Aug 6, 2021
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.

transformix can't be built as shared library

3 participants