Add installation support for windows#82
Conversation
Add support for building and testing on Windows OS in a consistent manner to Linux and macOS. On Windows, this uses RTools to provide the necessary build tools. Testing is performed in CircleCI as it offers a more powerful build machine under the free plan. Co-authored-by: Artur-man <artur-man@hotmail.com>
.github/workflows/main.yml
Outdated
| env: | ||
| ITK_GLOBAL_DEFAULT_NUMBER_OF_THREADS: 2 | ||
| run: | | ||
| $env:MAKEJ="4" |
There was a problem hiding this comment.
Not sure about this setting.
According to the current documentation the GH hosted windows runner has 4 CPUs, so this may be maxing it out? When building I usually leave at least one CPU for other system processes. Not sure if this makes a difference.
@blowekamp, @Artur-man - any thoughts about this setting or the approach of using at most N-1 processes?
There was a problem hiding this comment.
I am open to this, considering maxing results in a 1.5 hour run, I think 3 should be still under 6 hours. However, I did not have any problems with 4 cores myself before.
There was a problem hiding this comment.
There can be linking steps that system choke on due to memory. SimpleITK 3 reduces the size of one excessively large object file with fixed some build issues or delays that were occurring in 2.x.
Reducing the number of core can speed things up if there is an issue with running out of memory. It just requires experimentation and tuning with what is best.
There was a problem hiding this comment.
Lets try then, I have another branch now where I can test before you guys approve a workflow agian.
There was a problem hiding this comment.
I have added you to the team for this repo. That should enable your PRs to build without approval.
zivy
left a comment
zivy
left a comment
There was a problem hiding this comment.
The windows runner does not seem to be identified correctly (if: startsWith(matrix.config.os, 'windows')), so the actual configuration and building were not run. Successful and fast run on the dashboard, but this isn't a real success. The CircleCI was successful and fast too, so I checked that too and it was indeed run and built.
@Artur-man was this executing for you under your repository?
|
@zivy thanks for catching this, looking at it now. |
|
Is this dependent on a fixed FindR.cmake file? When is that change needed? |
|
@blowekamp yes, will send over that PR too soon. |
|
@blowekamp I think we also need to update the ITK_TAG to the version where problem with GDCM was fixed. I have not touched it here since we only introduce the windows builders, we can leave the PR as it is until we release the fixes to upstream ? or ... ? |
|
It looks like I only back ported the change to ITK release-5.4 branch but never update SImpleITK's release (2.x) branch.
We could just work on the the 3.0.0a03 tag instead of back porting things. Your call based on your needs. |
|
ok, win builds in R=4.4.1 with new SimpleITK release and default ITK_TAG ... but gives a strange error in 4.3 (too big file size?) Will push this here too. Should I use v3.0.0a3 tag now ... one problem is that, this fails in Rcheck since R version cannot have letters ... so it has to be 3.0.0 or smthn |
Use the hash of SimpleITK release: 1899b2 |
|
@Artur-man We have the fixed in SimpleITK upstream with the needed fixes for compilation and cmake configuration. What are the next steps to move this forward? Any additional help needed? |
|
Thanks for checking in @blowekamp. I was preparing for a hackathon so ignored this PR for a while. I think I can start testing again after runs in another branch Currently, my main issue is with the version of the SimpleITK. Currently we parse that from the version field of the DESCRIPTION file in the R package bundle. But in order for build to be successful it should be a proper version number (e.g. 2.5.3 or 3.0.0), but say 3.0.0a3, 1899b2 or release is not acceptable. Is there a tag/version like that we can use now ? or should I pull from 3.0.0a3 or 1899b2 ? (again you cannot put them in DESCRIPTION) Which works fine in R 4.4.1 but 4.3 is failing for some reason |
|
@blowekamp seems my pushes are not triggering actions. |
|
I am not sure. There are some restrictions based on user, but I though I configured those so it would work for you. Please make a separate PR with just the trivial .gitignore changes. Perhaps having a commit in the repo will help. |
This PR is based on discussions across #72 #78 #80 and meetings with @richardbeare @blowekamp @zivy.
Description
README.md.Notes
SimpleITK/SimpleITK(see WIP: Add support for Windows #78 for more information).