List of CMake related items

[MikeGitb]

As discussed in https://github.com/cinder/Cinder/issues/2142 I would like to modernize the cmake build scripts. The purpose of this issue is to keep track of all planned and proposed changes related to that and in particular any ordering requirement. E.g. there are already some open PRs that should get merged before I want to start any bigger refactoring of the CMLs.

0. Existing PRs that should be merged first:

   • [ ] https://github.com/cinder/Cinder/pull/2117: Remove the forcing of "/MT"
   • [x] https://github.com/cinder/Cinder/pull/2060: Added videoInput.cpp to msw platform cmake file and fixed header path
   • [ ] https://github.com/cinder/Cinder/pull/1857: Svg samples CMake build
   • [x] https://github.com/cinder/Cinder/pull/2174: Remove trailing slash from CINDER_LIB_DIRECTORY

1. First round of new PRs (Preparatory work to establish proper test coverage):

   • [x] https://github.com/cinder/Cinder/pull/2190: Update cmake_minimum_required to 3.10 as per #2142 and remove obsoleted manual policy settings
   • [x] https://github.com/cinder/Cinder/pull/2195: Remove cmake_policy( SET CMP0015 OLD )
   • [x] https://github.com/cinder/Cinder/pull/2196: Fix unit tests with cmake and msvc
   • [x] https://github.com/cinder/Cinder/pull/2199: Fix travis to build only with supported distributions
   • [ ] TBD: Install recent version of cmake on MacOS on travis when testing with older xcode versions
   • [ ] TBD: Make sure CMake based builds are getting tested in ci (idealy with the "correct" cmake version)
   • [ ] TBD: Add ability to skip samples when using CINDER_BUILD_ALL_SAMPLES

2. Second round of new PRs (cleanup/simplification/fixes):

   • [ ] TBD: Enable CMP0091 (activate MSVC_RUNTIME_LIBRARY)
   • [ ] TBD: Require c++17 on msvc
   • [ ] TBD: Remove manualy setting of -std=c++XX flags.
   • [ ] TBD: Check if /MP and/or /FS can be removed
   • [ ] TBD: Check if /NODEFAULTLIB is necessary
   • [ ] TBD: Use CMAKE_VS_PLATFORM_TOOLSET / update vs platform detection logic (cinder is requiring VS2019)
   • [ ] TBD: Use capability of catch2 to create separate ctest target for each test
   • [ ] ... (some more cleanup possible?)

3. Third round of PRs (more invasive stuff):

   • [ ] TBD: Move CMLs to example root folders
   • [ ] TBD: Create separate targets (objectlibs) for the third party libs in cinder
   • [ ] ...

Other open PRs affecting CMake (but are e.g. outdated/not ready to merge/need review/might get superseeded ...)

• [ ] https://github.com/cinder/Cinder/pull/2168
• [x] https://github.com/cinder/Cinder/pull/2167
• [ ] https://github.com/cinder/Cinder/pull/1537
• [ ] https://github.com/cinder/Cinder/pull/2133

Adding @ufidstudios-ch as I believe he is also planning to update some cmake files

[alias-r-cummins]

Sounds good, if you want to go ahead and merge these first, I can add my changes on top.

[MikeGitb]

Sorry, the post might have been confusing:

I'm not a project member, so I can't merge anything. I just thought it would be good to have a central overview over past and future cmake related PRs to avoid invalidating existing PRs / duplicating work as much as possible.

[MikeGitb]

@andrewfb : Do you think you or someone else on the project will be available the next weeks to regularly merge those PRs (especially the existing ones from other people)? I'd prefer to work on this in small incremental steps, such that each PR is straight forward to review and merge and such that I can suspend my work and come back a couple of month later if necessary, without anything becomming out of date in the meantime. But of course that only makes sense, if there isn't a hughe turnaround time.

[andrewfb]

Hey @MikeGitb - thanks for taking this work on. I'm not super deep on the CMake side of things but I can certainly do the merging if we can build some (general) consensus around the changes themselves. I think we've at least got a minimum version established as a starting point, and I know there's a couple of PRs in the queue.

[kino-dome]

@MikeGitb thanks a lot for your work in gathering around all the Cmake PRs and giving them a structure. It'll be great if they could be pushed through soon to update all that is Cmake. While we're at the subject of the minimum version, 3.10 supports Ninja as well, right? It's just that I want to start working on a project that could benefit from ninja support and I thought that with the upcoming Cmake overhaul, this could be a good time to insure that :-) Thanks.

[kino-dome]

Hey @andrewfb, sorry to jump into the discussion but I had a little suggestion if that's alright. Seeing the many PRs that are unattended (some of them date back a long time) like the ones that Mike mentioned here, I was thinking that it might not be such a bad idea to add one or two of the active community members to the list of the project maintainers so they can help with the PR merging process to make it faster and more streamlined.

I think when looking at the merge history of Cinder, there are times that the maintainers are probably busy and can't attend to the GitHub activities that well and there are times that things move a lot faster.

I believe widening the circle of the maintainers can help with keeping a consistant flow when it comes for the community to work on and update Cinder as a whole. That it is of course not to say that the current maintainers are not doing their job or anything like that, I hope that's clear from my comment here :-)

Judging by my observations in the past, as a thought, I think Mike himself could be a great candidate for such a role. Of course these are all just my observations and my two cents here. It'd be great to know your and other community members' thoughts on this. And sorry again if it wasn't my place to come out and meddle in the maintainers' territory :-) Cheers!

[MikeGitb]

@kino-dome : Thanks for the confidence, but I think I'm using cinder far too rarely (and not in a professional setting) to be a good choice for a project maintainer.

I think we've at least got a minimum version established

yes, thats what #2190 does - could you please merge it?

and I know there's a couple of PRs in the queue.

Yes, some of them have been in the queue for years, which is one of the reasons I made this list to give them a little more visibility. In particular the ones I listed in step 0 should be straight forward to merge.

[MikeGitb]

@andrewfb : #2195, #2196, #2199 are ready to merge and I think none of them has any controversial changes in them. However, if you have any insights, as to why cmake_policy( SET CMP0015 OLD )was set in the first place (#2195) I would appreciate it.

The purpose of those PRs is to work towards a proper CI baseline for future changes. I still need to figure out, how I can make appveyor to also build cinder via cmake.

[MikeGitb]

@kino-dome: Maybe you could create a separate issue for the topic of finding additional maintainers. I think it is a topic worth dicussing, just not necessarily in the context of this particular issue.

[richardeakin]

Just want to chime in and say that although this work has not gotten done yet, it is much appreciated and a couple of us are looking at how to get the contributions merged. Personally, I'm starting with how to navigate #2117 since it turned out a bit controversial, and haven't really formed an opinion on the matter myself. From that, just planning to run down the list - @MikeGitb thanks again for organizing this.

[MikeGitb]

Thanks for the feedback. I'll probably be able to start working on some of the TBD items this or next weekend.

[richardeakin]

Cool - let us know if there is anything blocked or if you think some things are ready to merge that we've overlooked. For myself, I'm pretty busy on an unrelated project, but it'd be nice to work towards getting the cmake improvements into core.