COMP: Update find python method. #60

chir-set · 2025-08-20T18:36:26Z

Using 'Python3 COMPONENTS Development' instead of 'PythonLibs'.

jamesobutler · 2025-08-20T19:04:23Z

CMake/vtkWrapPython.cmake

    set(_QUIET_LIBRARY "REQUIRED")
  endif()
-  find_package(PythonLibs ${_QUIET_LIBRARY})
+  find_package(Python3 COMPONENTS Development ${_QUIET_LIBRARY})


A heads up that the same type of change was proposed in the main Slicer project, but resulted in issues of it picking up the System python instead of Slicer's python. See Slicer/Slicer#8578 (comment)

but resulted in issues

Ok, this cannot be a generalised solution then - it stems from an attempt to build VMTK. I'll use it in my local branch.

Thank you for the information.

lassoan · 2025-08-20T19:53:32Z

Thank you! I don't see why CommitCheck fails, but could you add COMP: prefix to the commit comment headline and see if it akes the check pass? If you are at it, you could also describe in the commit message that the replaced code is deprecated. And maybe set the CMake minimum version to 3.12 (because that's when Python COMPONENT was introduced).

jcfr · 2025-08-20T19:55:15Z

I will update the CI to remove the use of the obsolete "CommitCheck"

chir-set · 2025-08-20T20:29:33Z

... could you add COMP: ...

The changes are applied. SlicerVMTK builds fine with this patch on Linux (Arch, system python 3.13.7).

Using find_package with 'Python3 COMPONENTS Development' instead of deprecated 'PythonLibs'.

jcfr · 2025-08-20T20:51:21Z

Closing. This is a duplicate of:

COMP: Update to newer find_package(Python3) support #59

jcfr · 2025-08-20T20:52:21Z

The changes are applied. SlicerVMTK builds fine with this patch on Linux (Arch, system python 3.13.7).

Thanks for contributing the change and for testing 🙏

When you have a chance, could you also check the patch proposed in #59 ?

lassoan · 2025-08-20T21:00:56Z

I've reviewed and merged #59. @chir-set please test your linux VMTK builds with it. Thank you!

jcfr · 2025-08-20T21:14:37Z

When I will finalize the integration in Slicer, I will likely revisit to either:

gracefully support both PythonLibs and Python3 based on CMP0148
or report an warning message if Python was discovered using PythonLibs

chir-set · 2025-08-20T21:30:38Z

please test your linux VMTK builds with it.

When you have a chance, could you also check the patch proposed in #59 ?

Sure, I'll do that tomorrow, it's getting late here. Thanks.

lassoan · 2025-08-20T21:45:55Z

No problem, thanks for all your help. I've updated the git hashes in VMTK and SlicerExtension-VMTK, so if everything goes well then we'll have VMTK extension builds tomorrow morning.

chir-set · 2025-08-21T07:52:57Z

No problem, thanks for all your help. I've updated the git hashes in VMTK and SlicerExtension-VMTK, so if everything goes well then we'll have VMTK extension builds tomorrow morning.

With Slicer 399ee32a, SlicerVMTK 8df32b8e builds nicely with the Python3 discovery scheme.

(But Slicer ec6d323c itself fails to build for other reasons.)

Thank you.

jamesobutler · 2025-08-21T11:30:31Z

Note that Slicer hasn’t been updated with the vtkaddon changes integrated yesterday. The vtkaddon hash at https://github.com/Slicer/Slicer/blob/ec6d323c4489082bc83d4cd918939d6f30148b75/SuperBuild.cmake#L203 has not been updated.

chir-set · 2025-08-21T12:45:46Z

The vtkaddon hash ... has not been updated.

Even with the latest vtkAddon hash (cf11265) the same build failure occurs. This seems unrelated to the recent vtkAddon changes, i.e, no success with both hashes. But I can't be that sure.

[ 14%] Building CXX object Libs/vtkITK/CMakeFiles/vtkITK.dir/vtkITKImageSequenceReader.cxx.o
In file included from /home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:20:
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.h:37:3: warning: 'SetFileName' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
   37 |   vtkSetStringMacro(FileName);
      |   ^
/home/arc/src/Slicer-SuperBuild9/VTK/Common/Core/vtkSetGet.h:197:16: note: expanded from macro 'vtkSetStringMacro'
  197 |   virtual void Set##name(const char* _arg) vtkSetStringBodyMacro(name, _arg)
      |                ^
<scratch space>:60:1: note: expanded from here
   60 | SetFileName
      | ^
/home/arc/src/Slicer-SuperBuild9/VTK/IO/Image/vtkImageReader2.h:53:16: note: overridden virtual function is here
   53 |   virtual void SetFileName(VTK_FILEPATH const char*);
      |                ^
