flatpak-builder --finish-only does not cleanup files

[alexlarsson]

From @dbnicholson on January 10, 2017 15:35

When running flatpak-builder --finish-only, the cleanup command is run, but none of the files in the cleanup list are removed. It looks like this is because there are no changes in the BuilderCache since the build wasn't run. I looked at how to rebuild the cache without building, but I couldn't figure it out.

Without this, we're just running flatpak-builder normally so the cleanup actions are respected, doing our metadata modifications, wiping the exports directory, then running flatpak build-finish again. It would be great if we could use --build-only and --finish-only normally.

Copied from original issue: flatpak/flatpak#480

[alexlarsson]

Yeah, that is not great. However, it should be fixable. All we need to do is repeat this part of builder_manifest_build():

      changes = builder_cache_get_changes (cache, error);
      if (changes == NULL)
        return FALSE;
      builder_module_set_changes (m, changes);

For each module. Assuming the cache is still around this should work fine without having done the builds now. However, the code above doesn't work as-is, since it relies on the cache having the checksum of everything built up until now. So, we either have to do a lookup by stage name, or we have to do the checksumming even when not building.

I think we could just add a "don't actually build" flag to builder_manifest_build(), which skip the build step. The question is, what do we do if while doing this, some part of the build is not in the cache? Do we error out? Do we ignore cleanups from that?

[alexlarsson]

From @dbnicholson on January 12, 2017 15:44

Yeah, I wrote a patch like above, but then it wasn't working because the cache wasn't initialized to some stage. I couldn't figure out how to do that correctly and gave up.

That said, why does the cleanup depend on knowing the changes? Why can't we just glob all the paths using the specified patterns? I guess it's faster if you're only checking the patterns against paths that you know have changed, but I doubt it's a real bottleneck. It's certainly simpler to just glob and delete anything that matches.

[alexlarsson]

The semantic of cleaup in a particular module is that those globs only affects things from that module.
I.e. you can do cleanup /app/bin to delete all executables from this module.

[guihkx]

I have a custom CI workflow using GitHub Actions that creates a Flatpak bundle, and after I noticed that the size of the bundle was suspiciously large, I extracted it and noticed that some files that were supposed to be removed were included there. :/

I understand this a known bug, but since it's been here forever, perhaps the flatpak-builder manual should be updated to reflect the current state?

As of v1.2.3, it states:

--finish-only
Only do the cleanup, finish and export stages, picking up where a --build-only command left off.

Which is not accurate.

[TingPing]

Well, I don't necessarily like changing the intention of the argument, but also it likely won't be fixed soon. I guess I'd take a patch rewording it.