Skip to content

Conversation

a17r
Copy link

@a17r a17r commented Sep 7, 2025

No description provided.

@a17r a17r mentioned this pull request Sep 7, 2025
@AnyOldName3
Copy link
Member

There are a few potential problems here, which is why we've not done this already:

  • The OSG's build system is particularly fragile, especially on non-Linux platforms (which it nominally supports several of beyond the obvious Windows and MacOS), so any change is likely to break something. In our fork, it's probably reasonable to not care about anything other than Linux, Windows, MacOS and Android, but it still needs more care and attention than the average CMake-based project.
  • Changing the version number passed to cmake_minimum_required toggles all CMake policies introduced between the two versions, so they all need checking in case anything affects the project. We already know that CMP0065, which is part of this range, makes a difference to how OSG plugins behave with Clang, and we never got to the bottom of whether that's a Clang bug or valid behaviour according to the C++ spec.
  • We don't have a way to guarantee we find everything that gets broken in a timely manner - it took a whole year after we changed the minimum CMake version in OpenMW to discover that doing so broke things because of CPM0065.
  • We're going to have to keep doing this as CMake will keep dropping support for older versions. CMake 4.0 gives a warning if you specify a version below 3.10 that tells you support will be dropped soon. If we keep increasing the minimum version and it takes us a year to identify problems each time, then it means we'll be permanently in a state of having undiscovered regressions without an obvious cause.

All this isn't necessarily to say this PR isn't something we want (although it'll need to enable exports at least under Clang for all the applications and examples to work around the CMP0065 regression), just that it needs an abundance of caution, and if we can think of a more reliable way to avoid problems, it might be preferable.

@a17r
Copy link
Author

a17r commented Sep 7, 2025

Thanks a lot for your reply. I hope we can work out those problems, along with silencing more warnings in current cmake output, with more eyes on the PR. The primary workaround to avoid a CMake error w/ >=CMake-4 for other distributions will be simply setting -DCMAKE_POLICY_VERSION_MINIMUM=3.5 which has just the same potential for cmake_policy induced havoc.

OSG upstream's inactivity has already lead to another fork: https://gitlab.com/flightgear/openscenegraph This is very unfortunate for distributions who are faced with namespace colliding libs and ending up dealing with the same issues in multiple packages ... They have already bumped to 3.20 though, and their repository could be skimmed for related fallout fixes, even if they are more agressively disabling stuff they don't need.

@AnyOldName3
Copy link
Member

If packagers make changes without testing them properly and it breaks their packages, that's their own responsibility and not something that OpenMW's maintainers have to end up dealing with. We still have the option of recommending people use CMake 3.x.

Project-specific forks aren't generally a good candidate to install system-wide, and should really be installed to their own directory or kept as part of the build system of the project that uses them. E.g. the Fedora package for OpenMW statically links this fork to avoid conflicting with the upstream OSG package.

@a17r
Copy link
Author

a17r commented Sep 7, 2025

If packagers make changes without testing them properly and it breaks their packages, that's their own responsibility and not something that OpenMW's maintainers have to end up dealing with. We still have the option of recommending people use CMake 3.x.

Packagers very often do not have that choice, and chances of that increase over time and spread of conflicting build system version ranges preferred by upstreams. Already, we need to discourage other upstreams to choose CMake 4 as minimum version, which if simply accepted as-is, ultimately leads to a hard decision on which packages to kick out of a repository in order to keep the others.

@AnyOldName3
Copy link
Member

And that's those packager's problem, not the OpenMW team's problem, whereas if we're the ones carrying the version bump that breaks something, it would be our problem. We'll try and make things easy for packagers within reason, but certain configurations will always be at the packager's own risk because we don't have the resources to confirm they work.

@a17r
Copy link
Author

a17r commented Sep 7, 2025

And do you prefer to do this fully on your own pace, without any outside input? Please note that filing a PR does not mean I expect it to be merged the next day, verbatim, either. But one of these days another packager may come along, look over and test the change, and with combined CMake build system experience, we can get something done that eventually may be merged.

@a17r a17r marked this pull request as draft September 7, 2025 17:02
@AnyOldName3
Copy link
Member

Help is welcome, but it's important to keep in mind that once something's in our repos, it's got an implicit promise of some level of functionality and support from us that a downstream packager's patches wouldn't have, so we need to have a higher standard of being sure things work before we merge them than a downstream packager does. If someone submits a patch to us as their first contribution and it turns out to break something, they might have disappeared forever by the time it's discovered, and then we're on the hook to investigate it and fix it.

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.

2 participants