Add Flatpak submodule and update CI configuration#15059
Add Flatpak submodule and update CI configuration#15059besdar wants to merge 5 commits into
Conversation
📝 WalkthroughWalkthroughThis PR migrates Flatpak packaging to the ChangesFlatpak Submodule Migration
Estimated code review effort: 2 (Simple) | ~12 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 |
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/flutter-build.yml:
- Around line 1980-1996: Add fail-fast validation around the jq manifest
transformation: before running the jq filter against com.rustdesk.RustDesk.json,
verify that a module with .name == "rustdesk" exists (fail and exit non-zero if
not); after writing com.rustdesk.RustDesk.local.json, validate that the rustdesk
module's .sources contains the two file entries ("rustdesk.deb" and
"com.rustdesk.RustDesk.metainfo.xml") and fail-fast if the expected sources are
not present. Tie these checks to the same shell step that runs the jq filter and
flatpak-builder so the script aborts immediately when the module is missing or
the post-transformation verification fails. Ensure error messages mention the
module name "rustdesk" and the manifest filenames to aid debugging.
🪄 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: defaults
Review profile: CHILL
Plan: Pro
Run ID: 4d2170d6-ef0b-47ca-8134-c1fd99445a78
📒 Files selected for processing (7)
.github/workflows/ci.yml.github/workflows/flutter-build.yml.github/workflows/flutter-ci.yml.gitmodulesflatpak/com.rustdesk.RustDesk.metainfo.xmlflatpak/rustdesk.jsonres/bump.sh
💤 Files with no reviewable changes (2)
- flatpak/rustdesk.json
- flatpak/com.rustdesk.RustDesk.metainfo.xml
There was a problem hiding this comment.
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
.github/workflows/flutter-build.yml (1)
1203-1211: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick winReplace
matrix.job.archinbuild-rustdesk-android-universal
This job has nostrategy.matrix, so the upload-artifact name will fail to resolve. Use a fixed value here, e.g.universal, instead of${{ matrix.job.arch }}.🤖 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/flutter-build.yml around lines 1203 - 1211, The build-rustdesk-android-universal job does not define a strategy.matrix, so any upload-artifact name that references matrix.job.arch will not resolve. Update the artifact naming in build-rustdesk-android-universal to use a fixed identifier such as universal, and keep the change scoped to the upload-artifact step in this job.Source: Linters/SAST tools
🧹 Nitpick comments (2)
.github/workflows/flutter-build.yml (2)
216-219: 🩺 Stability & Availability | 🔵 Trivial | ⚖️ Poor tradeoff
Swatinem/rust-cacheon the new arm64 build job enables caching by default (cache-poisoning risk).zizmor flags this as a potential cache-poisoning vector since untrusted PR runs could seed cache entries consumed by trusted runs. This is consistent with the same pattern already present elsewhere in the file, so it's more of a repo-wide posture question than something introduced solely by this PR.
🤖 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/flutter-build.yml around lines 216 - 219, The rust-cache step in the workflow is enabled by default on the new arm64 job, which can allow untrusted PR runs to seed caches used by trusted runs. Update the Swatinem/rust-cache configuration in the workflow to disable caching for untrusted contexts or otherwise constrain it so only trusted runs can write/read the cache, matching the repo’s safer caching posture used elsewhere.Source: Linters/SAST tools
109-119: 🔒 Security & Privacy | 🔵 Trivial | ⚡ Quick winNewly-added
actions/checkoutsteps (arm64 Windows job, etc.) don't setpersist-credentials: false.zizmor flags credential persistence through GitHub Actions artifacts on these
actions/checkout@...steps withsubmodules: recursive. Persisting the token in the local git config can leak it if any later step uploads or inspects.git/configin artifacts. Since several of these checkout steps are newly added/modified as part of this diff, consider adding the flag.♻️ Proposed fix
- name: Checkout source code uses: actions/checkout@34e114876b0b11c390a56381ad16ebd13914f8d5 # v4 with: submodules: recursive + persist-credentials: falseAlso applies to: 405-408, 681-684, 995-997
🤖 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/flutter-build.yml around lines 109 - 119, The newly added checkout steps using actions/checkout with submodules: recursive should not persist GitHub credentials in the local git config. Update each affected checkout invocation in the workflow, including the checkout step referenced here and the other matching checkout steps noted in the review, to explicitly disable credential persistence by setting persist-credentials to false. This change should be applied on the actions/checkout blocks themselves so the token is not stored for later steps.Source: Linters/SAST tools
🤖 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.
Outside diff comments:
In @.github/workflows/flutter-build.yml:
- Around line 1203-1211: The build-rustdesk-android-universal job does not
define a strategy.matrix, so any upload-artifact name that references
matrix.job.arch will not resolve. Update the artifact naming in
build-rustdesk-android-universal to use a fixed identifier such as universal,
and keep the change scoped to the upload-artifact step in this job.
---
Nitpick comments:
In @.github/workflows/flutter-build.yml:
- Around line 216-219: The rust-cache step in the workflow is enabled by default
on the new arm64 job, which can allow untrusted PR runs to seed caches used by
trusted runs. Update the Swatinem/rust-cache configuration in the workflow to
disable caching for untrusted contexts or otherwise constrain it so only trusted
runs can write/read the cache, matching the repo’s safer caching posture used
elsewhere.
- Around line 109-119: The newly added checkout steps using actions/checkout
with submodules: recursive should not persist GitHub credentials in the local
git config. Update each affected checkout invocation in the workflow, including
the checkout step referenced here and the other matching checkout steps noted in
the review, to explicitly disable credential persistence by setting
persist-credentials to false. This change should be applied on the
actions/checkout blocks themselves so the token is not stored for later steps.
ℹ️ Review info
⚙️ Run configuration
Configuration used: defaults
Review profile: CHILL
Plan: Pro
Run ID: 9b8d9167-f4f4-4950-bc47-74985510273b
📒 Files selected for processing (3)
.github/workflows/ci.yml.github/workflows/flutter-build.ymlres/bump.sh
🚧 Files skipped from review as they are similar to previous changes (1)
- res/bump.sh
Hello @rustdesk!
This PR removes the duplicated in-tree Flatpak manifest and switches RustDesk release builds to use the Flathub packaging repository through the
com.rustdesk.RustDesksubmodule.The old
flatpak/rustdesk.jsonhad drifted from Flathub. For example, PR #13530 fixed one instance of that drift by syncing runtime and PAM changes, but keeping two manifests means the same class of mismatch can happen again. With this change, the Flathub manifest becomes the source of truth for Flatpak packaging, and RustDesk CI only applies the small local changes needed to build release artifacts from the.debproduced by the pipeline.This resolves RustDesk/Flathub manifest misalignments once and forever: future Flathub packaging updates are consumed through the submodule instead of being manually mirrored into a second manifest.
What Changed
com.rustdesk.RustDeskas the Flatpak packaging submodule.flatpak/directory and its duplicated manifest/metainfo files.com.rustdesk.RustDesk/com.rustdesk.RustDesk.json.rustdesk.debartifact instead of downloading release.debfiles.--socket=wayland--socket=x11flatpak/directory.Summary by CodeRabbit
flatpak/*json, and regenerated Flatpak metadata to include both Wayland and X11 socket support.