transaction: Add delete app data option by razzeee · Pull Request #6728 · flatpak/flatpak

razzeee · 2026-07-08T18:41:10Z

Story here is, that I've read https://linuxnews.de/flatsweep-aufraeumhelfer-fuer-flatpak-reste/ and wondered, why there is even a need for an app like flatsweep

Looked a bit into it and there are legimate usecases, where flatpak can't remove the user data (e.g. its a system app and gets uninstalled by an admin while the user partition is not unlocked etc)

Then looked how stores like bazaar implement it right now and it turns out, they all need to roll their own logic.
So while you can run flatpak remove my.demo.app --delete-data from the cli, there is no way to get that behavior via libflatpak.

swick · 2026-07-09T10:52:07Z

This really needs to be split up but most importantly needs a motivation (ideally in the PR description and the commits).

razzeee · 2026-07-09T14:30:09Z

Is this shape for splitting commits fine? I would then rework like this:

Move old code into common
Improve old code in some places
Allow usage from libflatpak (which was the main motivation)

swick · 2026-07-09T15:19:24Z

Sure, seems reasonable.

swick · 2026-07-10T10:03:55Z

You really need to work on your commit splitting and commit messages. For one, the formatting: 80 chars per line. The first two commits don't give me enough information. The commit message tells me exactly the same as looking at the code, but not why (mention the broader goal), and not how (improve? seems like two commits, one removing the XdpDbusPermissionStore argument, one improving ownership).

razzeee · 2026-07-10T10:39:58Z

The broader goal is only archived by the 1st and last commits in tandem. I guess I can mention that there again, but it feels weird, as it's not complete without the third commit. (and was one of the reasons this was one commit in the first place)

I can split up the 2nd one or drop it and focus on the other commits. It's not needed, just removing an unused param and modernizing the pointer logic. (but my C is still very rusty, so happy to drop that)

Edit: Filled in the description in addition

swick · 2026-07-10T10:47:45Z

The first commit is just "common: Move app data helpers out of builtins".

I can see that. Why though? It only becomes clear when you reach the third commit. Just adding "We're going to expose them in libflatpak in a few commits" to the body is enough for a reviewer to understand.

The improvements in the second commit are good. It only needs to be split appropriately and have a message which isn't just "Improve things". Say what you're improving.

I know this can be annoying but it's an important part to get right and once you get it, it becomes pretty easy.

razzeee · 2026-07-10T10:49:00Z

No, it's totally fine, I was in a rush yesterday and shouldn't have pushed.

razzeee · 2026-07-10T11:22:36Z

Should the commit header also be shorter?

swick · 2026-07-10T11:50:56Z

    install : true,
    install_dir : docdir,
  )
-endif


Seems like an extra newline snuck in here.

swick · 2026-07-10T11:53:19Z

