DICOM @ OFFIS

Discussion Forum for OFFIS DICOM Tools - For registration, send email with desired user name to the OFFIS DICOM team
It is currently Thu, 2018-04-26, 22:09

All times are UTC + 1 hour




Post new topic Reply to topic  [ 5 posts ] 
Author Message
PostPosted: Thu, 2016-09-08, 19:33 
Offline

Joined: Thu, 2016-09-08, 15:55
Posts: 2
The DCMTKConfig.cmake places all of the module subdirectories directly into DCMTK_INCLUDE_DIRS. The standard CMake way to configure for this dependency would be along the lines of:

Code:
find_package(DCMTK REQUIRED CONFIG)
include_directories(${DCMTK_INCLUDE_DIRS})


This means the DCMTK headers which are normally separated by folder hierarchy
can now shadow system includes. As a specific example, the line:

Code:
#include <tchar.h>


Will find <dcmtk>/include/oflog/tchar.h instead of the Windows system header.

In the FAQ for the snapshot,
this standard config method is not used. Instead the directory is set manually into
DCMTK_DIR and ${DCMTK_DIR}/include is explicitly given.

Is there any reason why these subdirectories should be put directly into the include path?
My impression is that DCMTK includes should always take a form like "dcmtk/oflog/tchar.h"
and so DCMTK_INCLUDE_DIRS should only point to ${PACKAGE_PREFIX_DIR}/include


Top
 Profile  
 
PostPosted: Fri, 2016-09-09, 08:16 
Offline
OFFIS DICOM Team
OFFIS DICOM Team

Joined: Mon, 2014-03-03, 09:51
Posts: 231
Location: Oldenburg, Germany
Hi jakecobb,

I will have to investigate this further, but on the first glance you seem to be correct. I remember (roughly) that implementing the DCMTKConfig.cmake was a bit tricky, since you need a different config depending on wether it should reside in the build vs inside the final installation directory. So I consider a bug like you described entirely possible.
I have added it into our bug tracker so it is not forgotten, since I don't know when somebody will find the time to take a look at it (should be fixed before the next snapshot or release I think though).

Best regards
Jan


Top
 Profile  
 
PostPosted: Thu, 2016-09-22, 10:08 
Offline
OFFIS DICOM Team
OFFIS DICOM Team

Joined: Mon, 2014-03-03, 09:51
Posts: 231
Location: Oldenburg, Germany
Hi jakecobb,

this should be fixed in commit cf843f5, can you confirm that?

Best regards
Jan


Top
 Profile  
 
PostPosted: Wed, 2016-09-28, 21:15 
Offline

Joined: Thu, 2016-09-08, 15:55
Posts: 2
Sorry for the slow reply. I applied the patch and it does fix the problem, thanks!

-Jake


Top
 Profile  
 
PostPosted: Thu, 2016-09-29, 07:54 
Offline
OFFIS DICOM Team
OFFIS DICOM Team

Joined: Mon, 2014-03-03, 09:51
Posts: 231
Location: Oldenburg, Germany
Great, no problem!


Top
 Profile  
 
Display posts from previous:  Sort by  
Post new topic Reply to topic  [ 5 posts ] 

All times are UTC + 1 hour


Who is online

Users browsing this forum: No registered users and 1 guest


You cannot post new topics in this forum
You cannot reply to topics in this forum
You cannot edit your posts in this forum
You cannot delete your posts in this forum

Search for:
Jump to:  
cron
Powered by phpBB® Forum Software © phpBB Group