ludusavi icon indicating copy to clipboard operation
ludusavi copied to clipboard

Flatpak Progress

Open LeSnake04 opened this issue 2 years ago • 32 comments

I am currently working on making a flatpak for ludusavi

Open Questions

  • [x] Internet access needed ? - yes
  • [x] Sound needed ? - no
  • [x] xdg desktop portal support ? – Irrelevant
  • [x] following releases tag or master branch? - releases
  • [x] notifications? - no

TODO

Things that should be merged with main repo

When will they be able to be merged?

  • [x] desktop file
  • [x] ~~generated-sources – needs to be in repo for auto-updating flatpak, but can be provided via flathub repo without automated updates.~~Auto-Update Not realisticly possible
  • [x] metainfo

general

  • [x] desktop file

yaml

  • [x] Base structure
  • [x] add rust sdk
  • [x] ~~auto updating of release tag~~ not possible
  • [x] get cargo build to work – genrated sources needed
  • [x] move binary
  • [x] move desktop file
  • [x] move icons
  • [x] compiles
  • [x] cli works
  • [x] gui

metainfo

  • [x] base structure
  • [x] description
  • [x] OARS Rating – Internet needed ? – https://hughsie.github.io/oars/generate.html
  • [x] desktop file
  • [x] Screenshots

Folders it needs acess to

read access

  • [x] All? Should be avoided if possible – xdg desktop portal question releveant
  • [x] steam flatpak
  • [x] steam

write access

  • [x] ~/lududusavi-backup
  • [x] Everywhere

LeSnake04 avatar Apr 06 '22 17:04 LeSnake04

Note: for oars internet doesn't matter because this app isn't a social app. But i still need to know it because of permissions

LeSnake04 avatar Apr 06 '22 17:04 LeSnake04

@mtkennerly I need some feedback regarding open questions and what files you can put into the main repo.

LeSnake04 avatar Apr 06 '22 17:04 LeSnake04

About the generated sources: https://github.com/flatpak/flatpak-builder-tools/tree/master/cargo This script converts the cargo.toml into a flatpak compatible sources list. I made this for version 0.10.0 for know.

We have 2 or 3 options:

  • Manually update it on each release
  • You convert the cargo.toml on new releases
  • I figure out a way how to convert it while building the flatpak (I am not sure if its possible and it will increase the complexity and build time)

LeSnake04 avatar Apr 07 '22 16:04 LeSnake04

This sounds great! Thanks for working on this.

Internet access needed ?

  • At install time: no.
  • At runtime: yes (to check for manifest updates).

Sound needed ?

No.

xdg desktop portal support ?

I'm not familiar with this. Would it require something like ashpd, or is there a way for Flatpak to support normal file system access calls?

following releases tag or master branch?

It should definitely follow the releases. Master may not always be usable, even though it should be most of the time.

generated-sources – needs to be in repo for auto-updating flatpak, but can be provided via flathub repo without automated updates.

I'm assuming most people would want it on Flathub for standardization and discoverability, right? If the GitHub build produces the necessary files, I don't mind taking them and manually running a publish command when there's a new release.

All? Should be avoided if possible – xdg desktop portal question releveant

Technically, it should be all, since you can configure arbitrary paths as roots and in custom game locations. The absolute bare minimum would be:

  • ~
  • $XDG_DATA_HOME
  • $XDG_CONFIG_HOME

I figure out a way how to convert it while building the flatpak (I am not sure if its possible and it will increase the complexity and build time)

The ideal option would be some command that runs automatically as part of the GitHub build. The next best option would be some command that I can run manually when there's a new release.

mtkennerly avatar Apr 07 '22 18:04 mtkennerly

xdg portals means the app can request access to files and folders through the desktop file manager and as flatpaks get access to data outside the configured folders.

Once i get the GUI to work, i will see if its supports xdg portals. If it has no support its ok, most flatpaks dont support it (even major ones like firefox or steam). But if it has support already its great and makes it a bit easier. I assume for anything but the backup dir I can mark it as read only

Especially regarding external drives, which can be hassle with flatpaks.

Also I forgot to ask, does it use notifications?

LeSnake04 avatar Apr 07 '22 20:04 LeSnake04

Regarding the generated sources –its needed so flatpak knows what to download since during compiling the flatpak its cut of from the internet

That means somewhere during the update process this needs to be run: https://github.com/flatpak/flatpak-builder-tools/tree/master/cargo: python3 ./flatpak-cargo-generator.py Cargo.lock -o cargo-sources.json

