Update to work with the API of DCMTK 3.6.3

[fedorov]

No description provided.

[fedorov]

@che85 do you have any idea how to deal with the issue below? I don't know why it started just now, can't make sense of this error.

[jcfr]

@fedorov Instead of using jsonlint, a simpler way to validate the json files could be this one:

python3.5 -c "import json; json.load(open('./doc/schemas/common-schema.json'))"

[fedorov]

@jcfr python in the dockross does not have json package. How should this be fixed?

[jcfr]

How should this be fixed?

Update this line adding -DPYTHON_EXECUTABLE:FILEPATH=/opt/python/cp36-cp36m/bin/python

Since the manylinux image bundles multiple versions of python, one has to be explicitly selected.

[fedorov]

Doesn't seem to work. Looks like cmake picks /usr/bin/python no matter what are the path variables.

Also note right now there is also a line adding /opt/python/cp27-cp27mu/bin/ to the path -
should it be removed?

dcmqi/docker/Makefile

Line 96 in 0d5124d

$(TMP)/dockcross bash -c "export PATH=/work/build/node-v6.9.5-linux-x64/bin:/work/build/$(dicom3tools_version)/appsrc/dcfile:/opt/python/cp27-cp27mu/bin/:$$PATH && cd /work/build/dcmqi-build && ctest -j4 -D ExperimentalTest -VV --no-compress-output"

(the behavior does not change if I remove it from the path)

[jcfr]

I see ... this was done to support running the script comparejson.py used in some test that can only run with python 2.7.

I suggested python 3.6 earlier without recalling that specific details.

Two things:

• find_package(PythonInterp 2.7 REQUIRED) should probably be added, and test updated to consistently use ${PYTHON_EXECUTABLE}

• Now looking into why the option is not propagated

[jcfr]

• Consider also updating the top-level CMakeLists.txt adding the following in the Dependencies section. See here

• The "Testing" should also be moved up so that BUILD_TESTING var is defined.

#-----------------------------------------------------------------------------
# Testing
#
include(CTest)
mark_as_superbuild(BUILD_TESTING)

set(TEMP_DIR ${CMAKE_BINARY_DIR}/Testing/Temporary)
mark_as_superbuild(TEMP_DIR:PATH)

#-----------------------------------------------------------------------------
# Dependencies
#

# Slicer
if(DCMQI_BUILD_SLICER_EXTENSION)
  find_package(Slicer REQUIRED)
  include(${Slicer_USE_FILE})
  mark_as_superbuild(Slicer_DIR)
endif()

# JsonCpp
set(_msg "Checking if using built-in JsonCpp")
message(STATUS ${_msg})
if(EXISTS "${JsonCpp_LIBRARY}" AND EXISTS "${JsonCpp_INCLUDE_DIR}")
  set(DCMQI_BUILTIN_JSONCPP 0)
  message(STATUS "${_msg} - no")
else()
  set(DCMQI_BUILTIN_JSONCPP 1)
  message(STATUS "${_msg} - yes")
endif()
mark_as_superbuild(JsonCpp_INCLUDE_DIR:PATH)
mark_as_superbuild(JsonCpp_LIBRARY:FILEPATH)
mark_as_superbuild(DCMQI_BUILTIN_JSONCPP:BOOL)

# Python 2.7
if(BUILD_TESTING)
  find_package(PythonInterp 2.7 REQUIRED)
  mark_as_superbuild(PYTHON_EXECUTABLE)
endif()

[fedorov]

@jcfr CIs are green - thanks for all the help with PR! 👍

[jcfr]

🎆

[fedorov]

@jcfr I just discovered that when configuring against Slicer, I am getting this CMake error, which I think is related. Do you know what is going on?

[jcfr]

Are you doing a clean build or setting Slicer_DIR on an existing build dir ?

This is a sanity check to warn developer early.

I didn't have a problem building the extension against Slicer. This PR makes it even easier to build dcmqi against a Slicer Qt5 build. See #336

[fedorov]

No, I started from empty directory.

Interesting - if I run cmake with all variables from command line, configure works. If I run ccmake, and set variables one by one - I get the error above. See screen capture here: https://www.screencast.com/t/JvJRswmwpDAy.

[jcfr]

I see. This is expected. We could address this for python executable by forcing the new value. That said, the cmake_cxx_standard value can not be forced to a value, it has to be set early one so that the initial system introspection is correct. On way around this would to have default value matching slicer Jc

…

On Feb 23, 2018 8:49 PM, "Andrey Fedorov" ***@***.***> wrote: No, I started from empty directory. Interesting - if I run cmake with all variables from command line, configure works. If I run ccmake, and set variables one by one - I get the error above. See screen capture here: https://www.screencast.com/t/ JvJRswmwpDAy. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#333 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AANXo-DgNHP8U08wD8hzy2TNeuoVCxu2ks5tX2qWgaJpZM4SJKPK> .