In file included from /home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:20:
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.h:39:3: warning: 'GetFileName' overrides a member function but is not marked 'override' [-Winconsistent-missing-override]
   39 |   vtkGetStringMacro(FileName);
      |   ^
/home/arc/src/Slicer-SuperBuild9/VTK/Common/Core/vtkSetGet.h:250:47: note: expanded from macro 'vtkGetStringMacro'
  250 | #define vtkGetStringMacro(name) virtual char* Get##name() vtkGetStringBodyMacro(name)
      |                                               ^
<scratch space>:62:1: note: expanded from here
   62 | GetFileName
      | ^
/home/arc/src/Slicer-SuperBuild9/VTK/IO/Image/vtkImageReader2.h:54:3: note: overridden virtual function is here
   54 |   vtkGetFilePathMacro(FileName);
      |   ^
/home/arc/src/Slicer-SuperBuild9/VTK/Common/Core/vtkSetGet.h:274:47: note: expanded from macro 'vtkGetFilePathMacro'
  274 |   virtual VTK_FILEPATH VTK_FUTURE_CONST char* Get##name() VTK_FUTURE_CONST vtkGetStringBodyMacro(  \
      |                                               ^
<scratch space>:258:1: note: expanded from here
  258 | GetFileName
      | ^
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:148:3: error: missing 'typename' prior to dependent type name 'ReaderType::Pointer'
  148 |   ReaderType::Pointer reader = ReaderType::New();
      |   ^~~~~~~~~~~~~~~~~~~
      |   typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:157:3: error: missing 'typename' prior to dependent type name 'ImageType::ConstPointer'
  157 |   ImageType::ConstPointer image = reader->GetOutput();
      |   ^~~~~~~~~~~~~~~~~~~~~~~
      |   typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:187:3: error: missing 'typename' prior to dependent type name 'ExtractImageFilterType::Pointer'
  187 |   ExtractImageFilterType::Pointer extractImageFilter = ExtractImageFilterType::New();
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |   typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:191:3: error: missing 'typename' prior to dependent type name 'ImageType::RegionType'
  191 |   ImageType::RegionType fullRegion = image->GetLargestPossibleRegion();
      |   ^~~~~~~~~~~~~~~~~~~~~
      |   typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:192:3: error: missing 'typename' prior to dependent type name 'ImageType::SizeType'
  192 |   ImageType::SizeType extractionSize = fullRegion.GetSize();
      |   ^~~~~~~~~~~~~~~~~~~
      |   typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:197:3: error: missing 'typename' prior to dependent type name 'ImageType::RegionType'
  197 |   ImageType::RegionType extractionRegion;
      |   ^~~~~~~~~~~~~~~~~~~~~
      |   typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:198:3: error: missing 'typename' prior to dependent type name 'ImageType::IndexType'
  198 |   ImageType::IndexType extractionIndex = extractionRegion.GetIndex();
      |   ^~~~~~~~~~~~~~~~~~~~
      |   typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:203:3: error: missing 'typename' prior to dependent type name 'VTKExporterFilterType::Pointer'
  203 |   VTKExporterFilterType::Pointer vtkExportFilter = VTKExporterFilterType::New();
      |   ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
      |   typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:213:5: error: missing 'typename' prior to dependent type name 'FrameImageType::Pointer'
  213 |     FrameImageType::Pointer frameImage = extractImageFilter->GetOutput();
      |     ^~~~~~~~~~~~~~~~~~~~~~~
      |     typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:214:5: error: missing 'typename' prior to dependent type name 'FrameImageType::RegionType'
  214 |     FrameImageType::RegionType frameRegion = frameImage->GetLargestPossibleRegion();
      |     ^~~~~~~~~~~~~~~~~~~~~~~~~~
      |     typename 
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:376:26: warning: 13 enumeration values not handled in switch: 'UNKNOWNPIXELTYPE', 'SCALAR', 'OFFSET'... [-Wswitch]
  376 |         switch (imageIO->GetPixelType())
      |                 ~~~~~~~~~^~~~~~~~~~~~~~
/home/arc/src/Slicer/Libs/vtkITK/vtkITKImageSequenceReader.cxx:435:26: warning: 11 enumeration values not handled in switch: 'UNKNOWNPIXELTYPE', 'OFFSET', 'POINT'... [-Wswitch]
  435 |         switch (imageIO->GetPixelType())
      |                 ~~~~~~~~~^~~~~~~~~~~~~~
4 warnings and 10 errors generated.
make[2]: *** [Libs/vtkITK/CMakeFiles/vtkITK.dir/build.make:348: Libs/vtkITK/CMakeFiles/vtkITK.dir/vtkITKImageSequenceReader.cxx.o] Error 1
make[1]: *** [CMakeFiles/Makefile2:6609: Libs/vtkITK/CMakeFiles/vtkITK.dir/all] Error 2
make: *** [Makefile:156: all] Error 2

