NixOS Packaging Fixes - #676
Conversation
- boost::url parsing requires parse.hpp (boost 1.89 vs. 1.91) - boost packages need components listing - Missing find_package for BZip2 and PNG - Explicit linking required for aws-crt-cpp and PNG - GeographicLib should be passed camelcase to find_package
📝 WalkthroughSummary by CodeRabbit
WalkthroughCMake dependency requirements and link targets were tightened, Boost URL parsing was included, GCC 16 warning suppressions were expanded, CI added Linux compiler configurations, and Conan cache and PowerShell error handling were updated. ChangesBuild compatibility updates
Estimated code review effort: 3 (Moderate) | ~25 minutes 🚥 Pre-merge checks | ✅ 5✅ Passed checks (5 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 2
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@scwx-qt/scwx-qt.cmake`:
- Line 28: Update the find_package call for PNG to mark it as REQUIRED, ensuring
CMake configuration fails immediately when PNG::PNG is unavailable while
preserving the existing unconditional PNG::PNG linkage.
- Around line 796-800: Split the GCC 16 warning suppression options in the
target_compile_options calls for scwx-qt and supercell-wx into separate
generator expressions, or use SHELL: to ensure GCC receives -Wno-array-bounds
and -Wno-stringop-overflow as independent arguments.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: e504f246-1b66-4c5e-ae44-c039dbb9f7e8
📒 Files selected for processing (3)
scwx-qt/scwx-qt.cmakewxdata/source/scwx/provider/nws_level3_behavior.cppwxdata/wxdata.cmake
8125480 to
5f3500e
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
97-98: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winPrevent redundant Flatpak builds across new matrix entries.
Adding these new Ubuntu 24.04 and 26.04 matrix entries will cause the downstream
Build FlatPakandUpload FlatPaksteps to execute redundantly for each of the new compilers. Becauseflatpak-builderuses its own isolated freedesktop SDK and ignores the host'senv_cc, these extra Flatpak builds will be functionally identical but will consume significant CI time.Consider restricting the Flatpak steps (around lines 536-582) to only the
gcc-13jobs (as the existing comment "Publish only consumes gcc-13 artifacts" implies) by updating their conditions:if: ${{ contains(matrix.name, 'gcc-13') }}🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 97 - 98, Update the conditions on the downstream Build FlatPak and Upload FlatPak steps to run only when contains(matrix.name, 'gcc-13') is true. Use the existing matrix.name value and preserve all other matrix jobs and step behavior.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Around line 133-150: Update the linux_gcc-16_x64 matrix entry’s
compiler_packages value to install g++-16 instead of leaving it empty. If the
package is unavailable from the default Ubuntu 26.04 repositories, conditionally
add the ubuntu-toolchain-r/test PPA in the Setup Ubuntu Environment step for
this GCC 16 configuration.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 97-98: Update the conditions on the downstream Build FlatPak and
Upload FlatPak steps to run only when contains(matrix.name, 'gcc-13') is true.
Use the existing matrix.name value and preserve all other matrix jobs and step
behavior.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 31937634-7725-46c0-9ac5-d276027c43b8
📒 Files selected for processing (3)
.github/workflows/ci.ymlscwx-qt/scwx-qt.cmakewxdata/wxdata.cmake
🚧 Files skipped from review as they are similar to previous changes (2)
- scwx-qt/scwx-qt.cmake
- wxdata/wxdata.cmake
5f3500e to
4b7c213
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🧹 Nitpick comments (1)
.github/workflows/ci.yml (1)
536-540: 🚀 Performance & Scalability | 🔵 Trivial | ⚡ Quick winRestrict Flatpak build to
linux_gcc-13_x64to save CI resources.Since
flatpak-builderuses its own Freedesktop SDK internally (ignoring the host matrix compiler), running this step across all Ubuntu matrix jobs produces identical Flatpak bundles multiple times concurrently. As the comment notes, "Publish only consumes gcc-13 artifacts."Consider restricting this build step (and its corresponding upload step) to only the
linux_gcc-13_x64job.⚡ Proposed fix for both build and upload steps
- name: Build FlatPak (Linux) # Ubuntu 22.04 ships flatpak-builder < 1.4, which still runs appstream-compose # inside the SDK. Freedesktop 25.08 removed that tool; 24.04+ builder uses # host appstreamcli compose instead. Publish only consumes gcc-13 artifacts. - if: ${{ startsWith(matrix.os, 'ubuntu') && matrix.os != 'ubuntu-22.04' }} + if: ${{ matrix.name == 'linux_gcc-13_x64' }} env: INSTALL_DIR: ${{ github.workspace }}/supercell-wx/ FLATPAK_DIR: ${{ github.workspace }}/supercell-wx-flatpak/And similarly, update the upload step condition:
- name: Upload FlatPak (Linux) - if: ${{ startsWith(matrix.os, 'ubuntu') && matrix.os != 'ubuntu-22.04' }} + if: ${{ matrix.name == 'linux_gcc-13_x64' }} uses: actions/upload-artifact@v7 with:🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/ci.yml around lines 536 - 540, Restrict the “Build FlatPak (Linux)” step condition to run only for the linux_gcc-13_x64 matrix job, while retaining the Ubuntu-version exclusion. Apply the same job-specific condition to the corresponding FlatPak upload step so only the gcc-13 artifact is built and published.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In @.github/workflows/ci.yml:
- Line 116: Replace the unsupported ubuntu-26.04 runner label with a supported
label such as ubuntu-24.04 at .github/workflows/ci.yml lines 116, 134, 206, and
224, and ensure each environment setup adds the appropriate package repositories
for the newer compilers.
---
Nitpick comments:
In @.github/workflows/ci.yml:
- Around line 536-540: Restrict the “Build FlatPak (Linux)” step condition to
run only for the linux_gcc-13_x64 matrix job, while retaining the Ubuntu-version
exclusion. Apply the same job-specific condition to the corresponding FlatPak
upload step so only the gcc-13 artifact is built and published.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 227ae044-dc7d-49ce-8047-047ff3c10ea2
📒 Files selected for processing (1)
.github/workflows/ci.yml
If conan install succeeds and the overall job fails, the conan cache is still saved. Additionally, failures were not propagating due to `conan install` not being the last command in the step.
There was a problem hiding this comment.
🧹 Nitpick comments (1)
.github/workflows/clang-tidy-review.yml (1)
100-101: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low valueConsider applying strict native error handling to other PowerShell steps.
The addition of
$PSNativeCommandUseErrorActionPreference = $trueeffectively ensures fail-fast behavior for this step. Consider adding this setting to otherpwshsteps across your workflows that sequentially invoke multiple native commands (such as runningcmakefollowed byninjain theAutogeneratestep).By default, PowerShell in GitHub Actions continues executing the script if a native command returns a non-zero exit code. Enforcing this preference globally or in other heavily native steps prevents cascading errors and confusing logs when an intermediate command fails.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the rest with a brief reason, keep changes minimal, and validate. In @.github/workflows/clang-tidy-review.yml around lines 100 - 101, Apply $PSNativeCommandUseErrorActionPreference = $true to the other multi-command pwsh workflow steps, especially the Autogenerate step that runs cmake followed by ninja. Keep $ErrorActionPreference = 'Stop' alongside it so every native command failure stops the step before subsequent commands execute.
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Nitpick comments:
In @.github/workflows/clang-tidy-review.yml:
- Around line 100-101: Apply $PSNativeCommandUseErrorActionPreference = $true to
the other multi-command pwsh workflow steps, especially the Autogenerate step
that runs cmake followed by ninja. Keep $ErrorActionPreference = 'Stop'
alongside it so every native command failure stops the step before subsequent
commands execute.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 9586ed13-0aa8-4579-9f07-8006cb9b0fe8
📒 Files selected for processing (3)
.github/workflows/ci.yml.github/workflows/clang-tidy-review.yml.github/workflows/sign-windows-packages.yml
1 participant
Compatibility fixes for downstream NixOS packaging, reducing the number of patches required to build for nixpkgs
Also fixing incidental finding with GCC 16 and Boost, with -Wstringop-overflow false positives