Skip to content
Snippets Groups Projects

Fix itk packaging

Merged Guillaume Pasero requested to merge fix_itk_packaging into release-6.6

ITK libraries (such as ITKTransformFactory) were missing in the package, because OTB doesn't drag them.

The ITK targets are now parsed to build the package, so that find_package(ITK) works fine.

Merge request reports

Approval is optional
Ready to merge by members who can write to the target branch.

Merge details

  • 2 commits and 1 merge commit will be added to develop.
  • Source branch will not be deleted.

Activity

Filter activity
  • Approvals
  • Assignees & reviewers
  • Comments (from bots)
  • Comments (from users)
  • Commits & branches
  • Edits
  • Labels
  • Lock status
  • Mentions
  • Merge request status
  • Tracking
  • if ITKTransformFactory isn't linked with OTB. then who is asking them? cmake configure ? there are transformfactory and iofactory register classes created by itk_use_file. explicitly adding itk*.dll isn't very interesting for me. checking of dependencies ensure there aren't any missing ones.

  • Yes, CMake configure is looking for all the targets.

    About the inclusion of these modules, see !71 (merged)

  • still not comfortable with force including dlls from one library. It says somewhere packaging or build is broken. But I don't think you have much choice given the already merged branch. If these new itk modules are only needed for @ytanguy and not to build or use OTB. better not to add it in superbuild.

  • still not comfortable with force including dlls from one library

    Really? It was already the case: check Packaging/install_importlibs.cmake with the file GLOB on ${SUPERBUILD_INSTALL_DIR}/bin/itk*.dll

    As you said, we don't have much choice: find_package(ITK) will fail if you don't put all the targets in the package. And from a user point of view, it is perfectly acceptable to demand an OTB package containing a decent list of ITK modules, because they are at the basis of OTB.

  • yes. that line was because itk tends to include some of vnl and vcl libs which are not interested or required such as vcl test libs. I tried to push a patch to itk cmake and it goes never ending... hence that patch. remember the lowercase in itk.dll.

    agreed to include a list of required modules but there wasn't any use of it in otb. if so script would have picked it up. And these modules starts with ITK (uppercase). Hence the feeling it is used by one user for a remote module far far away..

  • I confirm it's only use in an application based on OTB. I don't know which is the best option : I agree it does not make sense if OTB does not use these modules itself. But on the other hand, if we only adapt SuperBuild, we will have different versions of OTB, some of them including these modules.

  • @ytanguy, is this app part of OTB or a remote module. If so we don't need to patch packaging script. just need to build this application and it includes those itk modules.

  • Okay, something needs to be decided here: either we patch Packaging script as proposed, or we remove ITKTransform packages. Until then, packages are broken and release is stalled.

  • I do not think that what matters in superbuild is that it is directly used by OTB or not. We already have gdal binaries and gdal python wrappings (not used by OTB at all), GSL (used by an official remote module) and other stuff as well. Superbuild is really more a small distribution than a build of OTB.

    @ytanguy has a remote module that requires those ITK modules. If we do not package them, then this remote module will never be compatible with superbuild and packaging. It is more convenient for developers, but not for users.

    If the patch provided by @gpasero is working and causes no further issue, I do not see why we should reject it.

  • GdalPython bindings had many requests and is used by many who use otb binary packages. But you are right about GSL part. That was added for a special for case of remote module not managed by OTB team. So its kind of not very useful.

    Anyway, IF there is too much demand to these itkmodules, then you can consider this resolved and move on to merge. Superbuild is building too many libs and expanding for one case isn't pleasant as you know it.

  • mentioned in commit 83c9b00d

  • added bug label

Please register or sign in to reply
Loading