transaction: Ignore uninstall operations for no deploy

[dbnicholson]

If no_deploy has been set to TRUE in a transaction, then the intention is that no changes will be made to the installed flatpaks. Currently that's not the case for explicitly or implicitly added uninstall operations. That's particularly bad for eol-rebase flatpaks since they old version will be automatically removed without the new version being installed. To address this, prevent uninstall operations from being added for no deploy transactions.

Closes: #5172

[dbnicholson]

I don't know if this is the best way to address this. 2 other ways I considered:

• Mark the uninstall operations to be skipped. That way they're still recorded but not actually executed. This seemed like a good plan but didn't quite look right in the implementation. Normally operations are marked to be skipped when resolving, but that would mean that flatpak_transaction_get_operations would show the uninstall operations until the transaction was run. I also looked at immediately marking them as skipped, but it seemed like resolving only expected pre-skipped operations in certain scenarios that I didn't quite understand.
• Add a no_deploy argument to flatpak_dir_uninstall and have it become a no-op in that case. That would be the most straightforward way of recording the desired transactions without executing them as everything funnels there eventually. But a no deploy uninstall seemed a bit awkward. Certainly you wouldn't expect a --no-deploy option for flatpak uninstall.

Let me know if you think either of those options seemed better.

[wjt]

I think this will also fix #5171

[dbnicholson]

Ping?

[wjt]

I am not a Flatpak committer but this LGTM

[smcv]

Not reviewing this myself, sorry - I mostly deal with the sandboxing part of Flatpak rather than the package-management part. But perhaps @mwleeds or @alexlarsson can help?

[pwithnall]

The changes look good to me, and I agree with your reasoning about the other two approaches considered. I’m also not really a flatpak committer, though, so don’t feel like I can merge it.

On the other hand, this is a moderately big bug which needs fixing (because of its combination with eol-rebase and the typical no-deploy/no-pull split of applying flatpak updates via gnome-software), so if nobody else with a bigger flatpak maintainer hat comments in the next couple of weeks, I’d be minded to go ahead and merge.

[pwithnall]

We have also just merged a backport of this in Endless OS, so it will get some real-world testing there, which should (hopefully) give confidence in a future upstream merge of this PR.

[smcv]

To the best of my ability, this looks good, but again I'm not really so familiar with the package-management side of Flatpak.

Based on how sure the Endless team are about this, I'm going to land it in 1.15.x, but not cherry-pick to 1.14.x straight away, and it can have some time in the development branch. Please be on the lookout for any regressions.

[dbnicholson]

Thanks. I did test this yesterday in our version and it fixed both #5172 and #5173 in both flatpak and gnome-software. I'm pretty confident this is the right thing to do.