Repository navigation
Build Linux Packages - #78
Conversation
WalkthroughThe changes transition the Linux build and packaging process from QMake/Make to CMake/CPack, update the GitHub workflow to automate version string replacement and artifact uploads, and enhance the PowerShell build script with explicit build, package, and install steps for multiple platforms. CPack packaging metadata, Debian control scripts, and install destinations are also refined in the CMake configuration. Changes
Sequence Diagram(s)sequenceDiagram
participant GitHub Actions
participant Build Script
participant CMake
participant CPack
participant Artifact Store
GitHub Actions->>Build Script: Trigger Linux workflow
Build Script->>CMake: Configure & build (Release, install prefix)
Build Script->>CMake: Install to prefix (if requested)
Build Script->>CPack: Package in multiple archive and Linux package formats (if requested)
Build Script->>Artifact Store: Upload packaged artifacts with version/platform naming
Possibly related PRs
Poem
🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
SupportNeed help? Create a ticket on our support page for assistance with any issues or questions. Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
There was a problem hiding this comment.
Pull Request Overview
This pull request updates the Linux build workflow for the SQLiteQueryAnalyzer project by transitioning to a CMake-based build system, integrating CPack for packaging, and enhancing cross-platform build scripts.
- Replaces QMake/Make with CMake commands and steps to generate multi-format packages.
- Updates package metadata and installation paths in CMakeLists.txt.
- Enhances the PowerShell build script to support Linux and macOS installation and packaging.
Files not reviewed (2)
- src/project/CMakeLists.txt: Language not supported
- src/project/build.ps1: Language not supported
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (3)
src/project/CMakeLists.txt (1)
70-78: Good CPack configuration setupThe CPack configuration is well-structured with all necessary metadata for proper package generation. The Debian dependencies (libc6, libstdc++6, libgcc-s1) are appropriate standard system libraries.
Consider also adding RPM package dependencies with CPACK_RPM_PACKAGE_DEPENDS for more comprehensive cross-platform packaging support.
.github/workflows/linux.yml (1)
53-58: Missing newline at end of fileThe artifact publication looks good, but there's a missing newline character at the end of the file.
Add a newline character at the end of the file to comply with YAML best practices.
🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 58-58: no new line character at the end of file
(new-line-at-end-of-file)
src/project/build.ps1 (1)
39-46: Redundant conditional check for macOS packagingThe macOS build section has a redundant conditional check for $Package.
Line 43's
if ($Package)check is redundant since the entire block at line 39 already checksif ($IsMacOS -And $Package). You can remove the nested condition and keep the macdeployqt command.-if ($IsMacOS -And $Package) { +if ($IsMacOS) { cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/sqlitequery cmake --build build --config Release --parallel 32 - if ($Package) { + if ($Package) { macdeployqt build/SQLiteQueryAnalyzer.app -dmg -appstore-compliant } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/linux.yml(1 hunks)src/project/CMakeLists.txt(2 hunks)src/project/build.ps1(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/linux.yml
[error] 58-58: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (5)
src/project/CMakeLists.txt (1)
59-60: Well-structured installation path changesThe updates to installation paths are appropriate. Using the current directory for BUNDLE and ${CMAKE_INSTALL_BINDIR} for RUNTIME follows CMake best practices and provides better consistency with standard installation locations.
.github/workflows/linux.yml (3)
27-28: Consistent version managementGood approach to update the version in CMakeLists.txt, which aligns with the migration to CMake build system.
36-41: Well-structured CMake build processThe transition from qmake/make to CMake is well-implemented with appropriate configuration for Release builds and installation paths.
42-52: Comprehensive package format supportExcellent addition of multiple package formats using CPack, which will improve distribution options for different Linux environments.
src/project/build.ps1 (1)
1-7: Improved parameter structureThe parameter structure is now more consistent and provides separate control for packaging and installation operations.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
.github/workflows/linux.yml (1)
53-58: Missing newline at end of fileWhile the artifact publishing looks good, the file is missing a newline at the end, which some tools flag as an issue.
src/project/SQLiteQueryAnalyzer-${{ env.VERSION }}-Linux.* +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 58-58: no new line character at the end of file
(new-line-at-end-of-file)
src/project/build.ps1 (1)
39-45: Redundant package condition check in macOS sectionThe macOS section checks
$Packagetwice - once in the outer if statement and again in the nested if. This is redundant and could be simplified.-if ($IsMacOS -And $Package) { +if ($IsMacOS) { cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/sqlitequery cmake --build build --config Release --parallel 32 if ($Package) { macdeployqt build/SQLiteQueryAnalyzer.app -dmg -appstore-compliant } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
.github/workflows/linux.yml(1 hunks)src/project/CMakeLists.txt(2 hunks)src/project/build.ps1(1 hunks)
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/linux.yml
[error] 58-58: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (7)
src/project/CMakeLists.txt (2)
59-60: Updated installation destinations improve deployment consistencyThe updated BUNDLE and RUNTIME destinations properly align with CMake best practices. Setting BUNDLE DESTINATION to "." ensures the application bundle is installed in the expected location, while updating RUNTIME DESTINATION to ${CMAKE_INSTALL_BINDIR} follows standard system directory conventions.
70-78:Details
✅ Verification successful
Well-structured CPack configuration for multi-format packaging
The CPack configuration is comprehensive and properly sets up metadata for various package formats. The installation prefix in
/optis appropriate for standalone applications, and the Debian package dependencies cover the essential runtime libraries.One suggestion to consider - verify that the installation prefix is consistent across all your build scripts and documentation.
🏁 Script executed:
#!/bin/bash # Check for consistency of installation paths across build files grep -r "CMAKE_INSTALL_PREFIX\|CPACK_PACKAGING_INSTALL_PREFIX" .github/workflows/ src/project/Length of output: 505
Installation prefix consistency verified
The CI workflows and PowerShell build scripts use CMAKE_INSTALL_PREFIX=/tmp/sqlitequery to stage files, while CPACK_PACKAGING_INSTALL_PREFIX is deliberately set to /opt/sqlitequery in CMakeLists.txt for the final package install location. These values serve different purposes (staging vs. target installation) and are intentionally distinct—no changes required.
.github/workflows/linux.yml (3)
27-28: Effective version string replacement strategyThe PowerShell commands correctly update the version string in CMakeLists.txt, ensuring consistent versioning across the project.
36-41: Build process successfully modernized to use CMakeThe transition from qmake/make to CMake with explicit build type and installation prefix is a significant improvement. The parallel build flag will accelerate CI builds on multicore systems.
42-52: Comprehensive packaging strategy with multiple formatsThe CPack commands generate an impressive variety of package formats (archives and Linux-specific packages), providing excellent distribution flexibility.
src/project/build.ps1 (2)
3-6: Parameter naming improved and installation option addedCapitalizing
$Packagefollows PowerShell conventions, and adding the$Installswitch provides useful flexibility for users.
18-31: Comprehensive Linux build and packaging configurationThe updated Linux build section properly configures CMake with release mode and generates multiple package formats consistent with the GitHub workflow.
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (2)
src/project/CMakeLists.txt (2)
10-13: Multi-line formatting for macOS resource is acceptable but consider consistency.
The refactoredset_source_files_propertiescall remains functionally identical. For readability, ensure that similar blocks elsewhere follow this indentation and quoting style (e.g., no unnecessary quotes around paths unless required).
56-58: Unify indentation fortarget_link_libraries.
The extra indentation here differs from the earlier Qt::Core/Gui/Sql block. For consistency, align the lists in alltarget_link_librariescalls:-if ((QT_VERSION_MAJOR GREATER 4)) - target_link_libraries(SQLiteQueryAnalyzer PRIVATE - Qt::Widgets - ) -endif () +if (QT_VERSION_MAJOR GREATER 4) + target_link_libraries(SQLiteQueryAnalyzer PRIVATE Qt::Widgets) +endif()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (3)
src/project/CMakeLists.txt(3 hunks)src/project/postinst(1 hunks)src/project/prerm(1 hunks)
✅ Files skipped from review due to trivial changes (2)
- src/project/prerm
- src/project/postinst
⏰ Context from checks skipped due to timeout of 90000ms (2)
- GitHub Check: macos-intel / template
- GitHub Check: build
🔇 Additional comments (1)
src/project/CMakeLists.txt (1)
62-63: Verify install destinations for all platforms.
InstallingBUNDLE DESTINATION .is intended for macOS bundles, whileRUNTIME DESTINATION ${CMAKE_INSTALL_BINDIR}targets executables on other systems. Confirm in CI and package tests that:
- On macOS, the
.appends up at<prefix>/SQLiteQueryAnalyzer.app.- On Linux/Windows, the binary installs to
<prefix>/bin(as perCMAKE_INSTALL_BINDIR).
| set(CPACK_PACKAGE_NAME SQLiteQueryAnalyzer) | ||
| set(CPACK_PACKAGE_DESCRIPTION_SUMMARY "SQLite Query Analyzer") | ||
| set(CPACK_PACKAGE_VENDOR "Christian Resma Helle") | ||
| set(CPACK_PACKAGE_INSTALL_DIRECTORY ${CPACK_PACKAGE_NAME}) | ||
| set(CPACK_VERBATIM_VARIABLES ON) | ||
| set(CPACK_PACKAGING_INSTALL_PREFIX "/opt/sqlitequery") | ||
| set(CPACK_DEBIAN_PACKAGE_MAINTAINER "Christian Resma Helle <crhelle@proton.me>") | ||
| set(CPACK_DEBIAN_PACKAGE_DEPENDS libxcb-xinerama0) | ||
| set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA "${CMAKE_CURRENT_SOURCE_DIR}/postinst" PARENT_SCOPE) | ||
| set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA "${CMAKE_CURRENT_SOURCE_DIR}/prerm" PARENT_SCOPE) | ||
| include(CPack) |
There was a problem hiding this comment.
Fix Debian control script inclusion to avoid overriding.
Currently, two separate set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA … PARENT_SCOPE) calls will overwrite each other, causing only prerm to be installed. Combine them into a single list and drop PARENT_SCOPE (unnecessary at top-level):
-set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA "${CMAKE_CURRENT_SOURCE_DIR}/postinst" PARENT_SCOPE)
-set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA "${CMAKE_CURRENT_SOURCE_DIR}/prerm" PARENT_SCOPE)
+set(CPACK_DEBIAN_PACKAGE_CONTROL_EXTRA
+ "${CMAKE_CURRENT_SOURCE_DIR}/postinst"
+ "${CMAKE_CURRENT_SOURCE_DIR}/prerm"
+)
Co-authored-by: Copilot <175728472+Copilot@users.noreply.github.com>
f3e06d4 to
29605d8
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (2)
src/project/build.ps1 (2)
33-36:⚠️ Potential issueLinux installation symlink is reversed.
The symbolic link command has the source and target reversed. It should create a symlink from the system directory to the installed binary, not the other way around.
if ($Install) { cmake --install build - ln -s ~/.local/bin/sqlitequery /tmp/sqlitequery/bin/SQLiteQueryAnalyzer + mkdir -p ~/.local/bin + ln -s /tmp/sqlitequery/bin/SQLiteQueryAnalyzer ~/.local/bin/sqlitequery }Additionally, you should create the
~/.local/bindirectory if it doesn't exist, as it's not guaranteed to be present on all Linux distributions.
33-36: 🛠️ Refactor suggestionConsider a more reliable installation location than /tmp
Using
/tmpfor installation is problematic as this directory is cleared on system reboot on most Linux distributions, leading to lost installations.Consider using a more persistent location aligned with the XDG specification:
if ($Install) { - cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/sqlitequery + cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX="$env:HOME/.local" cmake --build build --config Release --parallel 32 cmake --install build - ln -s ~/.local/bin/sqlitequery /tmp/sqlitequery/bin/SQLiteQueryAnalyzer }This installs directly to a user-specific location that persists across reboots and follows the XDG specification, eliminating the need for manual symlink creation.
🧹 Nitpick comments (3)
.github/workflows/linux.yml (2)
36-40: The CMake configuration looks good, but there's a redundant prefix setting.The
--prefixflag in the install command is redundant since you've already set the install prefix during configuration. Both are using the same value, so it's not causing issues, but one of them is unnecessary.cmake -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/sqlitequery . cmake --build . --config Release --parallel 32 - cmake --install . --config Release --prefix /tmp/sqlitequery + cmake --install . --config Release
53-58: Artifact publishing looks good, but missing newline at end of file.The artifact naming is correct, and the path pattern will capture all generated package formats.
Add a newline at the end of the file to satisfy YAML linting requirements:
path: | src/project/SQLiteQueryAnalyzer-${{ env.VERSION }}-Linux.* +🧰 Tools
🪛 YAMLlint (1.35.1)
[error] 58-58: no new line character at the end of file
(new-line-at-end-of-file)
src/project/build.ps1 (1)
39-46: macOS build block has a redundant condition.The condition on line 39 already checks for
$IsMacOS -And $Package, making the innerif ($Package)check redundant.-if ($IsMacOS -And $Package) { +if ($IsMacOS) { cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/sqlitequery cmake --build build --config Release --parallel 32 if ($Package) { macdeployqt build/SQLiteQueryAnalyzer.app -dmg -appstore-compliant } }
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (5)
.github/workflows/linux.yml(1 hunks)src/project/CMakeLists.txt(3 hunks)src/project/build.ps1(1 hunks)src/project/postinst(1 hunks)src/project/prerm(1 hunks)
🚧 Files skipped from review as they are similar to previous changes (3)
- src/project/prerm
- src/project/postinst
- src/project/CMakeLists.txt
🧰 Additional context used
🪛 YAMLlint (1.35.1)
.github/workflows/linux.yml
[error] 58-58: no new line character at the end of file
(new-line-at-end-of-file)
🔇 Additional comments (5)
.github/workflows/linux.yml (2)
27-28: Version string update in CMakeLists.txt looks good.The script now updates the version in the CMakeLists.txt file to maintain consistency with the version defined in the environment variables, which is an important part of the transition to CMake-based builds.
42-52: Comprehensive packaging approach with CPack.Using CPack to generate multiple package formats provides excellent flexibility for users. The inclusion of both archive formats (7Z, ZIP, TBZ2, etc.) and Linux package formats (DEB, RPM) is a good approach for distribution.
src/project/build.ps1 (3)
2-7: Parameter declaration improvements are good.The script now properly declares parameters using PowerShell conventions, with the
$Packageparameter renamed (capitalized) and a new$Installparameter added to support installation operations.
12-12: Variable name has been properly capitalized.The conditional now uses the capitalized
$Packageparameter name, consistent with the parameter declaration.
18-31: Linux build and package steps look good.The Linux-specific build section now properly uses CMake with appropriate build type and installation paths. The packaging options with CPack are comprehensive and align with the workflow configuration.
Co-authored-by: coderabbitai[bot] <136622811+coderabbitai[bot]@users.noreply.github.com>
|
There was a problem hiding this comment.
Actionable comments posted: 0
♻️ Duplicate comments (1)
src/project/build.ps1 (1)
33-37:⚠️ Potential issueEnsure reliable install location on Linux
The installation to
/tmp/sqlitequeryis problematic as this is an ephemeral directory that will be cleared on system reboot, leading to the application being unavailable after restart.Consider using a persistent location following the FHS or XDG specifications:
- cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/sqlitequery + cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX="$env:HOME/.local" if ($Install) { cmake --install build - mkdir -p ~/.local/bin - ln -s /tmp/sqlitequery/bin/SQLiteQueryAnalyzer ~/.local/bin/sqlitequery }Using
$env:HOME/.localas the install prefix follows the XDG Base Directory Specification and eliminates the need for manual symlinking.
🧹 Nitpick comments (2)
src/project/build.ps1 (2)
40-46: Simplify macOS conditional logicThe
$Packagecondition is checked twice unnecessarily.- if ($IsMacOS -And $Package) { + if ($IsMacOS) { cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/sqlitequery cmake --build build --config Release --parallel 32 if ($Package) { macdeployqt build/SQLiteQueryAnalyzer.app -dmg -appstore-compliant } }
40-47: Consider adding install support for macOSThe script supports installation on Linux but not on macOS. For consistency across platforms, consider adding similar install functionality for macOS.
if ($IsMacOS) { cmake -B build -DCMAKE_BUILD_TYPE=Release -DCMAKE_INSTALL_PREFIX=/tmp/sqlitequery cmake --build build --config Release --parallel 32 if ($Package) { macdeployqt build/SQLiteQueryAnalyzer.app -dmg -appstore-compliant } + if ($Install) { + cmake --install build + mkdir -p /usr/local/bin + ln -s /tmp/sqlitequery/bin/SQLiteQueryAnalyzer /usr/local/bin/sqlitequery + } }Note that this would also benefit from using a persistent installation path rather than
/tmp/sqlitequery.
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Plan: Pro
📒 Files selected for processing (1)
src/project/build.ps1(1 hunks)
⏰ Context from checks skipped due to timeout of 90000ms (3)
- GitHub Check: build / template
- GitHub Check: macos-intel / template
- GitHub Check: build
🔇 Additional comments (4)
src/project/build.ps1 (4)
3-6: Good parameter naming improvementsThe capitalization of
$Packageparameter follows PowerShell naming conventions better, and the addition of the$Installswitch is a good separation of build, package, and install concerns.
12-12: LGTM - Parameter name updated for consistencyUpdated to use the renamed parameter with capital P.
18-20: Improved explicit CMake configurationThe separate Linux block with explicit
CMAKE_BUILD_TYPEand installation prefix is clearer than before.
22-31: Comprehensive packaging options for LinuxThis is a great addition using CPack to support multiple packaging formats, aligning with the new CPack configuration in CMakeLists.txt.
This pull request introduces significant changes to the build and packaging workflows for the
SQLiteQueryAnalyzerproject. The modifications transition the project from usingqmakeandmaketo aCMake-based workflow, add support for packaging usingCPack, and enhance cross-platform build scripts. Below are the most important changes grouped by theme:Workflow and Build System Updates
.github/workflows/linux.ymlto replaceqmakeandmakewithCMakecommands for building, installing, and packaging. Added steps to generate multiple package formats (e.g.,.tar.gz,.deb,.rpm) and publish artifacts. ([.github/workflows/linux.ymlR27-R59](https://github.com/christianhelle/sqlitequery/pull/78/files#diff-21364b2e6fae1f2875cee1ab3daefb0685403687eaf8bc32b5c6eacda351c9d3R27-R59))Packaging Configuration
CPackconfiguration tosrc/project/CMakeLists.txt, including package metadata (e.g., name, description, vendor) and dependencies for Debian packages. ([src/project/CMakeLists.txtR69-R78](https://github.com/christianhelle/sqlitequery/pull/78/files#diff-d0b751957f9fb4c59680d6ce93db372b62d73a014e7986575db3ba0b1aff67ccR69-R78))CMakeLists.txtto ensure proper runtime and bundle destinations. ([src/project/CMakeLists.txtL59-R60](https://github.com/christianhelle/sqlitequery/pull/78/files#diff-d0b751957f9fb4c59680d6ce93db372b62d73a014e7986575db3ba0b1aff67ccL59-R60))Cross-Platform Build Script Enhancements
src/project/build.ps1to support packaging and installation on Linux and macOS. Introduced new flags (-Package,-Install) and addedCPackcommands for generating multiple package formats. ([src/project/build.ps1L3-R46](https://github.com/christianhelle/sqlitequery/pull/78/files#diff-3de23a5a48e2c382cd3171ccf8f406735f8588df45aeb0af436822b521fbf0ddL3-R46))Summary by CodeRabbit
New Features
Improvements