CMake update

Engineering
1

Ubuntu 18.04 will soon be out. It comes with CMake 3.10.2. We said we will require that version for ITK 5.0. I suppose now is a good time to start requiring it? Anyone wanting to do that patching? Saying that is just as synchronization mechanism to avoid duplication of work :smiley:
@jhlegarreta I think this is relevant for your effort to update WikiExamples.

2

True, this was mentioned in the minimum cmake version thread at least.

I can update that in ITK, ITKWikiExamples, and the remote modules, given that I was last in modifying them concerning the noncopyable topic.

So whenever the community gives green light to it, I’ll submit the patches. Not before next week.

Once that done, I can update the WikiExamples CMakeLists in the wiki itself. However, as discussed here, this will still not be consistent with the tarballs located in Bill’s repository, and rather than sync’ing all wiki examples to actually match their current counterparts in GitHub (which is unfeasible in practice now), the big challenge would be transitioning the remaining ones to Sphinx so that we avoid the current sync’ing burden.

2 Likes
3

Done for

Working on the remotes now.

2 Likes
4

Thanks for working on it Jon!

1 Like
5

The dashboard build machines need to have their CMake version updated to at least 3.10.2 before that patch is merged. @seanm @blowekamp @fbudin @hjmjohnson can you check you build machines and give a green light when ready?

6

Unless this addresses a bug, this will be a low priority for me to update all our machines.

7

I’m ready!

Hans

1 Like
8

PRs to set the cmake_minimum_required to 3.10.2 for the remote modules:

Additionally, the following have a minimum ITK VERSION requirement as well, e.g.

  find_package(ITK 4.9 REQUIRED)

Do these need to be update to ITK 5.0?

Also, do the CMake policies present across the CMakeLists.txt files need to be updated? If, yes, which is the policy to be enfored?

Once the PRs accepted, I’ll submit a topic to gerrit to update the commit hashes.

Thanks.

1 Like
9

WOW! Great work.

10

I don’t remember why we added minimum version to RLEImage, but I don’t think it needs to be updated.

11

Awesome, Jon!

I do not think we should require ITK 5.0 unless it is really required, e.g. the ITK 5.0 C++11 scripts have been executed. This allows folks to still use the modules with older versions of ITK.

Many of the CMake policies can be removed because they default to NEW when a newer version of CMake is required.

12

The GitHub external module CI image, insighttoolkit/module-ci:latest has been updated to CMake 3.11.1.

13

Great! @hjmjohnson @matt.mccormick you can update ISC ITKIsotropicWavelets from upstream. It includes cmake update, and the recently merged itkViewImage is removed.

The ITKv4 compatible is the branch: https://github.com/phcerdan/ITKIsotropicWavelets/tree/ITK4.13-Compatible

14

@phcerdan Should we just remove the fork?

15

That is a good discussion to have for remote modules in general (pinging @jhlegarreta). My personal opinion is that if the original author is active the InsightSoftwareConsortium fork just creates extra maintenance burden. But maybe don’t remove it from ISC, and keep it alive to open Pull Requests from there to the original repo, in case a general update for all modules is required (as @hjmjohnson did with the c++11 update).

16

All remotes have gone through the C++11 update process :-S

Awesome. Thanks !

Yes, this is something that should be agreed upon to set a policy. @phcerdan is right in that it creates extra maintenance if the author is active. I tend to think that forks outside ISC and which are not active do also create extra maintenance and delays :slight_smile:

Then we should agree what being active means in this respect: 6-months, or 1-year without commits?

Not skilled at this, but is it feasible to think about some script for these maintenance/sync/upstream pull issues?

OTOH, is it advised to have the cmake_minimum_required updated for these ITK(Kitware)-related projects as well?

1 Like
17

This is a good idea. How about 1 month?

We could bump KWStyle.

Another option for ITKIsotropicWavelets is to transfer the repository to the InsightSoftwareConsortium.

18

I will be happy about this if I keep admin privileges on this one!

1 month without commits seems too short to me if the repo is stable. I mean, the maintainer can be active, but not actively working on the external module because there are no open issues, etc.

1 Like
19

Great! Certainly, you will keep admin privileges. Since you are member of the InsightSoftwareConsortium GitHub Org, I think you should be able to transfer the repository, according to this documentation. Please let us know if you have any issues.

Agreed – fork if no activity in over 1 year?

1 Like
20

Sounds good, and we can open an issue and ask directly to confirm.

Cool, I will do it next week.

1 Like