Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

[WIP] extensions: add cleanup extension #2941

Closed
wants to merge 1 commit into from
Closed

[WIP] extensions: add cleanup extension #2941

wants to merge 1 commit into from

Conversation

merlijn-sebrechts
Copy link
Contributor

@merlijn-sebrechts merlijn-sebrechts commented Feb 14, 2020

This PR is a proof of concept to start a discussion.

I'd like to add a "cleanup" extensions which removes all the libraries from the prime folder which are already provided by the connected content snaps and the base snap. This turns out to be very useful for reducing the snap size. As an example; the SDLPoP snap size goes down from 31MB to 1 MB and the mc-installer snap size goes from 116MB to 40MB. This works best as an extension because extensions themselves might add content snaps and parts, so the person creating the snap doesn't know yet which content snaps need to be scanned and which parts the cleanup part needs to depend on in order to run last.

However, this means

  1. the extension needs to run last, because other extensions might add parts and content snaps.
  2. the extension needs to have a way to access to the expanded snapcraft.yaml instead of the original to see the changes of other extensions.

I'm not entirely sure how to do this.

  1. I solved 1 in the apply_extensions function by supplying extensions with the already-modified yaml_data instead of original_yaml_data. However, this way all extensions will use the already-expanded data.
  2. I solved 2 by prefixing the extension name with z. Either I could implement a system where an extension can specify its order using a number, or implementing a "two-stage" expansion process and a way for an extension to specify it should run in the second stage.

So do you think this is a good candidate for an extension? If so, how do I solve 1 and 2?

Note: I'm thinking of improving the removal logic but I first want to make sure I can actually turn it into an extension. The new removal logic could be that when a library is already available in the base snap, it only removes that library when the version in the snap itself is identical to the one in the base snap. This way, a snap can still override libraries in the base snap and still use this extension.

Which snaps would use this?

This extension would be used by a regular snap to remove any libraries from the snap which already exist in the base and content snaps it will use. It's basically an extension which adds this post-build cleanup part. The mc-installer snap, for example, would use this extension to remove any libraries which are already present in the gnome-2-28 and the core18 snaps.

Why is this cleanup extension important?

  • It's a really easy way to reduce your snap size. The mc-installer snap goes from 104MB to 39MB using this extension.
  • It GREATLY improves startup time. The mc-installer snap goes from a cold start of 5 seconds to 1 second using this extension.

Startup time and snap size are very dependent on each other because of the decompression time and the resolving of dynamically linked libraries. If you have less libraries in your snap, there is less to decompress and less to keep the dynamic linker busy.

  • Have you followed the guidelines for contributing?
  • Have you signed the CLA?
  • Have you successfully run ./runtests.sh static?
  • Have you successfully run ./runtests.sh tests/unit?

This extension adds the `cleanup` part which runs last and checks the prime directory for libraries that are already included in the base snap and any connected content snaps.
@hellsworth
Copy link

Wouldn't it make more sense to just remove the unnecessary bits from the content snaps directly, like the gnome-3-3-4-1804 snap?

@merlijn-sebrechts
Copy link
Contributor Author

merlijn-sebrechts commented Feb 17, 2020

@hellsworth I'm not entirely sure I understand what you're proposing.

This extension would be used by a regular snap to remove any libraries from the snap which already exist in the base and content snaps it will use. It's basically an extension which adds this post-build cleanup part. The mc-installer snap, for example, would use this extension to remove any libraries which are already present in the gnome-2-28 and the core18 snaps.

With the snap you linked, it seems like the gnome-sdk part stages a bunch of libraries and the debs part makes sure it doesn't overwrite/duplicate any of those libraries? The cleanup extension would do something similar but across snaps instead of across parts.

@hellsworth
Copy link

@galgalesh ok I see. I mistakenly thought you were trying to remove unnecessary libraries provided by the platform snap and I was trying to point out that that should be done in the platform snap itself (we try to do that now).

@cjp256
Copy link
Contributor

cjp256 commented Mar 11, 2020

After discussions last week with @sergiusens and @niemeyer, they were thinking about how to make this part of the core snapcraft experience, rather than requiring an extension. There were no thoughts on designs shared, so I can't elaborate on that, just wanted to give an update!

This is a very impactful feature, and I've given it a test myself on a sizeable snap (~660MB cut to 330MB uncompressed) :D

Are there any known issues with this approach?

@cjp256
Copy link
Contributor

cjp256 commented Apr 26, 2020

An update on related work. For core20, we've begun to address this by skipping stage-packages dependencies that are present in the core20 base snap, unless explicitly listed in stage-packages. If we can extend this to content snaps, I think it should address the root cause of having to do the cleanup to begin with. It might not be something we can enable by default for core20 at this point, but perhaps we can add a toggle.

One challenge area with the cleanup would be python apps, and ensuring the snap environment can handle the pivot from a staged-python to the core python.

#3092

@merlijn-sebrechts
Copy link
Contributor Author

merlijn-sebrechts commented Apr 27, 2020

@cjp256 That's a great start!

For platform snaps, however, many binaries do not come from the archive but are built for that specific platform snap. I'm thinking of GTK and Qt libraries for example. We still want to remove the archive-counterparts of these binaries, because they are often older and, in the case of Qt/kde-neon, incompatible. The archive-counterparts are often pulled in by other dependencies. As an example, libswt-gtk is required for some Java applications with GTK integration, but this pulls in the entire GTK stack from the archive even though the GTK stack is already available in the gnome platform snap.

What would you recommend for this? Should platform snaps carry a manifest of which Ubuntu packages they "supersede"?

@merlijn-sebrechts
Copy link
Contributor Author

merlijn-sebrechts commented Apr 27, 2020

Also, about the known issues; I've been using this approach for every snap I make/maintain and I haven't encountered any issue.

@diddledan talked about some issues they encountered here: https://forum.snapcraft.io/t/please-review-this-pr-to-add-cleanup-extension/15666

However, I have not been able to reproduce these issues anymore. I suspect most of them are already fixed in snapcraft and the desktop helpers.

@sergiusens
Copy link
Collaborator

I am really keen on seeing this implemented not as an extension but maybe a per-snap dynamic fileset so that you can then do something like:

prime:
  - -$snap_gnome-3-28-1804

@merlijn-sebrechts
Copy link
Contributor Author

merlijn-sebrechts commented May 12, 2020

@sergiusens are dynamic filesets already a thing or would this be a new feature of snapcraft?

Where would that fileset come from? Would it be part of the content snaps or generated at build time?

@sergiusens
Copy link
Collaborator

sergiusens commented May 13, 2020 via email

@sergiusens
Copy link
Collaborator

I have added this as a backlog task, something more core to Snapcraft itself.

@sergiusens sergiusens closed this Mar 31, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants