Shared library compilation v3.6.0

[benoit.bleuze]

Hi,
First congratulations on the 3.6 version, and specially on the porting of dcmtk to cmake for Unix.

We used to be forced to compile dcmtk 3.5 with configure whereas all our other projects were cmake based, it's a relief.

We also compile shared libraries of dcmtk. Indeed we load dcmtk libraries from several plugins in the same executable and your singleton architecture for some globals make the program crash if loaded from several places. The solution is shared libraries, which was fine with the configure.

Now that you shifted to cmake, changing just a few linking values doesn't work and should be handled easilly by cmake.

So I patched your cmake files to allow for that.
Here is the patch in question. In addition to the BUILD_SHARED_LIB flag from cmake, I had to add a few links in relevant places in the code. Could you please add this patch to the repository if you judge it satisfactory?
Thank you,
Benoit Bleuzé



diff --git a/CMakeLists.txt b/CMakeLists.txt
index 446d478..2b01c84 100644
--- a/CMakeLists.txt
+++ b/CMakeLists.txt
@@ -37,7 +37,7 @@ MARK_AS_ADVANCED(SNDFILE_DIR DCMTK_WITH_SNDFILE) # not yet needed in public DCMT
OPTION(DCMTK_WITH_PRIVATE_TAGS "Configure DCMTK with support for DICOM private tags coming with DCMTK" OFF)
OPTION(DCMTK_WITH_THREADS "Configure DCMTK with support for multi-threading" ON)
OPTION(DCMTK_WITH_DOXYGEN "Build API documentation with doxygen" ON)
-
+OPTION(BUILD_SHARED_LIBS TRUE)
#SET(CXX_TEST_PATH ${EXECUTABLE_OUTPUT_PATH})

#-----------------------------------------------------------------------------
diff --git a/dcmimage/libsrc/CMakeLists.txt b/dcmimage/libsrc/CMakeLists.txt
index 830f4ea..742e515 100644
--- a/dcmimage/libsrc/CMakeLists.txt
+++ b/dcmimage/libsrc/CMakeLists.txt
@@ -1,5 +1,7 @@
# create library from source files
ADD_LIBRARY(dcmimage diargimg dicmyimg dicoimg dicoopx dicopx dihsvimg dilogger dipalimg dipipng dipitiff diqtctab diqtfs diqthash diqthitl diqtpbox diquant diregist dirgbimg diybrimg diyf2img diyp2img)

+TARGET_LINK_LIBRARIES (dcmimage ${LIBPNG_LIBS} ${LIBTIFF_LIBS})
+
# declare installation files
INSTALL_TARGETS(${INSTALL_LIBDIR} dcmimage)
diff --git a/dcmimgle/libsrc/CMakeLists.txt b/dcmimgle/libsrc/CMakeLists.txt
index 5102a49..9682d3a 100644
--- a/dcmimgle/libsrc/CMakeLists.txt
+++ b/dcmimgle/libsrc/CMakeLists.txt
@@ -1,5 +1,5 @@
# create library from source files
ADD_LIBRARY(dcmimgle dcmimage dibaslut diciefn dicielut didislut didispfn didocu digsdfn digsdlut diimage diinpx diluptab dimo1img dimo2img dimoimg dimoimg3 dimoimg4 dimoimg5 dimomod dimoopx dimopx diovdat diovlay diovlimg diovpln diutils)
-
+TARGET_LINK_LIBRARIES (dcmimgle dcmdata)
# declare installation files
INSTALL_TARGETS(${INSTALL_LIBDIR} dcmimgle)
diff --git a/dcmpstat/libsrc/CMakeLists.txt b/dcmpstat/libsrc/CMakeLists.txt
index cc0567e..8e09692 100644
--- a/dcmpstat/libsrc/CMakeLists.txt
+++ b/dcmpstat/libsrc/CMakeLists.txt
@@ -1,5 +1,7 @@
# create library from source files
ADD_LIBRARY(dcmpstat dcmpstat dviface dvpsab dvpsabl dvpsal dvpsall dvpscf dvpscu dvpscul dvpsda dvpsdal dvpsfs dvpsga dvpsgal dvpsgl dvpsgll dvpsgr dvpsgrl dvpshlp dvpsib dvpsibl dvpsmsg dvpsov dvpsovl dvpspl dvpspl2 dvpspll dvpspr dvpsprt dvpsri dvpsril dvpsrs dvpsrsl dvpssp dvpsspl dvpssv dvpssvl dvpstat dvpstx dvpstxl dvpsvl dvpsvll dvpsvw dvpsvwl dvsighdl)

+TARGET_LINK_LIBRARIES(dcmpstat dcmsr)
+
# declare installation files
INSTALL_TARGETS(${INSTALL_LIBDIR} dcmpstat)

[Jörg Riesmeier]

Thank you for the report. This is not the first comment we get regarding shared library support in DCMTK 3.6.0 ... so it's on our top priority list

[benoit.bleuze]

Excellent, it's good to see you responsive like this.

Have you tried the patch, though? I can't test it on all platform, so I am not sure it is disruptive for windows users for instance.

[Jörg Riesmeier]

No, we haven't tried it yet because we are currently busy with finalizing all the other DCMTK modules that can be licensed separately (e.g. DCMPRINT, DCMJP2K, and so on).

[Jörg Riesmeier]

In the meantime, I've tested the "shared library" option with CMake: It seems to work - at least on a Linux system
However, I've used a slightly different approach by linking the "missing" libraries to the executables (and not to the libraries that depend on them).

The required changes will probably be part of the next development snapshot ...

[benoit.bleuze]

I tried that at first as well, this is suggested by solving the linking errors when building, but it doesn't make sense, the inclusion has to be made at the library level, so that any inclusion of this library should be sufficient, without having to add extra dependencies. It should be self contained, in my opinion.

Ben.

[Jörg Riesmeier]

Maybe, you are right. However, the approach described in my last posting is more consistent with the mechanism currently used throughout the DCMTK for both GNU autoconf and CMake generated Makefiles. The dependencies between the various libraries are shown in the following picture.

[ymartelli]

Is there any plan to support shared libraries but for windows? On top of the changes Benoit proposes, the code is missing the __declspec(dllexport/import) for the exported classes/methods...

I managed to generate them by:

- adding a DCMTK_BUILD_SHARED_LIBS option in the main CMakeList
- defining the following standard macro in CMake/osconfig.h.in:

Code: Select all

#cmakedefine DCMTK_BUILD_SHARED_LIBS @DCMTK_BUILD_SHARED_LIBS@

#if defined(DCMTK_BUILD_SHARED_LIBS) && (defined(_WIN32) || defined(WIN32))

# if defined(ofstd_EXPORTS)
#  define OFSTD_EXPORT __declspec(dllexport)
# else
#  define OFSTD_EXPORT __declspec(dllimport)
# endif

#else //linux or static: need nothing
# define OFSTD_EXPORT 
#endif

- and then exporting the classes/methods with the defined variable *_EXPORT

Best,
Yves

[J. Riesmeier]

Is there any plan to support shared libraries but for windows?

Yes, it is on our to-do list. Thank you for your suggestions.

[J. Riesmeier]

This issue is solved now, i.e. with the latest version from the public git repository you should be able to build Windows DLLs ...