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

Add snap support. #314

Merged
merged 18 commits into from
Feb 12, 2023
Merged

Add snap support. #314

merged 18 commits into from
Feb 12, 2023

Conversation

dingdang66686
Copy link
Contributor

No description provided.

@mjakeman
Copy link
Owner

Hi, thanks for working on this!

At a high level it all looks good to me. I'll have to test this in some detail but I'm happy to keep the snap build files in-tree as an 'official' port.

One comment: could you please rename the app from gnome-extension-manager to simply extension-manager? We're not a GNOME core app so using according to the App Naming Guidelines we should avoid using 'GNOME' in the name.

snap/snapcraft.yaml Outdated Show resolved Hide resolved
Rename to extension-manager.
Revert cleanup because libgtk4 in gnome-42-2204 is too old.
Add network plug and change install prefix to /usr.
@dingdang66686
Copy link
Contributor Author

This configuration is now usable.

Switch to devmode to disable apparmor. Can't find a plug to support extension managing. (Dbus message is blocked by apparmor)
@mjakeman
Copy link
Owner

Hi @dingdang66686, sorry for the delay in testing.

It seems snapcraft is completely broken on the arm64 architecture, at least on the two distributions I've tried it on, so I'm unable to test the snap at the moment. I'll try dig up an x86 laptop to test it with soon.

In regards to snap support, I notice you've got a package here: https://snapcraft.io/gnome-extension-manager. Did you want to continue maintaining this unofficially, or transfer ownership and have it maintained upstream (as part of this PR)?

@dingdang66686
Copy link
Contributor Author

Well, I create this package just for testing. If there is no problem, I'm glad to make this maintained upstream.

@dingdang66686
Copy link
Contributor Author

I have to mention that this application doesn't work in sandboxed mode. When apparmor is enabled, the dbus message is blocked. I have checked the official documentation but without luck.

@dingdang66686
Copy link
Contributor Author

dingdang66686 commented Dec 12, 2022

It seems snapcraft is completely broken on the arm64 architecture

It takes several minutes on first build, because it will create a vm (if the snap uses core22 as a base, a lxd container by default) and pull all the dependencies. If it is extremely slow, you should check your internet connection to ubuntu repositories and the snap repository.

@dingdang66686
Copy link
Contributor Author

https://1drv.ms/u/s!AjQ7y8jf_bBFmIRY_zOSIdzXAMD-6w?e=CfKWrG @mjakeman Test this snap on your arm64 device.

@mjakeman
Copy link
Owner

Hi @dingdang66686,

I've finally had the chance to review and test this. I've fixed the issues I was having with arm snaps by using an Ubuntu VM image. On the whole, the package looks good.

There are a few points:

  • Could you add 'arm64' to the supported architectures
  • The snap is currently in devmode. There's nothing preventing extension-manager from working within a sandbox; I believe it should be possible to use the dbus interface to make it confined again?

@mjakeman mjakeman self-requested a review January 22, 2023 06:11
Copy link
Owner

@mjakeman mjakeman left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See above

@mjakeman
Copy link
Owner

Unfortunately, AppArmor prevents the app from using dbus, meaning that it can't install anything now.

image

There isn't anything stopping the app from working with confinement (by design), so I'd ask that the snap file declares the proper permissions so we can use strict confinement. Maybe this will help: https://forum.snapcraft.io/t/the-dbus-interface/2038

@dingdang66686
Copy link
Contributor Author

As the document says, the dbus interface is only for communications between snaps. I don't think this will help.

@mjakeman
Copy link
Owner

mjakeman commented Feb 12, 2023

As the document says, the dbus interface is only for communications between snaps. I don't think this will help.

Yep, you're absolutely right. I misread that, apologies.

I've spent a lot of time looking at snap interfaces and how we can work around this for confinement purposes. Unfortunately, if there is a solution it does not look trivial (which is a bit odd, given that DBus is the primary means of achieving confinement under flatpak).

Thanks for your patience with this PR - I'm happy to take this as-is in time for the 0.4.1 release, and we can look at tightening things up later on.

Much appreciated!

@mjakeman mjakeman merged commit 7bd1595 into mjakeman:master Feb 12, 2023
@mjakeman
Copy link
Owner

Hi @dingdang66686, I'm looking at making a new release of Extension Manager in the next week or so. I'm happy to take ownership of the snap package and handle releases if you're agreeable?

@dingdang66686
Copy link
Contributor Author

I'm glad to do so. 😁

@dingdang66686
Copy link
Contributor Author

Maybe we should ask snap developers to add a new slot/plug for gnome extension managing.

@mjakeman
Copy link
Owner

Great! I've opened a request: https://forum.snapcraft.io/t/transfer-ownership-of-extension-manager-and-rename-package/33870?u=mjakeman

Maybe we should ask snap developers to add a new slot/plug for gnome extension managing.

I think that's a good idea. Let's publish v0.4.1 using classic confinement so we have a working version out in the wild. We can ask for either a session-dbus interface, or maybe some kind of gnome-specific plug that whitelists org.gnome.Shell.Extensions?

@dingdang66686
Copy link
Contributor Author

You can register a different snap named extension-manager. Then I will close my snap.

@mjakeman mjakeman mentioned this pull request Jul 15, 2023
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.

None yet

3 participants