lassoan · 2025-08-21T13:39:42Z

These are Slicer core build errors that were introduced by my unrelated commit for adding 5D image support. The build was fine on Windows but there were error on linux.

chir-set · 2025-08-21T14:03:19Z

introduced by my unrelated commit for adding 5D image support

I was in such a mood of 'What mess have I created?' that I did a quick bisection before reading you quoted message:

$ LC_ALL=C.UTF-8 LANG=C.UTF-8 git bisect log
git bisect start
# status : en attente d'un commit bon et d'un commit mauvais
# bad: [ec6d323c4489082bc83d4cd918939d6f30148b75] BUG: Fix traceback using line profile on 2D image
git bisect bad ec6d323c4489082bc83d4cd918939d6f30148b75
# status : en attente de bon(s) commit(s), un mauvais commit connu
# good: [399ee32a7d6152db2e3a8167e2beac01557469bc] DOC: Remove warning section for libtirpc GSS/Kerberos symbol lookup error
git bisect good 399ee32a7d6152db2e3a8167e2beac01557469bc
# bad: [11df881b46c907c92947ae89d1ffb3ef9cd95142] ENH: Allow storing 5D images and displacement fields in NRRD files
git bisect bad 11df881b46c907c92947ae89d1ffb3ef9cd95142
# good: [27597d0ea2ff4e249e3dc21404ee4ca579ec7347] BUG: Restore https download on macOS fixing packaging of SSL python modules (#8654)
git bisect good 27597d0ea2ff4e249e3dc21404ee4ca579ec7347
# good: [f62cfc4e524f1fe005f464000c283932e1c110bb] DOC: Add dedicated GOVERNANCE document and reference it from the about page
git bisect good f62cfc4e524f1fe005f464000c283932e1c110bb
# first bad commit: [11df881b46c907c92947ae89d1ffb3ef9cd95142] ENH: Allow storing 5D images and displacement fields in NRRD files

😄

lassoan · 2025-08-21T15:42:22Z

Note that Slicer hasn’t been updated with the vtkaddon changes integrated yesterday.

The vtkAddon changes were needed in VMTK and I updated the hashes there.

What I don't understand is how it is possible that find_package(PythonLibs) does not cause build failure in Slicer core but it does in extensions. @jcfr is it possible that in Slicer core some CMake variables or files are created so that find_package(PythonLibs) is not actually executed (because it has already found the package) but this information is not passed to extensions?

chir-set changed the title ~~Update find python method.~~ ENH: Update find python method. Aug 20, 2025

chir-set mentioned this pull request Aug 20, 2025

Update VMTK tag for vtkAddon wrapper files. vmtk/SlicerExtension-VMTK#163

Merged

jamesobutler reviewed Aug 20, 2025

View reviewed changes

chir-set changed the title ~~ENH: Update find python method.~~ COMP: Update find python method. Aug 20, 2025

chir-set force-pushed the UpdateFindPython branch from 178965e to 4369db1 Compare August 20, 2025 20:24

jcfr force-pushed the UpdateFindPython branch from 4369db1 to 768cbcd Compare August 20, 2025 20:48

COMP: Update find python method.

1b540d5

Using find_package with 'Python3 COMPONENTS Development' instead of deprecated 'PythonLibs'.

jcfr force-pushed the UpdateFindPython branch from 768cbcd to 1b540d5 Compare August 20, 2025 20:50

jcfr closed this Aug 20, 2025

COMP: Update find python method. #60

COMP: Update find python method. #60

Uh oh!

Conversation

chir-set commented Aug 20, 2025

Uh oh!

jamesobutler Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

chir-set Aug 20, 2025

Choose a reason for hiding this comment

Uh oh!

lassoan commented Aug 20, 2025

Uh oh!

jcfr commented Aug 20, 2025

Uh oh!

chir-set commented Aug 20, 2025

Uh oh!

jcfr commented Aug 20, 2025

Uh oh!

jcfr commented Aug 20, 2025

Uh oh!

lassoan commented Aug 20, 2025

Uh oh!

jcfr commented Aug 20, 2025

Uh oh!

chir-set commented Aug 20, 2025

Uh oh!

lassoan commented Aug 20, 2025

Uh oh!

chir-set commented Aug 21, 2025

Uh oh!

jamesobutler commented Aug 21, 2025

Uh oh!

chir-set commented Aug 21, 2025 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

lassoan commented Aug 21, 2025

Uh oh!

chir-set commented Aug 21, 2025

Uh oh!

lassoan commented Aug 21, 2025

Uh oh!

Reviewers

Assignees

Labels

Milestone

Development

Uh oh!

4 participants

chir-set commented Aug 21, 2025 •

edited

Loading