Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Run CMake with -Werror=dev in CI #7322

Merged
merged 8 commits into from
Jul 6, 2024
Merged

Conversation

DomClark
Copy link
Member

This causes CMake warnings (including AUTHOR_WARNING and DEPRECATION level messages) to be reported as errors, which means that all CMake warnings must be fixed for CI to pass. See the CMake documentation for this flag.

@DomClark
Copy link
Member Author

For MinGW builds, the mingw-std-threads submodule triggers a deprecation message, so I had to make deprecation messages warning only there. It should be possible to remove the -Wdeprecated flag with #7283. For Linux builds, the qt5-x11embed submodule and its own dependencies trigger both deprecation messages and normal warnings, so unfortunately we can't use -Werror=dev there at all. MSVC and macOS builds have no problems.

Disabling warnings or errors for third-party CMake code isn't trivial. The warning-as-error feature is controlled by the undocumented cache variables CMAKE_SUPPRESS_DEVELOPER_ERRORS and CMAKE_ERROR_DEPRECATED (this is different to the documented CMAKE_ERROR_DEPRECATED normal variable, which only affects message(DEPRECATION) and not internally-triggered deprecation warnings). It's possible to set these appropriately before using third-party code, then restore them afterward, but since they're cache variables, the disabled state will persist if CMake exits while in third-party code (due to an error, for example). As a workaround, it's possible to use set_property(CACHE PROPERTY VALUE) to restore the old value before running third-party code, as the warning mode is only updated internally in response to set(CACHE). However, this is yet another layer of undocumented behaviour on which to rely. As long as we catch most warnings for most jobs, I think it's better to stick with documented features and not worry about perfection. This is still an improvement over not checking for warnings at all, like before.

@FyiurAmron
Copy link
Contributor

@DomClark +1 on the PR. FWIW, mingw-std-threads shouldn't be a problem eventually, since it's possible to safely remove it when all the prerequisites for replacement are fulfilled, any workaround related to it is thus 100% OK in my book.

@Veratil
Copy link
Contributor

Veratil commented Jun 17, 2024

Configure warnings (more than just this one):

CMake Warning (dev) at /usr/share/cmake-3.29/Modules/FindPackageHandleStandardArgs.cmake:438 (message):
  The package name passed to `find_package_handle_standard_args` (XCB_XCB)
  does not match the name of the calling package (XCB).  This can lead to
  problems in calling code that expects `find_package` result variables
  (e.g., `_FOUND`) to follow a certain pattern.
Call Stack (most recent call first):
  src/3rdparty/qt5-x11embed/3rdparty/ECM/modules/ECMFindModuleHelpers.cmake:248 (find_package_handle_standard_args)
  src/3rdparty/qt5-x11embed/3rdparty/ECM/find-modules/FindXCB.cmake:183 (ecm_find_package_handle_library_components)
  src/3rdparty/qt5-x11embed/CMakeLists.txt:9 (find_package)
This warning is for project developers.  Use -Wno-dev to suppress it.

@DomClark
Copy link
Member Author

Configure warnings (more than just this one)

These are known about, and are in third-party code that I'm not trying to fix. See my comment above.

@DomClark DomClark merged commit bdd94ec into LMMS:master Jul 6, 2024
10 checks passed
@DomClark DomClark deleted the cmake-werror-dev branch July 6, 2024 13:14
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.

None yet

4 participants