@@ -131,8 +131,5 @@ flatpak_delete_app_data (const char *app_id,
        return FALSE;


The prefix common is not helpful because it's too generic. Use app-data:.

swick · 2026-07-10T11:59:49Z


      if (res)
        {
+          if (priv->delete_data && flatpak_decomposed_is_app (op->ref))


A few notes:

Should this really be part of a transaction or of individual ops?

If it is per-op, then I guess we should only allow this for apps when setting it

What happens when this fails? The app has been uninstalled but the data has not been removed, now there is no way to remove the data via libflatpak.

you mean it's own operation?

we do skip non apps - but this is still the transaction implementation (not sure I follow)

Yeah, I actually iterated on this over the past days. But ended up here again as I don't think we can recover. We could give a better return, but that would probably not be as backwards compatible.

you mean it's own operation?

Either that, or part of the uninstall op?

I mean, I'm not even sure if this really should be part of a transaction in the first place. Maybe it's fine to just provide a separate api to delete the data?

we do skip non apps - but this is still the transaction implementation (not sure I follow)

Right, my point is that if this were on the uninstall op, we could error out when constructing the transaction/op and it's not an app.

Having it's own operation seems simpler for us, but might not be the ideal API. Which would you prefer?

Edit: Give me a gut feeling, I can explore from there

Maybe try to answer the question what we get from it being part of a transaction? If there isn't a good reason, let's not make it part of a transaction.

My feeling is, that having a separate method to call would be better - even if it causes some implementation detail on the apps that are using it.

But it also might allow cleaning up app data, when the app is not installed at all or resetting it with keeping the app installed.

Alright, then let's do that instead.

swick

Looks almost good. The merge commit needs to go and a few things should probably change.

Btw, if you want me to re-review something, please add a comment and say something.

swick · 2026-08-17T12:11:00Z

+#include "flatpak-utils-private.h"
+
+char **
+get_permission_tables (XdpDbusPermissionStore *store)


That doesn't seem to be the right place. The app data has nothing to do with the permission tables.

Got it's own common file now

swick · 2026-08-17T12:19:11Z

 gboolean
-flatpak_delete_app_data (const char *app_id,
-                         GError    **error)
+flatpak_delete_app_data_for_file (const char   *app_id,


What's the purpose of flatpak_delete_app_data_for_file? We need an app id anyway, so why make it variable on the path?

Got rid of it, don't remember why I did this.

swick · 2026-08-17T12:22:25Z

+  if (!flatpak_is_valid_name (app_id, -1, error))
+    return FALSE;
+
+  path = g_build_filename (g_get_home_dir (), ".var", "app", app_id, NULL);


We have to be careful about paths. If any of the components is attacker controlled, a path can point to anywhere via symlinks. In this case everything is fine because ~/.var/app/$app_id is still all not attacker controlled (any path inside would be a vulnerability). Probably worth documenting.

razzeee · 2026-08-18T14:00:17Z

+ * License along with this library. If not, see <http://www.gnu.org/licenses/>.
+ *
+ * Authors:
+ *       Alexander Larsson <[email protected]>


This is inherited from app/flatpak-builtins-uninstall.c

razzeee · 2026-08-18T14:32:29Z

@swick hopefully addesses everything

swick

A few more nitpicks

swick · 2026-08-19T10:50:37Z

+
+char **  get_permission_tables             (XdpDbusPermissionStore  *store);
+gboolean flatpak_reset_permissions_for_app (const char              *app_id,
+                                             GError                 **error);


Classic claude. It often fails to properly align things.

Hopefully better now

swick · 2026-08-19T10:52:24Z

+}
+
+gboolean
+flatpak_reset_permissions_for_app (const char *app_id,


So, two things:

We have a FlatpakPermission struct already, which makes this a bit awkard

The functions here don't operate on an object but are just a collection of useful things, so it's probably better to call it something -utils in the name (or find an existing utils file which fits)

not really sure, what you expect me to do here

added -utils to the file name

swick · 2026-08-19T10:55:03Z

+
+#include <gio/gio.h>
+
+gboolean flatpak_delete_app_data           (const char              *app_id,


If the file is called flatpak-app-data, and we operate on the app data, it's better to name things accordingly: $prefix_$thing_$verb, so flatpak_app_data_delete

Should be addressed

Still flatpak_delete_app_data instead of flatpak_app_data_delete

razzeee · 2026-08-23T16:18:53Z

Rebase due to test changes

swick · 2026-08-27T16:49:44Z

+  'flatpak-app-data.c',
  'flatpak-appdata.c',


that is super confusing

Agreed, changed

swick · 2026-08-27T16:50:13Z

+
+#include <gio/gio.h>
+
+gboolean flatpak_delete_app_data           (const char              *app_id,


Still flatpak_delete_app_data instead of flatpak_app_data_delete

swick · 2026-09-21T15:25:34Z

Mh, just calling the file app-data-delete.c doesn't really help with the app-data name conflict. The header is still just app-data.h as well. A function also gets renamed flatpak_delete_app_data to flatpak_app_data_delete in one commit to the next. Not super happy with all of that.

Libflatpak needs to reuse user-data deletion and permission reset logic that was previously private to the command-line builtins. This will let software stores offer the equivalent of uninstall --delete-data. Move each concern into a separate common module without changing behavior so a public API can be added in a later commit. Name the user-data module and its helper consistently here, and keep them distinct from the existing appdata metadata code.

Return the permission reset result directly after app data removal. This keeps the success and error paths equivalent while removing an unnecessary conditional.

Use GLib's automatic string-vector cleanup for permission table lists. This makes ownership explicit and ensures early returns release the list.

Permission table discovery reads the on-disk database directory and does not use the D-Bus permission store proxy. Remove the unused argument and update callers.

Applications using libflatpak currently have to duplicate the CLI's private cleanup logic to offer the equivalent of uninstall --delete-data. Add a standalone API that removes ~/.var/app data and resets permission store entries without requiring the app to be installed. Keep it separate from transactions so callers can retry partial failures or reset data for an installed app. Validate app IDs before constructing paths, propagate cancellation and permission table errors, and document partial failure behavior.

razzeee force-pushed the transaction-delete-data branch from 01d8468 to 4764f3b Compare July 8, 2026 18:41

razzeee changed the title ~~transaction: Add delete-data option~~ transaction: Add delete app data option Jul 8, 2026

razzeee force-pushed the transaction-delete-data branch from 4764f3b to 1154be5 Compare July 8, 2026 19:06

razzeee force-pushed the transaction-delete-data branch 8 times, most recently from bbaa11e to 6c0838c Compare July 10, 2026 00:37

razzeee force-pushed the transaction-delete-data branch from 6c0838c to e721335 Compare July 10, 2026 11:20

swick reviewed Jul 10, 2026

View reviewed changes

razzeee force-pushed the transaction-delete-data branch 2 times, most recently from dd805fe to d266554 Compare August 6, 2026 10:14

swick requested changes Aug 17, 2026

View reviewed changes

razzeee force-pushed the transaction-delete-data branch from 6f8ec3e to 3849485 Compare August 18, 2026 13:51

razzeee commented Aug 18, 2026

View reviewed changes

razzeee force-pushed the transaction-delete-data branch from 3849485 to 62cb28b Compare August 18, 2026 14:15

razzeee requested a review from swick August 18, 2026 14:31

swick reviewed Aug 19, 2026

View reviewed changes

razzeee force-pushed the transaction-delete-data branch 3 times, most recently from fb5b5ef to 522f3a1 Compare August 19, 2026 23:00

razzeee requested a review from swick August 19, 2026 23:30

razzeee force-pushed the transaction-delete-data branch from 522f3a1 to cea39b5 Compare August 23, 2026 16:17

swick reviewed Aug 27, 2026

View reviewed changes

razzeee force-pushed the transaction-delete-data branch from cea39b5 to cb2d7d3 Compare August 27, 2026 19:49

razzeee added 4 commits September 23, 2026 00:28

user-data: Simplify delete result handling

b2f8fb1

Return the permission reset result directly after app data removal. This keeps the success and error paths equivalent while removing an unnecessary conditional.

permission: Automatically free permission table lists

52068cf

Use GLib's automatic string-vector cleanup for permission table lists. This makes ownership explicit and ensures early returns release the list.

permission: Remove unused permission store argument

54fa5e4

Permission table discovery reads the on-disk database directory and does not use the D-Bus permission store proxy. Remove the unused argument and update callers.

razzeee force-pushed the transaction-delete-data branch from cb2d7d3 to db3a84c Compare September 22, 2026 22:40

razzeee force-pushed the transaction-delete-data branch from db3a84c to 2c97205 Compare September 22, 2026 23:20

swick added this pull request to the merge queue Sep 23, 2026

Merged via the queue into flatpak:main with commit dec343a Sep 23, 2026
11 checks passed

		@@ -131,8 +131,5 @@ flatpak_delete_app_data (const char *app_id,
		return FALSE;


		#include <gio/gio.h>

		gboolean flatpak_delete_app_data (const char *app_id,

Conversation

razzeee commented Jul 8, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swick commented Jul 9, 2026

Uh oh!

razzeee commented Jul 9, 2026

Uh oh!

swick commented Jul 9, 2026

Uh oh!

swick commented Jul 10, 2026

Uh oh!

razzeee commented Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

swick commented Jul 10, 2026

Uh oh!

razzeee commented Jul 10, 2026

Uh oh!

razzeee commented Jul 10, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

razzeee Jul 10, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

swick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

razzeee commented Aug 18, 2026

Uh oh!

swick left a comment

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Choose a reason for hiding this comment

Uh oh!

razzeee commented Aug 23, 2026

Uh oh!

Choose a reason for hiding this comment

Uh oh!

razzeee commented Jul 8, 2026 •

edited

Loading

razzeee commented Jul 10, 2026 •

edited

Loading

razzeee Jul 10, 2026 •

edited

Loading