fix: prevent launcher breakage when upgrading Linux packages - #209
Conversation
During a package upgrade (e.g. 0.2.0 → 0.2.1), the old package's post-uninstall.sh ran after the new post-install.sh, deleting the symlink, .desktop file, and icons — breaking the app launcher. - post-uninstall.sh: skip cleanup when another openstan-* versioned directory exists (indicates an upgrade, not a full uninstall) - fpm: add --replaces openstan so dpkg/dnf coordinate upgrades properly - Remove unused openstan.desktop at repo root (packaging/ version is the one used by cx_Freeze)
There was a problem hiding this comment.
🟡 Changes recommended
The upgrade-detection and fpm metadata changes include correctness/maintainability issues that should be addressed to avoid unintended uninstall behavior and confusing package metadata.
Once you've addressed the issues Copilot identified, you can request another Copilot review.
Pull request overview
Fixes Linux desktop launcher breakage during package upgrades by preventing the old package’s uninstall script from tearing down desktop integration mid-upgrade, and by cleaning up packaging/release artifacts to reduce future drift.
Changes:
- Update
packaging/post-uninstall.shto skip desktop cleanup when anopenstan-*install directory still exists (upgrade scenario). - Add
--replaces openstanto the fpm build commands in the release workflow (x86_64 + arm64). - Remove the repo-root
openstan.desktopfile (unused by the packaging path).
File summaries
| File | Description |
|---|---|
| packaging/post-uninstall.sh | Skips desktop-integration removal during upgrades to avoid breaking launchers. |
| .github/workflows/release.yml | Adjusts fpm packaging arguments for Linux release artifacts. |
| openstan.desktop | Removes an unused desktop entry file at repo root to reduce maintenance risk. |
Review details
Suppressed comments (1)
.github/workflows/release.yml:458
FPM_COMMONalready sets--name openstan, so adding--replaces openstanmakes the package declare it replaces itself. That metadata is redundant and can be confusing for future packaging changes; omit it unless you’re explicitly replacing a differently named legacy package.
--description "Open source bank statement analysis and visualization"
--license "LGPL-3.0-or-later"
--replaces openstan
--after-install packaging/post-install.sh
--after-remove packaging/post-uninstall.sh
- Files reviewed: 3/3 changed files
- Comments generated: 2
- Review effort level: Lite
💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.
- post-uninstall.sh: verify the openstan binary exists inside each versioned directory before skipping cleanup (avoids false positives from stale directories) - release.yml: remove redundant --replaces openstan (package already declares --name openstan, so replaces-self is a no-op)
|
Addressed both review comments:
|
Problem
Fixes #203
During a package upgrade (e.g. 0.2.0 → 0.2.1) on Fedora or Ubuntu, the old package's
post-uninstall.shran after the new package'spost-install.sh, deleting the/usr/bin/openstansymlink,.desktopfile, and icons — breaking the app launcher.Clean installs worked fine because there was no old package to run the removal script.
Changes
packaging/post-uninstall.sh— skip cleanup when anyopenstan-*versioned directory still exists under/usr/lib(indicates an upgrade, not a full uninstall).github/workflows/release.yml— add--replaces openstanto both x86_64 and ARM64 fpm builds so dpkg/dnf coordinate upgrades properlyopenstan.desktopat repo root — unused by packaging (packaging/openstan.desktopis the one used by cx_Freeze); its differentIconpath was a maintenance riskTesting