Skip to content

Conversation

@AnyOldName3
Copy link
Member

So far, I've got to CMake 3.0, and the bump from 2.8 to 3.0 was bigger than 3.0 to 3.5, so going all the way to 3.5 hopefully will be something I can do in one sitting.

… cmake_minimum_required

These were already NEW because the OSG already uses 2.8
CMP0012 to CMP0023 do not look relevant to this one file.
This now-illegal macro doesn't seem to be used anywhere
It was only used to check OSG's minimum version anyway.
It seems to be a hallucinated way to check the current CMake version - it's not mentioned even in old versions of the docs - but LLMs wouldn't be invented for well over a decade.
Spaces are illegal in target names (although legal in labels - it's not worth setting that up here, though)
All policies up to CMP0050 have been verified compatible (after far more tweaking than I'd have liked)
I've verified all the policies up to CMP0056
I didn't notice that the condition was inverted so this code had become unreachable rather than unavoidable.
This one definitely broke OpenMW when we bumped our minimum CMake version past 3.5, but that meant that we already knew about it and already knew how to fix it and which platforms were affected.
@AnyOldName3
Copy link
Member Author

I believe we've got this to where it needs to be, at least until CMake drop support for versions below 3.10 within the next few months/years.

@Capostrophic
Copy link

LGTM

@AnyOldName3
Copy link
Member Author

The test branch I pushed to confirm that this resolved the FFMPEG CMake issue with vcpkg finished building, so we've done all the testing we can do other than put dev builds in the hands of users and see if we get bug reports.

@AnyOldName3
Copy link
Member Author

LGTM

The approval button is in the Files changed tab.

@Capostrophic
Copy link

The approval button is in the Files changed tab.

Well, doesn't really matter, does it? Just a formality. No approval requirement set up here. 😛

@AnyOldName3
Copy link
Member Author

The machine-enforced version of the policy isn't quite the real policy on any of our repos. Anyway, the main thing is that approvals show up in the approval total, but LGTM comments don't, so they're harder to find.

If we go with the standard OpenMW rules, we're waiting for:

  • @ssunday's threads to be marked resolved. I don't know if she's got the Resolve conversation button, but they've got 👍 on them, so this doesn't really matter.
  • A second approval/it being someone other than me or Capo pressing the merge button.
  • Technically, we don't need more hours, as I opened a partially-complete version of this PR on Saturday night, but the bit that made it fit for purpose and match its own title only got pushed last night, so it'd be justifiable to claim the PR was opened prematurely and the real countdown still has nearly ten hours left.

@Capostrophic Capostrophic merged commit 61a4c88 into OpenMW:3.6 Jan 20, 2026
1 check passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants