[CMake] FindDCMTK Not setting up DCMTK_INCLUDE_DIRECTORIES Correctly?

[Michael Hertling]

On 11/29/2011 08:49 PM, Thomas Sondergaard wrote:
> On 2011-11-29 18:19, kent williams wrote:
>> I actually install DCMTK in a subdirectory of my top-level build, and
>> then my program that uses DCMTK is configured with DCMTK_DIR.
> 
> What is DCMTK_DIR pointing at? The installation dir or the source dir? 
> As you can see in the description of the string it is meant to point at 
> the root of the source directory.

A source tree beneath /usr/include? That's a very strange default, IMO.

>> The problem isnt MY source, the problem is that the DCMTK headers use
>> the dcmtk/x/y.h path form to include OTHER DCMTK headers.
> 
> That shouldn't be a problem as long as the headers are actually 
> available in a directory structure that matches that.

AFAICS, the problem isn't the subpath in the #include directives, but
the fact that FIND_PATH() in FindDCMTK.cmake doesn't return suitable
paths. E.g., if osconfig.h is included via "dcmtk/config/osconfig.h",
FIND_PATH() must return the path up to but *excluding* dcmtk/config;
however, FindDCMTK.cmake has FIND_PATH() search for "osconfig.h", so
it'll yield the header's full path *including* dcmtk/config. If this
is passed to INCLUDE_DIRECTORIES(), one will end up with -I.../dcmtk/
config, and beneath this path, dcmtk/config/osconfig.h can't be found,
i.e. the headers' inclusions among each other can't work in this way
unless the root of DCMTK's header tree is found by other means. The
DCMTK_dcmtk_INCLUDE_DIR variable nearly adds the crucial path to
DCMTK_INCLUDE_DIRS, but seems to keep one directory too much.

@Kent: Including headers by use of subpaths is not bad, IMO;
instead, it's rather clever, e.g. for the following reasons:

(1) It has an additional documentation effect because the #include
    directive indicates which module the included header stems from.

(2) It separates the namespaces of the package's modules, so multiple
    modules might have headers of the same name. In fact, this applies
    to DCMTK as the djdecode.h header appears twice; how would you use

    "dcmtk/dcmjpls/djdecode.h" and "dcmtk/dcmjpeg/djdecode.h"

    within the same compilation unit without specifying their
    respective subpath in the #include directive? You'd need to use
    further measures like configured pseudo headers to accomplish this.

(3) It reduces the number of necessary -I switches, i.e. arguments to
    INCLUDE_DIRECTORIES(), and that's particularly true for DCMTK. If
    all of the package's headers and all client code consistently use
    subpaths in the #include directives, you need just one single -I
    switch per installed header tree, and DCMTK has only one, so it
    would actually suffice to have <DCMTK_INSTALL_PREFIX>/include
    in DCMTK_INCLUDE_DIRS.

Due to the latter point, and as long as client code uses subpaths
for header inclusion, the FIND_PATH() command in FindDCMTK.cmake
for the population of DCMTK_INCLUDE_DIRS might be simplified to

FIND_PATH(DCMTK_DCMTK_INCLUDE_DIR dcmtk PATHS ${DCMTK_ROOT}/include)
MARK_AS_ADVANCED(DCMTK_DCMTK_INCLUDE_DIR)
IF(DCMTK_DCMTK_INCLUDE_DIR)
    SET(DCMTK_INCLUDE_DIRS ${DCMTK_DCMTK_INCLUDE_DIR})
ENDIF()

with DCMTK_ROOT pointing at a DCMTK installation tree. If one wants to
still have individual DCMTK_*_INCLUDE_DIR variables, e.g. as indicators
if the installation is complete, one could retain the loop and possibly
invoke LIST(REMOVE_DUPLICATES DCMTK_INCLUDE_DIRS). In either case, the
headers should be denoted with subpaths. In order to have this work if
FindDCMTK.cmake is searching for a source tree, the PATHS need to be
modified a bit, but is there a special reason why this find module
should work with source trees, too? AFAIK, that's quite unusual.

>> I am doing exactly what you're doing, and if you don't have this
>> problem I'm interested in how you got it to work.
> 
> What does your DCMTK_INCLUDE_DIRS variable contain?

If directed against a DCMTK installation tree via CMAKE_PREFIX_PATH,
FindDCMTK.cmake leaves this variable empty whereas DCMTK_LIBRARIES is
correctly populated with the package's libraries. IMO, this indicates
that the find module doesn't work properly. One has to point DCMTK_DIR
- against its documented meaning - at the installation tree to get the
headers' full paths, but as already mentioned, this is unsuitable for
the inter-header inclusions. As a possible workaround, one might add

FIND_PATH(DCMTK_DCMTK_INCLUDE_DIR dcmtk PATHS ${DCMTK_ROOT}/include)
MARK_AS_ADVANCED(DCMTK_DCMTK_INCLUDE_DIR)
IF(DCMTK_DCMTK_INCLUDE_DIR)
    LIST(APPEND DCMTK_INCLUDE_DIRS ${DCMTK_DCMTK_INCLUDE_DIR})
ENDIF()

to FindDCMTK.cmake, with or without a DCMTK_ROOT. In addition, one
would need to drop any other DCMTK_*_INCLUDE_DIR variable from the
FPHSA() call. In this manner, FIND_PACKAGE() would return the path
to the DCMTK installation's header tree, so that the inter-header
inclusions work after INCLUDE_DIRECTORIES(${DCMTK_INCLUDE_DIRS}).

Regards,

Michael