Zandronum Chat on our Discord Server Get the latest version: 3.0
Source Code

View Issue Details Jump to Notes ] Issue History ] Print ]
IDProjectCategoryView StatusDate SubmittedLast Update
0003824Doomseeker[All Projects] Cleanuppublic2020-06-09 14:382021-08-16 18:36
ReporterWubTheCaptain 
Assigned ToBlzut3 
PrioritylowSeveritytrivialReproducibilityalways
Statusneeds reviewResolutionopen 
PlatformOSOS Version
Product Version1.3.1 
Target Version1.4Fixed in Version 
Summary0003824: No AUTOUIC, qt5_wrap_ui() macro is used
DescriptionCMake's build files handle Qt's XML format user interface definition (.ui) files manually, with qt5_wrap_ui() macro. May be more ideal to use CMake's AUTOUIC to avoid cluttering the build files, which has been supported since CMake 3.0 or so.
Steps To Reproduce
$ grep -r "qt5_wrap_ui" .
./src/plugins/zandronum/CMakeLists.txt:qt5_wrap_ui(ZANDRONUM_UI_FILES
./src/core/CMakeLists.txt:qt5_wrap_ui(doomseekerUI ${UI_FILES})

$ grep -r "UI_FILES" .
./src/plugins/zandronum/CMakeLists.txt:qt5_wrap_ui(ZANDRONUM_UI_FILES
./src/plugins/zandronum/CMakeLists.txt: ${ZANDRONUM_UI_FILES}
./src/plugins/zandronum/CMakeLists.txt: ${ZANDRONUM_UI_FILES}
./src/core/CMakeSpawnFileListing.cmake:file(GLOB_RECURSE UI_FILES RELATIVE ${CMAKE_CURRENT_SOURCE_DIR} 
"*.ui")
./src/core/CMakeSpawnFileListing.cmake:append_file_list(${FILE_NAME} "UI_FILES" "${UI_FILES}")

./src/core/CMakeLists.txt:qt5_wrap_ui(doomseekerUI ${UI_FILES})
./src/core/CMakeLists.txt:      ${UI_FILES}
./src/core/CMakeFileListing.cmake:set(UI_FILES
Additional Informationhttps://cmake.org/cmake/help/latest/prop_tgt/AUTOUIC.html [^]
https://stackoverflow.com/a/42331649 [^]
git blame src/core/CMakeLists.txt says git show 95cdd026d, by Pol M.
Attached Filespatch file icon 0001-Reintroduce-cmake_policy-for-CMake-3.12.patch [^] (1,856 bytes) 2021-08-10 10:02 [Show Content]
patch file icon 0002-core-Bump-minimum-CMake-version-to-3.5.patch [^] (1,211 bytes) 2021-08-10 10:02 [Show Content]
patch file icon 0003-core-Suppress-CMake-CMP0028-warnings-again.patch [^] (1,056 bytes) 2021-08-10 10:55 [Show Content]

- Relationships
child of 0003895needs testingBlzut3 Add support for Qt 6.2+ 

-  Notes
User avatar (0021460)
WubTheCaptain (developer)
2020-06-09 14:40

And no offense to Pol M, the previous qt4_wrap_ui() that commit mentioned in OP was likely predating CMake 3.0 too.
User avatar (0021463)
Pol M (developer)
2020-06-15 00:22

Pretty in favor, I did not know this existed and it seems cleaner and more future proof. Any shenanigans that we do in cmake that can be automated further are a clear improvement.
No offense taken, I just took the simple path as at the time that was as much as it was needed.
User avatar (0021708)
Blzut3 (administrator)
2021-08-10 07:57

Enabled AUTOUIC:https://bitbucket.org/Doomseeker/doomseeker/commits/d6ec9cc30dc5373835511db1b15703b5491f6fbb [^]

Then noticed AUTORCC is a thing too so:https://bitbucket.org/Doomseeker/doomseeker/commits/952afc8de4b2854761348055b04b202276c83aef [^]
User avatar (0021709)
WubTheCaptain (developer)
2021-08-10 09:12

Quote
-cmake_minimum_required(VERSION 2.8.12)
+cmake_minimum_required(VERSION 3.5...3.21)


Quote from cmake_minimum_required — CMake 3.21.1 Documentation
If the running version of CMake is older than 3.12, the extra ... dots will be seen as version component separators, resulting in the ...<max> part being ignored and preserving the pre-3.12 behavior of basing policies on <min>.

https://cmake.org/cmake/help/v3.21/command/cmake_minimum_required.html [^]

Should that be
cmake_minimum_required(VERSION 3.5)
instead?

Quote
-if (POLICY CMP0071)
-	cmake_policy(SET CMP0071 NEW)
-endif()


Quote from CMP0071 — CMake 3.21.1 Documentation
New in version 3.10.

Quote from CMP0071 — CMake 3.21.1 Documentation
CMake version 3.21.1 warns when the policy is not set and uses OLD behavior.

https://cmake.org/cmake/help/v3.21/policy/CMP0071.html [^]
User avatar (0021710)
WubTheCaptain (developer)
2021-08-10 09:20

Sorry, I had misunderstood or misread the cmake_minimum_required documentation.
User avatar (0021712)
Blzut3 (administrator)
2021-08-10 09:23

Yeah the point of using a range is so that you can keep the minimum low while having all the new policies set to NEW automatically. I suppose you're correct that 3.10 and 3.11 would raise a warning on that though.
User avatar (0021713)
WubTheCaptain (developer)
2021-08-10 09:46

What requires CMake 3.5? As far as I know, AUTOUIC was introduced in 3.0.
User avatar (0021714)
WubTheCaptain (developer)
2021-08-10 10:03
edited on: 2021-08-10 10:03

Maybe the attached patches 0001 & 0002 address the notes from the previous code review. But I didn't compile this.

User avatar (0021715)
WubTheCaptain (developer)
2021-08-10 10:18

Also fyi, using AUTOUIC means we no longer support Ubuntu 14.04 LTS (which I believe only has extended security maintenance until April 2022, and has ended standard support already), which ships with CMake 2.8.12. Ubuntu 16.04 LTS is not affected - it has CMake 3.5.
User avatar (0021716)
Blzut3 (administrator)
2021-08-10 10:26

Can't comment on the patches right now, but as far as the 3.5 requirement that comes from practicality. This change requires 3.0, going to 3.1 allows dropping some legacy junk. It's not impossible that 3.1 could be the new literal minimum, but why spend cycles figuring that out? We only test for 3.5+ since that's what our CI container has. Given that we only really support Ubuntu 18.04+ and similar era distributions at this point (per policy of following Canonical's free support period) we really could bump it to 3.10, so 3.5 is being plenty generous I think.
User avatar (0021720)
Blzut3 (administrator)
2021-08-10 17:55
edited on: 2021-08-10 17:55

Patch 0001 - I believe we can just use cmake_policy(VERSION 3.21) with a comment that it can be removed after the minimum becomes 3.12. Would need to test.

Patch 0002 - The line can just be removed. I thought I did remove it, but I guess not.

Patch 0003 - That policy was introduced in 3.0 so even if the minimum was set to that this policy should always be NEW anyway.

User avatar (0021725)
Blzut3 (administrator)
2021-08-15 02:44

Policy issues should be addressed withhttps://bitbucket.org/Doomseeker/doomseeker/commits/5c20503430cc845bfaa7273b21475f036c50123d [^]

Turns out cmake_policy(VERSION) will reject versions greater than CMAKE_VERSION, but I suppose there's no reason we can't feed CMAKE_VERSION into it if we're running on a version of CMake less than 3.12 and effectively emulate the range behavior.

Issue Community Support
Only registered users can voice their support. Click here to register, or here to log in.
Supporters: Zalewa Pol M
Opponents: No one explicitly opposes this issue yet.

- Issue History
Date Modified Username Field Change
2020-06-09 14:38 WubTheCaptain New Issue
2020-06-09 14:40 WubTheCaptain Note Added: 0021460
2020-06-09 14:40 WubTheCaptain Severity feature => trivial
2020-06-15 00:22 Pol M Note Added: 0021463
2020-06-15 00:22 Pol M Status new => confirmed
2021-08-08 21:49 Blzut3 Relationship added child of 0003895
2021-08-10 05:43 WubTheCaptain Priority none => low
2021-08-10 07:46 Blzut3 Assigned To => Blzut3
2021-08-10 07:46 Blzut3 Status confirmed => assigned
2021-08-10 07:57 Blzut3 Note Added: 0021708
2021-08-10 07:57 Blzut3 Status assigned => needs testing
2021-08-10 07:57 Blzut3 Target Version => 1.4
2021-08-10 09:12 WubTheCaptain Note Added: 0021709
2021-08-10 09:12 WubTheCaptain Status needs testing => needs review
2021-08-10 09:20 WubTheCaptain Note Added: 0021710
2021-08-10 09:23 Blzut3 Note Added: 0021712
2021-08-10 09:38 WubTheCaptain Assigned To Blzut3 => WubTheCaptain
2021-08-10 09:38 WubTheCaptain Status needs review => assigned
2021-08-10 09:46 WubTheCaptain Note Added: 0021713
2021-08-10 10:02 WubTheCaptain File Added: 0001-Reintroduce-cmake_policy-for-CMake-3.12.patch
2021-08-10 10:02 WubTheCaptain File Added: 0002-core-Bump-minimum-CMake-version-to-3.5.patch
2021-08-10 10:03 WubTheCaptain Note Added: 0021714
2021-08-10 10:03 WubTheCaptain Assigned To WubTheCaptain => Blzut3
2021-08-10 10:03 WubTheCaptain Status assigned => needs review
2021-08-10 10:03 WubTheCaptain Note Edited: 0021714 View Revisions
2021-08-10 10:18 WubTheCaptain Note Added: 0021715
2021-08-10 10:26 Blzut3 Note Added: 0021716
2021-08-10 10:55 WubTheCaptain File Added: 0003-core-Suppress-CMake-CMP0028-warnings-again.patch
2021-08-10 17:55 Blzut3 Note Added: 0021720
2021-08-10 17:55 Blzut3 Note Edited: 0021720 View Revisions
2021-08-15 02:44 Blzut3 Note Added: 0021725
2021-08-16 18:36 WubTheCaptain Relationship added parent of 0003900
2021-08-16 18:36 WubTheCaptain Relationship deleted parent of 0003900






Questions or other issues? Contact Us.

Links


Copyright © 2000 - 2021 MantisBT Team
Powered by Mantis Bugtracker