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 flatpak support #89

Merged
merged 22 commits into from
Apr 23, 2023

Conversation

vemonet
Copy link
Collaborator

@vemonet vemonet commented Apr 1, 2023

Hi @tom-james-watson, thanks a lot for this wonderful emoji picker!

I have never worked with flatpak builder and most of the linux packaging environment, but I thought I'll give it a try since apparently Ubuntu is stopping development on snap, and it's the last application I have on snap (like many people in #11 apparently!)

And I must say I agree with you on the lack of documentation for packaging python based apps with meson/flatpak! It requires a lot of manually defined files, and installing dependencies can be a real hassle compared to pretty much every other package manager (there is not really a concept of standardized packages you can just import)

⚒️ Tooling used

I used the meson build system, which required to add meson.build files in the directories we want to build. I took inspiration from this other emoji picker, which has a relatively similar structure: built on python and GObject (ironically the author of this emoji picker mentions they took inspiration and code from Emote!)

I used the gnome SDK because it has more required dependencies pre-installed than freedesktop (I hope it does not means it only works on GNOME!), and I needed to install 2 additional modules: keybinder-3.0 and xdotool (I used the latest release available on their respective GitHub repo)

I added a emote.in python executable (the flatpak entrypoint), and 2 config variables for the flatpak path, similar to the one already used for snap, and added the necessary changes to use the right path when loading assets, e.g.:

if config.is_snap:
    manimpango.register_font(f"{config.snap_root}/static/NotoColorEmoji.ttf")
elif config.is_flatpak:
    manimpango.register_font(f"{config.flatpak_root}/static/NotoColorEmoji.ttf")
else:
    manimpango.register_font("static/NotoColorEmoji.ttf")

🚂 Does it work as expected?

It seems to work as expected overall:

  • The desktop entry, icon, and metainfo.xml (for applications hubs) are properly setup and exported

  • The integrated bindings Ctrl + Alt + E with keybinder works, and display the popup really quick as mentioned in Opening the emoji picker window takes ~2 seconds #54

  • Emote is able to automatically paste the emoji in the field selected on screen using xdotool on X11 (installed in the flatpak container)

  • I feel like the startup time is a bit faster and more consistent than with the snap version, especially when using a custom keybinding instead of the built-in one, usually under 1s

Note that currently I can run it with flatpak run com.tomjwatson.Emote or from the Desktop entry, but it does not work directly with emote from the terminal (not sure if this is the expected behavior)

🚀 Publishing it

If we want to publish it, once this PR has been merged to the main repo, the best would be to use flathub: https://github.com/flathub/flathub/wiki/App-Submission. We can put the python3-requirements.json and flathub manifest YAML in the folder on flathub (pointing to your git repository for the main module)

Let me know if you want me to do it @tom-james-watson, I think once everything is setup all you need to do to build and publish to flathub is to run:

flatpak-builder --repo=flathub --force-clean build flatpak/com.tomjwatson.Emote.yml` 
# or
make flathub

I think it will automatically update the commit pointing to your repo in the flathub repo

💬 Feedbacks welcome

It was tested on Fedora 37 with GNOME 43.3 (X11 Mutter). And built with flatpak-builder 1.2.3

It would be nice if someone could try it in other environments to make sure it works properly (e.g. KDE, Mint).

I added a few commands in the Makefile and provided instructions in the README.md to build and test the flatpak package. To build and try the flatpak package you'll just need to run 2 commands after cloning the fork:

git clone --branch=add-flatpak https://github.com/vemonet/Emote
cd Emote
# install build dependencies with flatpak:
make flatpak-install
# build and run emote locally, should take less than a minute:
make flatpak

You can now use Emote from the desktop entry or usual shortcut

I tried to build the snap to see if I broke anything on this side, but it takes forever, and snapcraft seems to have issues with starting a VM: An error occurred with the instance when trying to launch with 'multipass': returned exit code 2.. I'll let you check!

@vemonet
Copy link
Collaborator Author

vemonet commented Apr 2, 2023

@tom-james-watson I updated the list of emojis:

  • used the latest CSV available at openmoji.org (adding recent emojis like 🫠 🫴), and added the command make update-emojis to easily update the CSV file to the latest version (requires wget installed)
  • on flatpak I disabled a list of blocked emojis that were not working on snap apparently (e.g. 👨‍🦲🧑‍🦰🏳️‍⚧️)

They seems to be all working in the flatpak package, adding a few dozen or more emojis!

Note that it seems to overcome the issue #48 related to upgrading snap to core20

Also I checked and Emote is properly showing in the GNOME software GUI when installed, with reviews automatically retrieved (this is due to the metainfo.xml being exported and validated, so it should shows up properly in every software center). And apparently it's the best noted emoji picker with 4.4 for 35 reviews :)

Screenshot

Fyi: if we want to reduce the size of the application, 9.5M of the 11M are due to static/NotoColorEmoji.ttf

Copy link
Owner

@tom-james-watson tom-james-watson left a comment

Choose a reason for hiding this comment

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

Hello! Thanks for this - this looks really great and I definitely think we should be able to merge this. 💪

apparently Ubuntu is stopping development on snap

by the way this is an April Fool's article 😆. But yes, I'm also in the camp of wanting to migrate off of snap and go all-in on Flatpak too!

install build dependencies with flatpak:
make flatpak-install
build and run emote locally, should take less than a minute:
make flatpak

I did this but make flatpak doesn't seem to do anything. I think we need to change the entries that don't actually have file dependencies to look like this:

.PHONY: flatpak
flatpak:
	flatpak-builder --user --install --force-clean build flatpak/com.tomjwatson.Emote.yml
	flatpak run com.tomjwatson.Emote

This is a problem with the Makefile in general and is my fault to be honest. Probably worth just adding these to all the entries though. That way you should never need the -B flag.

Some problems that I'm seeing with the flatpak build at the moment:

When I close and relaunch the app, it's as if nothing is persisted. I see the guide (which should only show on first run) and my emoji history is gone:

image

The flatpak build also doesn't seem to be following my system dark mode preference. The above screenshot shows how it appears on my machine, but I am currently using dark mode and the snap version looks like this:

image

This is on Ubuntu 22.04.

I've tried to build the snap but it fails to run. I get a whole bunch of errors on launch and it just closes.

The full error:

~/P/emote > emote
/bin/bash: warning: setlocale: LC_ALL: cannot change locale (en_GB.UTF-8)
Fontconfig warning: "/etc/fonts/conf.d/10-hinting-slight.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/10-hinting-slight.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/10-hinting-slight.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/10-hinting-slight.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/10-hinting-slight.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/10-hinting-slight.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/10-hinting-slight.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/10-scale-bitmap-fonts.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/11-lcdfilter-default.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/11-lcdfilter-default.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/11-lcdfilter-default.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/11-lcdfilter-default.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/11-lcdfilter-default.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/11-lcdfilter-default.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/11-lcdfilter-default.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/20-unhint-small-vera.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/20-unhint-small-vera.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/20-unhint-small-vera.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/20-unhint-small-vera.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/20-unhint-small-vera.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/20-unhint-small-vera.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/20-unhint-small-vera.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/30-metric-aliases.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/30-metric-aliases.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/30-metric-aliases.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/30-metric-aliases.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/30-metric-aliases.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/30-metric-aliases.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/30-metric-aliases.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/40-nonlatin.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/40-nonlatin.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/40-nonlatin.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/40-nonlatin.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/40-nonlatin.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/40-nonlatin.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/40-nonlatin.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/45-generic.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/45-generic.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/45-generic.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/45-generic.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/45-generic.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/45-generic.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/45-generic.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/45-latin.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/45-latin.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/45-latin.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/45-latin.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/45-latin.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/45-latin.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/45-latin.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/49-sansserif.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/49-sansserif.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/49-sansserif.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/49-sansserif.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/49-sansserif.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/49-sansserif.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/49-sansserif.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/50-user.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/50-user.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/50-user.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/50-user.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/50-user.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/50-user.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/50-user.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/51-local.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/51-local.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/51-local.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/51-local.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/51-local.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/51-local.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/51-local.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/60-generic.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/60-generic.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/60-generic.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/60-generic.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/60-generic.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/60-generic.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/60-generic.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/60-latin.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/60-latin.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/60-latin.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/60-latin.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/60-latin.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/60-latin.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/60-latin.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/65-fonts-persian.conf", line 34: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/65-fonts-persian.conf", line 35: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/65-fonts-persian.conf", line 35: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/65-fonts-persian.conf", line 35: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/65-fonts-persian.conf", line 36: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/65-fonts-persian.conf", line 36: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/65-nonlatin.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/65-nonlatin.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/65-nonlatin.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/65-nonlatin.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/65-nonlatin.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/65-nonlatin.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/65-nonlatin.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/69-unifont.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/69-unifont.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/69-unifont.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/69-unifont.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/69-unifont.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/69-unifont.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/70-no-bitmaps.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/70-no-bitmaps.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/70-no-bitmaps.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/70-no-bitmaps.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/70-no-bitmaps.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/70-no-bitmaps.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/70-no-bitmaps.conf", line 8: unknown element "description"
Fontconfig warning: "/etc/fonts/conf.d/80-delicious.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/80-delicious.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/80-delicious.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/80-delicious.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/80-delicious.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/80-delicious.conf", line 6: invalid attribute 'version'
Fontconfig warning: "/etc/fonts/conf.d/90-synthetic.conf", line 4: unknown element "its:rules"
Fontconfig warning: "/etc/fonts/conf.d/90-synthetic.conf", line 5: unknown element "its:translateRule"
Fontconfig error: "/etc/fonts/conf.d/90-synthetic.conf", line 5: invalid attribute 'translate'
Fontconfig error: "/etc/fonts/conf.d/90-synthetic.conf", line 5: invalid attribute 'selector'
Fontconfig error: "/etc/fonts/conf.d/90-synthetic.conf", line 6: invalid attribute 'xmlns:its'
Fontconfig error: "/etc/fonts/conf.d/90-synthetic.conf", line 6: invalid attribute 'version'

(emote:35677): Gtk-WARNING **: 21:26:19.023: Theme parsing error: gtk.css:1413:23: 'font-feature-settings' is not a valid property name

(emote:35677): Gtk-WARNING **: 21:26:19.026: Theme parsing error: gtk.css:3286:25: 'font-feature-settings' is not a valid property name

(emote:35677): Gtk-WARNING **: 21:26:19.027: Theme parsing error: gtk.css:3748:23: 'font-feature-settings' is not a valid property name

Honestly though, I'm actually happy to just completely ditch snap. I am happy to rip out snap-related code and make a version on another branch that informs people that it is deprecated and that they should switch to the flatpak version. So if you want, you can just not worry about snap. I will remove the associated snap code as a follow up PR.

I'll check Wayland in a minute, just going to submit these comments before switching.

Makefile Outdated Show resolved Hide resolved
emote/emojis.py Show resolved Hide resolved
Comment on lines +644 to +645
if config.is_wayland:
os.system(f'wl-copy "{content}"')
Copy link
Owner

Choose a reason for hiding this comment

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

How come this is necessary? Copying on wayland worked fine before as far as I understand

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Good question!

As I understand it it should be done by Gtk.Clipboard, but it was just not doing it with wayland for me, and no logs. Maybe there are some additional GTK dependencies that are missing from the flatpak for wayland?

You mentioned wanting to potentially migrate to GTK4, we could take a deeper look at it at this moment

flatpak/com.tomjwatson.Emote.metainfo.xml Outdated Show resolved Hide resolved
@tom-james-watson
Copy link
Owner

Fyi: if we want to reduce the size of the application, 9.5M of the 11M are due to static/NotoColorEmoji.ttf

I am fine with removing this. It's been a long time since I wrote this but I assume it was because otherwise you'd sometimes not see the right emojis and it was probably to do with snap-related things. If the flatpak works fine without that then yes that would be great.

if it's easier to just rip out snap-related code as part of this PR and you're happy to do that, that is also fine. It shouldn't be too hard to be honest.

@tom-james-watson
Copy link
Owner

Ah also another reason that we should just completely drop snap is that these new emojis you've added (which is great!) will almost definitely render with tonnes of problems due to the old core version the snap is using that I was unable to upgrade from.

@tom-james-watson
Copy link
Owner

Another thing that we really would want to get working in the flatpak version is autostarting the app on login.

@tom-james-watson
Copy link
Owner

Looks good on Wayland! I have the same issues as pointed out above that I have on X11 too though.

@tom-james-watson
Copy link
Owner

I'll leave you with that for now. I am pretty busy at the moment and honestly a bit low on motivation to work on open source stuff, but this is a really great PR and I'm happy to help get this across the line, so let me know if you have any questions or anything.

It will be exciting to get this working, especially as it should unblock some other work that was blocked by snap before. Maybe we can finally upgrade to gtk 4!

@tom-james-watson
Copy link
Owner

Ah also in terms of publishing, I am happy for you to set up the flathub repository, as long as I can also be added as an admin of that repository, which as far as I understand should be fine?

vemonet and others added 3 commits April 2, 2023 22:46
Co-authored-by: Tom Watson <tom@tomjwatson.com>
Co-authored-by: Tom Watson <tom@tomjwatson.com>
@vemonet
Copy link
Collaborator Author

vemonet commented Apr 2, 2023

by the way this is an April Fool's article laughing. But yes, I'm also in the camp of wanting to migrate off of snap and go all-in on Flatpak too!

Haha, I forgot which day it was and I already started working on the flatpak packaging when I saw it, so I went for the confirmation :)

Yes, on the long run if you don't want to spend too much time maintaining it, picking 1 of the 2 would be better. Flatpak dependencies bundling is a bit tricky, and the docs for python clearly lacking, but everything else seems quite good (open source, decentralized with a reliable central repo, technically well executed, built on emerging containerization standards, well integrated in most linux environments)

  • I did this but make flatpak doesn't seem to do anything. I think we need to change the entries that don't actually have file dependencies to look like this: .PHONY: flatpak

Thanks! I just fixed it

  • When I close and relaunch the app, it's as if nothing is persisted. I see the guide (which should only show on first run) and my emoji history is gone

I forgot to also update the user data path, I just did it and pushed to my fork, history now works (stored in ~/.var/app/com.tomjwatson.Emote/data/user_data)

  • The flatpak build also doesn't seem to be following my system dark mode preference.

Weird, on my machine (Fedora 37 Gnome 43) it has always properly picked up the dark mode (I did not touch anything related to it). Do you have any logs that could help debug this?

  • Another thing that we really would want to get working in the flatpak version is autostarting the app on login.

I thought it was properly autostarting since the built-in keybinding is displaying faster than using a custom keybinding (which starts a new daemon). With snap the autostart was defined directly in the snapcraft.yml? the autostart: emote.desktop?

There seems to be some autostart capabilities in gnome, not sure if there are solutions built in flatpak

Adding this line to the desktop file would be a start on GNOME: X-GNOME-Autostart-enabled=true, but I think it would still require the user to put the desktop file in ~/.config/autostart

Fyi: if we want to reduce the size of the application, 9.5M of the 11M are due to static/NotoColorEmoji.ttf

It's not a necessity, but could be nice to check if it is still necessary when everything else have been fixed

Ah also in terms of publishing, I am happy for you to set up the flathub repository, as long as I can also be added as an admin of that repository, which as far as I understand should be fine?

I think publishing will be linked to your git repo, so the admin of the githup repo will be the admin of the package, and you might need to do something on your website https://tomjwatson.com to get a nice verified badge, I will tag you in the PR to flathub so you can follow the process

Note that I also quickly tried to enable auto-paste on wayland with ydotool (seems to be the most popular for this purpose). I needed to make some fixes to get it to compile (it was failing due to manpage generation...), but getting error failed to open uinput device: No such file or directory (seems like permissions related, could maybe fixed by playing with flatpak permissions)

I'll check how much will be required to move to GTK-4, it might be a good solution to migrate before trying to solve all the small issues, like the dark mode and copy of wayland

@tom-james-watson
Copy link
Owner

Ok, if you want to look at gtk4 feel free, though it probably makes more sense to do that after this is merged, unless doing that fixes any of these issues, as you suggest.

I've looked at ydotool before and yeh you need access to /dev/uinput which normally requires sudo. If you could find some way of getting it working with flatpak, that would be incredible.

I'll take a closer look at dark mode tomorrow. I think it's a ubuntu-specific thing to do with yaru themes.

@vemonet
Copy link
Collaborator Author

vemonet commented Apr 3, 2023

For GTK4, it's only if the migration does not need too much work, so that we don't need to fix all the small interoperability issues twice. I have started to take a look into it, and from my first test their will be a lot to changes needed to move to GTK4 😅. It's quite discouraging, they removed a lot of convenient APIs without giving clear instructions on how to replace them, everything seems to be made harder for developers in GTK4, and we don't seems to gain much from the migration, apart at least some better performance hopefully... So as it stands we will be just fix the issue on GTK3 and consider the migration later :) I'll share the few progress I did in an issue later

For ydotool, yes the requirements it asks are quite crazy. And it also take the road of making their tool less usable, as the latest version now requires to run a daemon to use ydotool... It is not ideal to add one more daemon running just for such a small functionality... Anyway we can try to see if we can make it work by changing the flatpak permissions (maybe a --device=all could help)

Otherwise there seems to be a new potential alternative written in go that claims to works everywhere: https://git.sr.ht/~geb/dotool the snippet to build the module should look like below, I just need to find out how to enable internet access for the build (required to ddl go dependencies)

  - name: dotool
    buildsystem: simple
    build-options:
      env:
        GOBIN: /app/bin/
    build-args:
      - --share=network
    build-commands:
      - . /usr/lib/sdk/golang/enable.sh; ./install.sh
    sources:
      - type: archive
        url: https://git.sr.ht/~geb/dotool/archive/1.2.tar.gz
        sha256: 80dcbc1bc4877bbef3eb30cb8c0ad7db161fb76d4999bb9b5f4a484e2267e5a1

Summary

So, to sum up what is needed to do to have a publishable version:


  • System dark mode preference not respected on Ubuntu 22.04

@tom-james-watson will check what is going wrong on his machine

There seems to be a few number of issues and discussions related to dark theme config in flatpak GTK apps:

I don't think it is currently possible to change the theme directly in Emote no?

A potential long term alternative to enable users to workaround future breaking changes in the theming system, would be to add a menu button to let users switch between light and dark mode. Ideally Emote should automatically use the system theme, but if the user wants to switch manually they should be able to do it, and we can store it in the userdata. And it's nice to give more liberty to the user (I use mainly the dark mode, but I like light better for some apps, like maps for example)

So worst case if getting dark mode from the system fails, the user just need to change it manually once


  • Autostart the app on login

There is this issue discussing how to enable autostart for flatpak: flatpak/flatpak#118

The documentation provided is a bit weird, it seems like the maintainer refuses to take 1 min to write a human readable answer, and prefer to just send a URL to a random XML file that people need to figure how to integrate into their app. And the proposed "solution" seems to consist in putting flatpak run in the Startup Applications program... It's a bit concerning that this is the support provided by flatpak/gnome for defining such a simple behavior as "my app should autostart on user login", apparently developers should have the less options possible when building apps!

A potential solution I can think of would be to try to mount the desktop entry file from flatpak in ~/.config/autostart/emote.desktop, not sure if it is possible though, and it might add some permissions warning on the flathub app page

On my side I feel like the starting time with the built-in shortcut on X11 is pretty much instant, so I am not sure if we really need to look into it (but I would need to debug a bit more, maybe autostart is activated automatically on my machine from previous experiments)


  • GTK clipboard copy does not seems to work on Wayland

Copying to the clipboard using GTK APIs was working on wayland in the snap. But it was not working on wayland in flatpak. There was not issue, not sure why, and there was absolutely no logs or error displayed

So I currently added the wl-clipboard to copy on wayland, which works perfectly, but ideally we should use the GTK APIs on wayland too if possible


  • Remove snap related code?

It seems like building the snap does not work anymore, and there are many issues to migrate to core20 (cf related github issue). For easier maintenance one ubiquitous packaging system would be better. And if flatpaks seems to be harder to setup from scratch, they also seems more stable and easy to maintain. I think the main maintenance tasks will be to upgrade the Gnome SDK when new versions are released

Should we already remove all snap related code for this release? @tom-james-watson


Let me know if I forgot anything

@ad-on-is

This comment was marked as off-topic.

@tom-james-watson
Copy link
Owner

tom-james-watson commented Apr 6, 2023

Has been a busy week - just found some time to respond properly!

Can confirm the user data persistence is fixed - nice one!

I worked out the issue with dark mode - it was to do with having selected one of the non-standard style variants in Ubuntu, which has a different underlying gtk theme. If I put it back to default it works fine.

I don't think it is currently possible to change the theme directly in Emote no?

Yes, it actually is possible, but only when running as the snap.

image

It's under "Preferences" in the menu, but that's hidden when not a snap. See

Emote/emote/picker.py

Lines 95 to 99 in f71e41b

if config.is_snap or config.is_dev:
prefs_btn = Gtk.ModelButton("Preferences")
prefs_btn.set_alignment(0, 0.5)
prefs_btn.connect("clicked", lambda prefs_btn: self.open_preferences())
items_box.pack_start(prefs_btn, False, True, 0)
. I just tried enabling that block for flatpak and it doesn't seem to work. I assume you'd need to include all of the individual theme flatpaks that you want to support. I think we can just remove this - we should just use the user's theme I think. I've ticked the "System dark mode preference not respected on Ubuntu 22.04" box, because I don't think there's anything to do here in the end.

In terms of autostart, this isn't ideal, but if there's no way to do this built into flatpak, we can just tell users to add a startup applications entry with the command set to flatpak run com.tomjwatson.Emote. That works fine. We can add it to the wiki like https://github.com/tom-james-watson/Emote/wiki/Hotkey-In-Wayland.

I think publishing will be linked to your git repo, so the admin of the githup repo will be the admin of the package, and you might need to do something on your website https://tomjwatson.com/ to get a nice verified badge, I will tag you in the PR to flathub so you can follow the process

I am fine with you doing the PR if that's the case. And I'm happy to add whatever is necessary to get the flatpak verified, too.

FYI for gtk4, I know that libadwaita is pretty much the core toolkit now, but that may not actually show up in the gtk4 docs. I've not really looked at gtk since I made this, so I'm not super familiar with it. See https://gnome.pages.gitlab.gnome.org/libadwaita/doc/1.3/widget-gallery.html. But yes, this would definitely be trying to do too much at once, so let's look at this later.

Otherwise there seems to be a new potential alternative written in go that claims to works everywhere: https://git.sr.ht/~geb/dotool

dotool I think has the same problem as ydotool - it requires permission to access /dev/uinput, which requires sudo. That means you'd need a daemon that runs as root. That may be possible to do with a flatpak, I'm honestly not sure. But as that would be new functionality, I guess it's best to just focus on just the migration to flatpak for now and that can be worked on separately afterwards if there's anyone interested in doing it.

If we can't get copying working via the gtk lib - it's fine to leave the wl-copy in.

Should we already remove all snap related code for this release?

Yes, let's just go all in on flatpak. I will mark the snap package as deprecated once we are happy with the flatpak, and will push an update that also mentions it in-app somewhere.

Thanks for your hard work on this 🙏

@vemonet
Copy link
Collaborator Author

vemonet commented Apr 10, 2023

On my side I could not resist to see what it would take to migrate to GTK4! And I found out that https://github.com/mijorus/smile already uses GTK4, it was a good source of inspiration to quickly make the changes required to get a proof of concept running. Basic features are working (search, categories, auto-paste, copy works with GTK4 on wayland, no need for wl-clipboard!), but it still need a few hours of work to fix all the events handlers and UI margins/sizes. As we said it is too much work for the 1st flatpak release, so I created a separate issue to discuss it further: #90

It's under "Preferences" in the menu, but that's hidden when not a snap. I think we can just remove this - we should just use the user's theme I think

Ok perfect, for now I guess using the user's theme is good enough, but being able to switch the theme manually between "Always light", "Always dark", and "Use system preference" is quite a nice to have, and also would avoid to get too many issues related to Emote not picking up the user's theme

We can ignore it for the first flatpak release, but I would be up to check if we can reintroduce it under flatpak in the future (I'll create an issue to track it)

In terms of autostart, this isn't ideal, but if there's no way to do this built into flatpak, we can just tell users to add a startup applications entry with the command set to flatpak run com.tomjwatson.Emote. That works fine.

Look good to me, we can even put it up in the program description (that is shown on flathub), or in the Emote welcome popup that shows up at the first start to make it more accessible. And if we find a completely integrated solution in the future we can still add it

  • I will check the easiest way to enable austostart, and add the necessary documentation

dotool I think has the same problem as ydotool - it requires permission to access /dev/uinput, which requires sudo. That means you'd need a daemon that runs as root. That may be possible to do with a flatpak, I'm honestly not sure. But as that would be new functionality, I guess it's best to just focus on just the migration to flatpak for now and that can be worked on separately afterwards if there's anyone interested in doing it.

Thanks for the details! Yes, I agree we should not try to make it work for now as it was not a feature before anyway. I will create a separate issue to keep track of the various infos we already know about this implementation, to help going back to it in the future

If we can't get copying working via the gtk lib - it's fine to leave the wl-copy in.

Copying is working fine on wayland when using GTK4 API, so it might be able to work with GTK3 too, maybe there's just a little detail to fix

  • I will check again if I can make add to clipboard works on wayland with the GTK3 API, otherwise we can keep wl-clipboard

Yes, let's just go all in on flatpak. I will mark the snap package as deprecated once we are happy with the flatpak, and will push an update that also mentions it in-app somewhere.

  • Perfect, I'll remove all the code related to snap for the first flatpak release

I don't have that much free time in my hands in the coming month (I am not super hurry anyway, I have already removed snap from my laptop and uses the flatpak version full time 🙃), but I should be able to make the changes required to publish in the coming weeks and I'll create a PR to add it to flathub once the checkboxes above are resolved

Thanks a lot for your helpful support and feedbacks for this migration!

@tom-james-watson
Copy link
Owner

Fantastic - that all sounds perfect. Yeah no rush on my end either, so whenever you get the chance 🙏. Will be awesome to publish it officially on Flatpak finally.

Please also add yourself to authors!

@vemonet
Copy link
Collaborator Author

vemonet commented Apr 18, 2023

A small update on the progress:

  • I will check the easiest way to enable austostart, and add the necessary documentation

Technically we should be able to do it by copying the .desktop file in ~/.config/autostart/

But I could not find a way to do it cleanly, the only existing apps I found that implements autostart for flatpaks were either closed source (dropbox client) or using Vala to build the project

The cheapest hack at the moment would be to copy the desktop file after installation

cp -L "/var/lib/flatpak/exports/share/applications/com.tomjwatson.Emote.desktop" ~/.config/autostart/
# Or if it was installed locally:
cp -L "cp ~/.local/share/flatpak/exports/share/applications/com.tomjwatson.Emote.desktop" ~/.config/autostart/

We can probably automate this in the meson build, or the first time the app is started

  • I will check again if I can make add to clipboard works on wayland with the GTK3 API, otherwise we can keep wl-clipboard

The current approach use to get the clipboard and set the text seems to be the recommended one for Gtk 3.0

cb = Gtk.Clipboard.get(Gdk.SELECTION_CLIPBOARD)
cb.set_text(content, -1)

Yet, it still does not work for on my Fedora 37 GNOME 43, and does not give any warning or error message

So I kept wl-clipboard for now (will be removed if we migrate to GTK4)

  • Perfect, I'll remove all the code related to snap for the first flatpak release

I did not take the time to do this, I wanted to figure out autostart better first


I'll not be able to work on it in the next 3 weeks, but I'll come back to it after. If someone have the time to figure out the autostart on flatpak in the meantime that would be great 🙂

@vemonet vemonet mentioned this pull request Apr 18, 2023
9 tasks
@tom-james-watson
Copy link
Owner

Thanks for the update! Sounds like good progress. I will see if I can find time to look at autostart.

@tom-james-watson
Copy link
Owner

By the way, have you seen #88? We can look at this afterwards, but I think in theory it means that we can register global shortcuts with flatpak without the user having to manually do that via settings.

@tom-james-watson
Copy link
Owner

I have flatpak autostart working on this branch which I've opened as a PR to your branch: vemonet#1. Seems to be working fine as far as I can tell!

I've also made a PR that fixes the desktop file that flatpak ends up generating which previously wouldn't actually launch the app: vemonet#2.

When you get a chance can you double check those and merge them? 🙏

@vemonet
Copy link
Collaborator Author

vemonet commented Apr 20, 2023

Thanks a lot for checking this!

To some regards implementing auto start this way has some advantages: auto start will be enabled only if the user actually start Emote (nice if emote was installed as part of a bundle and the user doesn't really use it), and it will enable us to make auto start opt-in in the future if requested

The only issue I see is that the auto start desktop file will not be removed if the user uninstall Emote

So there's just removing the snap code left (the funniest part), I'll do it in 3 weeks :)

@tom-james-watson
Copy link
Owner

If you don't mind I think I'll actually merge this and get this over the finish line - I have a bit of time to try and get things wrapped up, I think!

@tom-james-watson tom-james-watson changed the base branch from master to flatpak April 23, 2023 10:27
@tom-james-watson tom-james-watson merged commit 9f8f94b into tom-james-watson:flatpak Apr 23, 2023
@tom-james-watson
Copy link
Owner

I've merged this into https://github.com/tom-james-watson/Emote/tree/flatpak along with my fixes too. I've also just removed all snap code: 6d510ff.

@tom-james-watson
Copy link
Owner

I've fixed the skintone selection - 8ea2bf5

Note that you'll need to do rm -rf ~/.var/app/com.tomjwatson.Emote/data/

@tom-james-watson
Copy link
Owner

tom-james-watson commented Apr 23, 2023

I've actually switched my system to Fedora 38 (as opposed to Ubuntu 22.04 which is what I was running before). On this system the app doesn't seem to respect dark mode - it is always rendered in light mode. Not really sure why at this point. It seems like this is something that is handled a lot better if we use libadwaita, which would be part of the gtk4 migration.

@tom-james-watson
Copy link
Owner

I was looking at https://github.com/flathub/flathub/wiki/App-Requirements for submitting the flatpak to flathub, and it seems like the manifest has to be at the top level of the repository. I tried extracting things out of the flatpak folder but I'm hitting some issues and don't know how to proceed.

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.

3 participants