The flatpak needs that json, so eighter it needs to be genrated automatically (in the flatpak repo or the main repo) or one of us needs to generate it every new release

LeSnake04 avatar Apr 07 '22 20:04 LeSnake04

I assume for anything but the backup dir I can mark it as read only

You can also restore a backup to the original paths, so everything is read/write, unfortunately 😅

Also I forgot to ask, does it use notifications?

Nope, no OS notifications.

The flatpak needs that json, so eighter it needs to be genrated automatically (in the flatpak repo or the main repo) or one of us needs to generate it every new release

It should be easy enough to generate automatically during a GitHub build. I'm fine with it happening during the Ludusavi build or another repo's build - whichever way's easier.

mtkennerly avatar Apr 08 '22 02:04 mtkennerly

Firstly, Right, I somehow totally forgot about storage access on restoring. ^^

So there is no way around giving it acess to everything. Even if it has xdg portals support, since xdg portals only give temporary access. They are mainly intended for editors like gimp or libreoffice, which only needs access for files explicitly opened and need a file chooser anyway. They might work for custom paths but not for permanent and silent access to data.

Sorry for being a bit obnoxious but the flathub team tries to minimize the folder access, especially for new apps, so I needed arguments for explaining why this one needs so broad access.

Secondly, Its just downloading a python script and running it.

I think its way easier to start the script via a github action on your side. It may be possible via the flatpak, but requires extra steps because of the way flatpaks are build

LeSnake04 avatar Apr 08 '22 06:04 LeSnake04

I am on holiday for the next days so the progress will be slow for a few days

LeSnake04 avatar Apr 08 '22 06:04 LeSnake04

The GUI crashes instantly for some reason. I assume based on the dependencies there is no logging that could help fixing it?

LeSnake04 avatar Apr 18 '22 20:04 LeSnake04

Does it print anything at all in the console? You could try running it with the RUST_BACKTRACE environment variable set to 1.

Here's a debug build with simple_logger added, which will print logs from various dependencies to the console:

ludusavi-ec73cbdc26c8c1564af30ab0b9874e8c4fb74856-logging.zip

diff --git a/Cargo.toml b/Cargo.toml
index 6d19926..cce9402 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -28,6 +28,7 @@ reqwest = { version = "0.10.6", features = ["blocking", "rustls-tls"], default-f
 serde = { version = "1.0.111", features = ["derive"] }
 serde_json = "1.0.57"
 serde_yaml = "0.8.13"
+simple_logger = "2.1.0"
 structopt = "0.3.15"
 thiserror = "1.0.20"
 walkdir = "2.3.1"
diff --git a/src/main.rs b/src/main.rs
index a3514a2..885e1ac 100644
--- a/src/main.rs
+++ b/src/main.rs
@@ -13,6 +13,7 @@ mod shortcuts;
 mod registry;
 
 fn main() {
+    simple_logger::SimpleLogger::new().init().unwrap();
     prelude::migrate_legacy_config();
 
     let args = cli::parse_cli();

mtkennerly avatar Apr 18 '22 21:04 mtkennerly

No output even with RUST_BACKTRACE...

And you build is an exe, so I cannot use it. I will just make a simple fork tomorrow, add logging there and see if the log helps (having it on a repo is way easier to test anyway and I dont wanna force you to add commits just for this.) That way I can also try applying potentional minor fixes and prepare my pull request with the apdata, desktop file etc.

When It works on my repo I will push a pull request to yours and change the urls in the flatpak builder.

Looking back I sbould have made a fork way earlier since many files will later come from the project repo anyway...

LeSnake04 avatar Apr 18 '22 22:04 LeSnake04

Oh, duh, I don't know why I didn't realize I needed to give you a Linux build 😅 Sorry about that.

mtkennerly avatar Apr 18 '22 22:04 mtkennerly

A fork would definitely be good, but here's a proper Linux build with logging in the meantime. I was going to give you a debug build, but it was too big for GitHub to let me attach it, so it's a release build instead:

ludusavi-v0.10.0-post.11+9e5cf6e-linux.zip

mtkennerly avatar Apr 18 '22 23:04 mtkennerly

FIXED IT!

Turns out this had nothing to do with libraries, I just didn't realized there was a permission needed for GPU acceleration.

Playing around in flatseal helped me find this permission.

LeSnake04 avatar Apr 20 '22 19:04 LeSnake04

Any progress on this? Would love to be able to install this on my Steam Deck quickly and easily via flatpak.

lahma0 avatar Jul 18 '22 11:07 lahma0

Its basically done, I just need to get the desktop icon to work. I am trying to fix it within the next weeks (I dont have acess to my main pc for the next 2-3 weeks)

Also auto update is not realisticly possible since cargo isnt properly integrated in flatpak yet. So I will have to update the flatpak whenever there is a new release, so you can forget for now what I said about generated-sources

LeSnake04 avatar Jul 18 '22 12:07 LeSnake04

Thanks for working on this flatpak, I think it will be quite useful for Steam Deck users.

Especially if it has this environment variable set as per the linked issue:

WINIT_X11_SCALE_FACTOR=1

https://github.com/mtkennerly/ludusavi/issues/91#issuecomment-1197671483

psymin avatar Aug 10 '22 15:08 psymin

@psymin: done

@mtkennerly : About the extra files, that should be merged: can you push single commmits to the tag 0.10.0 (without all the other commits) or do we need to wait for the next release and I add the files untill then?

LeSnake04 avatar Aug 11 '22 12:08 LeSnake04

@LeSnake04 Although Git/GitHub allow it, I don't think it's a good idea to change the content of a tag for an existing release. One option would be:

  • I create a release/v0.10.0-flatpak branch that matches the v0.10.0 tag
  • You open a PR to add the files to that branch (the assets folder would be good)
  • I create a v0.10.0+flatpak tag
  • I merge that branch into master so the files can be updated for future releases

I've gone ahead and created the branch, but do you know if this could interfere with Flatpak's x-checker-data behavior? v0.10.0+flatpak would just be a tag without an associated GitHub release.

mtkennerly avatar Aug 11 '22 15:08 mtkennerly

I should probably comment out x-checher-data anyway because of the generated sources situation (the generation makes it very hard to auto update untill cargo become a integrated module type), so the tag needs to be updated manually.

So this would absolutely work

LeSnake04 avatar Aug 11 '22 17:08 LeSnake04

It works!

you can install it with this command: flatpak install --user https://dl.flathub.org/build-repo/102371/com.github.mtkennerly.ludusavi.flatpakref

You might net to reinstall it once its published to get updates: flatpak install --user com.github.mtkennerly.ludusavi
I will notify you once its published.

LeSnake04 avatar Aug 11 '22 19:08 LeSnake04

Yep, it does seem to work on the Steam Deck in Desktop and Game mode.

psymin avatar Aug 11 '22 22:08 psymin

@psymin and the scale is correct as well?

LeSnake04 avatar Aug 11 '22 22:08 LeSnake04

Yep. Now I'm looking forward to cloud support ;)

psymin avatar Aug 11 '22 22:08 psymin

@psymin

Do you happen to know where the steam deck mounts the sd card?

According to the internet its /run/media/mmcblk0p1 can you confirm?

LeSnake04 avatar Aug 12 '22 11:08 LeSnake04

I restricted the flatpak to the home directory and /run/media/.

Interestingly I just realised, that when I click an folder Icon, it opens the desktop-portal file chooser. Seems like Iced uses xdg-desktop-portal. This is good to know for potential future flatpaks with iced projects.

Screenshot_20220812_154227

But I am pretty sure this cant be used too much in this case, since the code will refer to the native path in safes, while the paths gereted by the desktop portal are be different, so it will still write acess to the entire home folder for restoring files without "save to" dialog and, but this will likely work for specifing backup targets to external drives easily. it MAY be possible to integrate completely but this would require a extensive setup by the user and a lot of extra work in the back, which is not work

Though in order to read and write saves from/to external drives outside /run/media/ the user will be required to add acess to the mount path for the flatpak. I added instructions in the flatpak readme. Maybe add the note to the upstream readme as well when including the flatpak instructions @mtkennerly .

LeSnake04 avatar Aug 12 '22 13:08 LeSnake04

The default SD card path is /run/media/mmcblk0p1

A slightly related note: In Desktop mode the file chooser works properly, and I can "Add root" paths.

In Game Mode I'm not seeing a file chooser UI when attempting to "Add root". Not a huge issue since the paths can easily be added in desktop mode.

psymin avatar Aug 12 '22 14:08 psymin

can you please test again with this version?

flatpak uninstall com.github.mtkennerly.ludusavi && flatpak install --user https://dl.flathub.org/build-repo/102371/com.github.mtkennerly.ludusavi.flatpakref

LeSnake04 avatar Aug 12 '22 14:08 LeSnake04

The UI appears to function the same as before in both Desktop and Game modes.

It is possible that I didn't accurately test it in Game mode previously.

Edit: Were you hoping I'd test the file chooser, or something else?

psymin avatar Aug 12 '22 14:08 psymin