8000 BUG: Pass cmake-options to Windows script for Python module by SimonRit · Pull Request #58 · InsightSoftwareConsortium/ITKRemoteModuleBuildTestPackageAction · GitHub
[go: up one dir, main page]
Skip to content

BUG: Pass cmake-options to Windows script for Python module#58

Merged
tbirdso merged 1 commit intoInsightSoftwareConsortium:mainfrom
SimonRit:cmake_options_win
Feb 24, 2023
Merged

BUG: Pass cmake-options to Windows script for Python module#58
tbirdso merged 1 commit intoInsightSoftwareConsortium:mainfrom
SimonRit:cmake_options_win

Conversation

@SimonRit
Copy link
Collaborator
@SimonRit SimonRit commented Feb 20, 2023

CMake options were not passed to the Windows script for Python packaging. MacOS and Linux scripts work adequately.

The '-DRTK_BUILD_APPLICATIONS:BOOL=OFF` option should turn off the compilation of applications using gengetopt and a simple search for ggo in the raw logs shows that it is the case for
https://github.com/RTKConsortium/RTK/actions/runs/4222316866/jobs/7330680579
and not before this commit
https://github.com/RTKConsortium/RTK/actions/runs/4201859054/jobs/7295273280.

@dzenanz dzenanz requested a review from tbirdso February 20, 2023 13:41
@SimonRit
Copy link
Collaborator Author
SimonRit commented Feb 20, 2023

I think there is a problem with InsightSoftwareConsortium/ITKPythonPackage@2dca3c8. In this build, the two cmake options -DRTK_BUILD_APPLICATIONS:BOOL=OFF -DRTK_CUDA_VERSION=11.6 are processed as one string. I'll try to find a fix, do not merge yet...

@SimonRit SimonRit changed the title BUG: Pass cmake-options to Windows script for Python module WIP: Pass cmake-options to Windows script for Python module Feb 20, 2023
@SimonRit SimonRit changed the title WIP: Pass cmake-options to Windows script for Python module BUG: Pass cmake-options to Windows script for Python module Feb 21, 2023
@SimonRit
Copy link
Collaborator Author
SimonRit commented Feb 21, 2023

I think there is a problem with InsightSoftwareConsortium/ITKPythonPackage@2dca3c8. In this build, the two cmake options -DRTK_BUILD_APPLICATIONS:BOOL=OFF -DRTK_CUDA_VERSION=11.6 are processed as one string. I'll try to find a fix, do not merge yet...

This is fixed with double quotes (in a single quote string): https://github.com/RTKConsortium/RTK/compare/f6473ee2ee05e9404597863e17259edd39e85132..e8f0874585dcea01f89a520b765205b7466f599c.

@SimonRit SimonRit mentioned this pull request Feb 21, 2023
Open
@tbirdso
Copy link
Collaborator
tbirdso commented Feb 21, 2023

@SimonRit Thanks for working on this!

Does adding extra quotes around cmake-options parameters impact processing for Linux and macOS builds? Do you have a link to where the changes proposed in https://github.com/RTKConsortium/RTK/compare/f6473ee2ee05e9404597863e17259edd39e85132..e8f0874585dcea01f89a520b765205b7466f599c succeed across platforms?

@SimonRit
Copy link
Collaborator Author
SimonRit commented Feb 22, 2023

@SimonRit Thanks for working on this!

Does adding extra quotes around cmake-options parameters impact processing for Linux and macOS builds? Do you have a link to where the changes proposed in https://github.com/RTKConsortium/RTK/compare/f6473ee2ee05e9404597863e17259edd39e85132..e8f0874585dcea01f89a520b765205b7466f599c succeed across platforms?

No... I'm not testing MacOS but it fails on Linux:
https://github.com/RTKConsortium/RTK/actions/runs/4229904801/jobs/7356304475#step:4:496
I'll work on a fix today...

@SimonRit SimonRit marked this pull request as draft February 22, 2023 09:31
@SimonRit SimonRit force-pushed the cmake_options_win branch 3 times, most recently from 16378a5 to 9ab9aa5 Compare February 22, 2023 14:48
@SimonRit
Copy link
Collaborator Author
SimonRit commented Feb 22, 2023

The PR will be ready for review after a small modification (ITKPythonPackage hash and repo) when InsightSoftwareConsortium/ITKPythonPackage#252 is merged.

@SimonRit SimonRit marked this pull request as ready for review February 23, 2023 09:02
@SimonRit
Copy link
Collaborator Author
SimonRit commented Feb 24, 2023

This is ready to be merged in my opinion...

tbirdso
tbirdso approved these changes Feb 24, 2023
View reviewed changes
Copy link
Collaborator
@tbirdso tbirdso left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great!

EDIT: xref passing jobs in InsightSoftwareConsortium/ITKPythonPackage#252

@tbirdso tbirdso merged commit f2191a0 into InsightSoftwareConsortium:main Feb 24, 2023
@SimonRit SimonRit deleted the cmake_options_win branch February 24, 2023 20:14
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@dzenanz dzenanz dzenanz approved these changes

@tbirdso tbirdso tbirdso approved these changes

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@SimonRit @tbirdso @dzenanz
0