elastix
elastix copied to clipboard
WIP: Use vcpkg to get ITK
See also https://discourse.itk.org/t/how-to-avoid-rebuilding-itk-by-azure-ci-at-the-github-of-another-project-elastix/2002
Compilation errors at https://dev.azure.com/kaspermarstal/Elastix/_build/results?buildId=467 for example:
itkAccumulateDerivativesParallellizationTest.cxx(62): error C2039: 'ThreadInfoStruct': is not a member of 'itk::PlatformMultiThreader'
because of ITK_LEGACY_REMOVE:
https://github.com/microsoft/vcpkg/blob/47d206e149e88201333b5ca8fe55c30e2b1a8177/ports/itk/portfile.cmake#L38
The missing ITK legacy support should be added by https://github.com/microsoft/vcpkg/pull/7241
Interesting compile errors at https://dev.azure.com/kaspermarstal/Elastix/_build/results?buildId=505:
elxAdaptiveStochasticLBFGS.hxx(1573): error C2440: 'initializing': cannot convert from 'const itk::AdvancedTransform<double,2,2> *' to 'itk::SmartPointer<itk::AdvancedTransform<double,2,2>>
elxAdaptiveStochasticLBFGS.hxx(970): error C2662: 'void itk::Transform<TParametersValueType,2,2>::SetParameters(const itk::OptimizerParameters<TParametersValueType> &)': cannot convert 'this' pointer from 'const itk::AdvancedTransform<double,2,2>' to 'itk::Transform<TParametersValueType,2,2> &'
elxAdaptiveStochasticLBFGS.hxx(988): error C2664: 'void itk::ComputeJacobianTerms<itk::Image<float,2>,itk::AdvancedTransform<double,2,2>>::SetTransform(itk::AdvancedTransform<double,2,2> *)': cannot convert argument 1 from 'const itk::AdvancedTransform<double,2,2> *' to 'itk::AdvancedTransform<double,2,2> *'
And also link errors:
LINK : fatal error LNK1181: cannot open input file '\lib\double-conversion.lib'
@dzenanz Dženan I'm still trying to build elastix with ITK from vcpkg, but we're getting link errors, at https://dev.azure.com/kaspermarstal/Elastix/_build/results?buildId=516:
LINK : fatal error LNK1181: cannot open input file '\lib\double-conversion.lib'
Could this be because ITK_USE_SYSTEM_DOUBLECONVERSION
is explicitly switched ON
, in the vcpkg ITK portfile.cmake? https://github.com/microsoft/vcpkg/blob/743e168ef5c7705e44d1d5cab5b9cca22328345e/ports/itk/portfile.cmake#L41
Do you have a suggestion how to avoid these link errors?
There was some work regarding ITK_USE_SYSTEM_DOUBLECONVERSION
between 5.0.0 and 5.0.1. Can you try with vcpkg 7352fab or newer?
There was some work regarding
ITK_USE_SYSTEM_DOUBLECONVERSION
between 5.0.0 and 5.0.1. Can you try with vcpkg 7352fab or newer?
Thanks @dzenanz Unfortunately the virtual machine I'm using at azure, 'vs2017-win2016', still has a vcpkg git repository of July 16: https://github.com/microsoft/vcpkg/commit/ac00ef2070be876479bcf667ac2f41c0a888bdb8 I guess I just have to wait.
Did you also get those double-conversion.lib link errors from ITK 5.0.0 at vcpkg?
Those errors do look familiar. You can look at https://github.com/InsightSoftwareConsortium/ITK/issues/579 and stuff linked from there.
You can look at InsightSoftwareConsortium/ITK#579
Thanks @dzenanz ! So what would then happen when ITK_USE_SYSTEM_DOUBLECONVERSION = ON
and there is no doubleconversion library on the system? I have no idea if doubleconversion is available "out of the box" on the azure vs2017-win2016 virtual machine, what do you think?
That should use double-conversion from vcpkg
.
That should use double-conversion from
vcpkg
.
In the azure pipelines yml file of elastix (of this PR), I just did:
vcpkg.exe install itk
See https://github.com/SuperElastix/elastix/blob/ceafb802df2478ca8dd0b1ba5d6de87da73e27d0/Testing/CI/Azure/ci.yml#L13
Is that sufficient, or should it also have vcpkg.exe install double-conversion
?
double-conversion
is specified as a dependency in the CONTROL file, so vcpkg.exe install itk
should be enough.
@dzenanz I finally got elastix building at azure vmImage 'vs2017-win2016', with ITK 5.0.1 from vcpkg (latest ITK portfile.cmake). But I have been cheating! I added link_directories
to the root CMakeLists of elastix, so that the projects could link to "./lib/double-conversion.lib" and "openjp2.lib": https://github.com/SuperElastix/elastix/pull/161/commits/58a2900e66d0551ce7626b5ef4cd6508819941d0
link_directories(
C:/vcpkg/installed/x64-windows # For lib/double-conversion.lib
C:/vcpkg/installed/x64-windows/lib # For openjp2.lib
)
I did so to avoid link errors. Do you have a suggestion how to avoid such an ugly workaround?
Perhaps execute vcpkg integrate install
?
Perhaps execute
vcpkg integrate install
?
Thanks @dzenanz I gave it a try, https://github.com/SuperElastix/elastix/pull/161/commits/97a013f72aa27c9d0b2382e96a68d9a4d44b886a and it looks like your suggestion almost fixes the problem! It adds "C:\vcpkg\installed\x64-windows\lib" to the linker. Which does contain "double-conversion.lib", the file mentioned in the link error:
fatal error LNK1181: cannot open input file '\lib\double-conversion.lib'.
However, the linker still cannot find the lib file, as it is looking for "\lib\double-conversion.lib". If it was just looking for "double-conversion.lib", it would have found it in "C:\vcpkg\installed\x64-windows\lib". Now it still can't find the file: https://dev.azure.com/kaspermarstal/Elastix/_build/results?buildId=505
Do you have one more suggestion, please?
That might be a bug in vcpkg
, or something related to https://github.com/InsightSoftwareConsortium/ITK/issues/579 or https://github.com/google/double-conversion/issues/100.
@dzenanz Update: yesterday I got elastix fully building with ITK 5.0.1 from vcpkg. It appeared essential to do vcpkg integrate install
, and also to add msbuildArguments: /p:VcpkgEnabled=true
, in order to fix most link errors. Unfortunately, it still appeared necessary to add link_directories(C:/vcpkg/installed/x64-windows)
, as a "hack", to the elastix CMakeLists, in order to link to lib/double-conversion.lib
For the time being, we could leave that hack in there. But I still find the build times too long: more than two hours for an entire elastix build, of which almost an hour is spent building and installing ITK. I hope the new Azure Pipelines CacheBeta
task can be of help, but I'm still looking for an example: https://github.com/MicrosoftDocs/vsts-docs/issues/4988