I couldn't get Qt4 easily on my WSL 24.04 so I made Gemini fix it. #15
Conversation
This commit modernizes the build system to support Qt 6 and modern C++ compilers. The key changes are: - Updated CMakeLists.txt files to use modern CMake and find Qt 6. - Added missing includes to the source code to support stricter C++ compilers. - Updated the protobuf handling to be compatible with the new build system. - Updated the README.md with the new build instructions. This commit was generated by an AI assistant.
There was a problem hiding this comment.
Pull Request Overview
This PR modernizes the build system by migrating from Qt4/Qt5 to Qt6 and updating CMake configurations to use modern practices. The changes aim to resolve compatibility issues with newer development environments where Qt4 is not easily available.
- Migrated from Qt4/Qt5 to Qt6 across all CMake files
- Enabled modern CMake automation features (AUTOMOC, AUTOUIC, AUTORCC)
- Added missing Qt header includes for stricter C++ compiler compatibility
Reviewed Changes
Copilot reviewed 9 out of 10 changed files in this pull request and generated 2 comments.
Show a summary per file
| File | Description |
|---|---|
| CMakeLists.txt | Simplified main CMake configuration to use Qt6 and modern CMake features |
| src/qt/CMakeLists.txt | Removed manual MOC handling, now relies on CMAKE_AUTOMOC |
| src/logplayer/CMakeLists.txt | Simplified build configuration for Qt6 |
| src/common/CMakeLists.txt | Updated library linking for Qt6 components |
| src/protobuf/CMakeLists.txt | Added custom protobuf generation logic with error handling |
| src/common/network.cpp | Added missing Qt header includes |
| src/common/format/*.cpp | Added missing Qt header includes |
| README.md | Updated build instructions for Qt6 and modern toolchain |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
You can also share your feedback on Copilot code review for a chance to win a $100 gift card. Take the survey.
| @@ -1,3 +1,9 @@ | |||
| find_package(Protobuf REQUIRED) | |||
| find_program(PROTOC_EXECUTABLE protoc HINTS /usr/bin) | |||
There was a problem hiding this comment.
The hardcoded path hint '/usr/bin' makes the build system less portable. Consider removing the HINTS parameter to let CMake find protoc in the standard system paths, or use a more comprehensive set of hints that covers different operating systems.
| find_program(PROTOC_EXECUTABLE protoc HINTS /usr/bin) | |
| find_program(PROTOC_EXECUTABLE protoc) |
There was a problem hiding this comment.
The comment seems reasonable. It works without the hint for me.
|
Looks ok on first scan. Have you tested it with Ubuntu? Also with older versions, at least 22.04? |
|
I did test it on WSL2 (so the actual Ubuntu VM version) on Ubuntu 24.04.3 and just now also on 22.04.5. I did not test on Ubuntu "on the metal" - although it is the same package sources and I expect it not to be a problem. I also added the two libraries I had to install on 22.04.5 to make the build go through. |
|
Works on my arm64-based Ubuntu 24.04 VM as well. Should be good after resolving the last comment. |
2 participants
This commit modernizes the build system to support Qt 6 and modern C++ compilers.
The key changes are:
This commit was generated by an AI assistant.