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

[FIX-Linux] Snaps Issues Part 1 #3128

Merged
merged 12 commits into from Nov 3, 2023
Merged

[FIX-Linux] Snaps Issues Part 1 #3128

merged 12 commits into from Nov 3, 2023

Conversation

flavioislima
Copy link
Member

@flavioislima flavioislima commented Oct 11, 2023

Fixes:

Current Discussions:
https://forum.snapcraft.io/t/help-fixing-heroic-games-launcher-snap/37248/5
#3127

For Part 2

  • Access to Steam and Lutris folders to look for Proton and Wine.
  • Gamemode
  • Mangohud
  • Gamescope

Use the following Checklist if you have changed something on the Backend or Frontend:

  • Tested the feature and it's working on a current and clean install.
  • Tested the main App features and they are still working on a current and clean install. (Login, Install, Play, Uninstall, Move games, etc.)
  • Created / Updated Tests (If necessary)
  • Created / Updated documentation (If necessary)

@flavioislima flavioislima added pr:wip WIP, don't merge. pr:testing This PR is in testing, don't merge. labels Oct 11, 2023
@soumyaDghosh
Copy link

Hey, these changes probably are not needed. And as said before in the forum, please don't use electron-builder. It'll be worse to maintain later. Please.

@flavioislima
Copy link
Member Author

Hey, these changes probably are not needed. And as said before in the forum, please don't use electron-builder. It'll be worse to maintain later. Please.

It is actually worse for us to maintain a separate thing, if we can do it using electron-builder would be easier since we don't need to change our workflows.

Because otherwise we will need a specific workflow just for snaps and find a way to automate the update and release of it on every release as well.

So unless it is impossible to solve with it then we would try a different way.
I think to fix the other issues now I just need to make the layouts work, right? so I can symlink the binaries on the system to snap?

@soumyaDghosh
Copy link

soumyaDghosh commented Oct 11, 2023

name: heroic
base: core22 
version: '2.9.2' 
summary: Single-line elevator pitch for your amazing snap 
description: |
  This is my-snap's description. You have a paragraph or two to tell the
  most important story about your snap. Keep it under 100 words though,
  we live in tweetspace and your description wants to look good in the snap
  store.

grade: devel
confinement: devmode 
compression: lzo

parts:
  heroic:
    plugin: dump
    source: https://github.com/Heroic-Games-Launcher/HeroicGamesLauncher/releases/download/v$SNAPCRAFT_PROJECT_VERSION/heroic_$SNAPCRAFT_PROJECT_VERSION_$CRAFT_TARGET_ARCH.deb
  
  deps:
    plugin: nil
    stage-packages:
      - curl
    prime:
      - usr/bin/curl

plugs:
  dot-local-share:
    interface: personal-files
    write:
      - $HOME/.local/share/Steam
      - $HOME/.local/share/lutris
      - 
apps:
  heroic:
    command: opt/Heroic/heroic
    desktop: usr/share/applications/heroic.desktop
    environment:
      TMPDIR: $XDG_RUNTIME_DIR
    extensions:
      - gnome

Bringing up my latest manifest here again to compare. Heroic is a game launcher, a very sensitive app, that needs to be fast and efficient. Now, here with this manifest, curl is working prefectly. I can download wine and proton. Now here, check that I only allowed the curl binary to stage and no other file, this way I can keep the thing in check. Now, regarding the workflow, what I am thinking to build is

  1. New Tag made, Deb package made
  2. After that the github workflow will chaneg the Line 3, version: 2.9.2 to the latest tag
  3. The snap package will be made by the snapcraft builder, released to snapcraft directly from the github workflow

Another alternative way, not using github workflow to build the snap

After 2:
This repository will be connected with snapcraft website
It'll auto build from the latest changed manifest, when the manifest changes, if not, just click the Trigger New Build button and it'll build and release to the edge channel.

EDIT: if the team agrees, I can completely maintain the snap thing, on my own.

@flavioislima
Copy link
Member Author

@soumyaDghosh yes I think electron-builder might now allow those options and we will need to go with our own snapcraft file then.

The way that you described could work, but then we trigger the workflow on published like we are doing for the flatpak and the AUR which are totally automated now as well. The snap should be something similar to what we do on the AUR, maybe more simpler since we don't need the even check the md5sum of the package for instance.

About maintaining the Snap, I am fine with it, no problem. But even that might not be needed if we can automate it.

@soumyaDghosh
Copy link

Forgot to mention, Electron Builder uses core20 where as core22 is the current ongoing base and soon core24 will be there based on Ubuntu 24.04 LTS. Latest bases are necessary for latest software drivers etc. Also, core22 brings significant performance boost for Wayland.

@flavioislima
Copy link
Member Author

Forgot to mention, Electron Builder uses core20 where as core22 is the current ongoing base and soon core24 will be there based on Ubuntu 24.04 LTS. Latest bases are necessary for latest software drivers etc. Also, core22 brings significant performance boost for Wayland.

Updated electron-builder to the latest version and using core22 now works fine.
Testing here your snapcraft.yml as well.

@soumyaDghosh
Copy link

My snap manifest isn't complete yet. I most probably hae one with most of the things working. I will share it soon

@arielj
Copy link
Collaborator

arielj commented Oct 11, 2023

question about the curl access, do we need curl? what I mean is... we have node, it should be able to do anything we are using curl for without curl

maybe it's easier to remove the curl dependency than to fix the access to curl?

@soumyaDghosh
Copy link

Forgot to mention, Electron Builder uses core20 where as core22 is the current ongoing base and soon core24 will be there based on Ubuntu 24.04 LTS. Latest bases are necessary for latest software drivers etc. Also, core22 brings significant performance boost for Wayland.

Updated electron-builder to the latest version and using core22 now works fine. Testing here your snapcraft.yml as well.

Kindly give me the snap link. Would like to test the snap.

@ashuntu
Copy link

ashuntu commented Oct 11, 2023

@flavioislima Is there a reason you are not staging curl? I.e. stagePackages: ["default", "curl"]

@flavioislima
Copy link
Member Author

@flavioislima Is there a reason you are not staging curl? I.e. stagePackages: ["default", "curl"]

I tried but it didn't work. I will try again but on Ubuntu.

@flavioislima
Copy link
Member Author

question about the curl access, do we need curl? what I mean is... we have node, it should be able to do anything we are using curl for without curl

maybe it's easier to remove the curl dependency than to fix the access to curl?

For sure. We can use EasyDL lib to download all packages, would be a medium size pr to refactor everything but it's possible.

But we still need to figure it out the rest of the plug-ins like mangohud, etc.

@flavioislima
Copy link
Member Author

I will modify this PR so it will generate a snap to download so we can test it.


// if not on a SNAP environment, set the XDG_CONFIG_HOME to the same location as the config file
if (!process.env.SNAP) {
options.env.XDG_CONFIG_HOME = dirname(legendaryConfigPath)
Copy link
Member

Choose a reason for hiding this comment

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

does changing XDG_CONFIG_HOME under snap cause issues? if it does does it still cause issues if a different variable is used (like LEGENDARY_CONFIG_PATH)?

it might be better to update legendary and use LEGENDARY_CONFIG_PATH once my PR gets merged

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, it breaks the SNAPs because then legendary was trying to write its configs to a place that it is not possible so it was giving permission issues.

@imLinguin
Copy link
Member

For sure. We can use EasyDL lib to download all packages, would be a medium size pr to refactor everything but it's possible.

Don't we have axios that's capable of that already?

@soumyaDghosh
Copy link

@flavioislima Is there a reason you are not staging curl? I.e. stagePackages: ["default", "curl"]

All these uncertainties are the reason, I am still on the side of a custom manifest.

@flavioislima
Copy link
Member Author

For sure. We can use EasyDL lib to download all packages, would be a medium size pr to refactor everything but it's possible.

Don't we have axios that's capable of that already?

Yes but Axios was not intended to download files, it is possible but it is not stable. EasyDL is a pretty good library that does that and supports progress, split files into chunks and continuing to download in case something fails, different from axios that does not have any of these features.

@flavioislima
Copy link
Member Author

@flavioislima Is there a reason you are not staging curl? I.e. stagePackages: ["default", "curl"]

All these uncertainties are the reason, I am still on the side of a custom manifest.

Tried here on Ubuntu and on Arch (with multipass). The curl package is successfully installed but Heroic still cannot access it.
When I try adding gamescope, mangohud or gamemode to the list of stage packages, Snapcraft fails saying that the packages are not available.
So I am starting to think that the only way would be for Heroic to use Classic confinement but it does not meet the requirements for the Snap Store team to approve it.

This leads me to two choices:

  1. Refactor the download part of Heroic that uses Curl so it is not required anymore and show a message to the users saying that some features might not work or are not available on Snaps and recommend it to use a different package if they need that (for Mangohud, Gamemode, and Gamescope), those are not really a HUGE deal to most people so I believe that is fine (perhaps Mangohud only).
  2. Drop the Snap support for good, which I think it is not the best decision since it is the default install method on Ubuntu and Derivates.

For 1, I still need approval from the Snap Store to be able to access those home folders we need. I just tried to publish the package on Edge channel and they denied it because of these new accesses.

@ashuntu
Copy link

ashuntu commented Oct 12, 2023

I still would be on the side of a custom snapcraft yaml, as I think you'd have trouble getting GameMode and MangoHUD to work only by adding their packages via electron-builder. Maybe in the meantime, a message to users about functionality wouldn't be a bad idea, though.

We include GameMode and MangoHUD as separate parts in the Steam Snap if you're looking for inspiration there (https://github.com/canonical/steam-snap/blob/main/snap/snapcraft.yaml#L250-L310). Steam is a finicky case since we force the client to run in 32-bit, so functionality isn't very consistent. I'd be curious to see how well something similar would work for Heroic.

@flavioislima
Copy link
Member Author

I still would be on the side of a custom snapcraft yaml, as I think you'd have trouble getting GameMode and MangoHUD to work only by adding their packages via electron-builder. Maybe in the meantime, a message to users about functionality wouldn't be a bad idea, though.

We include GameMode and MangoHUD as separate parts in the Steam Snap if you're looking for inspiration there (https://github.com/canonical/steam-snap/blob/main/snap/snapcraft.yaml#L250-L310). Steam is a finicky case since we force the client to run in 32-bit, so functionality isn't very consistent. I'd be curious to see how well something similar would work for Heroic.

Ok, so for now I will remove the need of curl from the code and fix these other issues and in the future we try to bring these other tools to the snap package.

@flavioislima
Copy link
Member Author

I am removing the curl dependency here on this PR:
#3129

@soumyaDghosh
Copy link

I still would be on the side of a custom snapcraft yaml, as I think you'd have trouble getting GameMode and MangoHUD to work only by adding their packages via electron-builder. Maybe in the meantime, a message to users about functionality wouldn't be a bad idea, though.

We include GameMode and MangoHUD as separate parts in the Steam Snap if you're looking for inspiration there (https://github.com/canonical/steam-snap/blob/main/snap/snapcraft.yaml#L250-L310). Steam is a finicky case since we force the client to run in 32-bit, so functionality isn't very consistent. I'd be curious to see how well something similar would work for Heroic.

@flavioislima So, should I share a PR on the snap workflow?

@flavioislima
Copy link
Member Author

I still would be on the side of a custom snapcraft yaml, as I think you'd have trouble getting GameMode and MangoHUD to work only by adding their packages via electron-builder. Maybe in the meantime, a message to users about functionality wouldn't be a bad idea, though.
We include GameMode and MangoHUD as separate parts in the Steam Snap if you're looking for inspiration there (https://github.com/canonical/steam-snap/blob/main/snap/snapcraft.yaml#L250-L310). Steam is a finicky case since we force the client to run in 32-bit, so functionality isn't very consistent. I'd be curious to see how well something similar would work for Heroic.

@flavioislima So, should I share a PR on the snap workflow?

sure

@soumyaDghosh
Copy link

I still would be on the side of a custom snapcraft yaml, as I think you'd have trouble getting GameMode and MangoHUD to work only by adding their packages via electron-builder. Maybe in the meantime, a message to users about functionality wouldn't be a bad idea, though.
We include GameMode and MangoHUD as separate parts in the Steam Snap if you're looking for inspiration there (https://github.com/canonical/steam-snap/blob/main/snap/snapcraft.yaml#L250-L310). Steam is a finicky case since we force the client to run in 32-bit, so functionality isn't very consistent. I'd be curious to see how well something similar would work for Heroic.

@flavioislima So, should I share a PR on the snap workflow?

sure

Problem, I want to test this pr, but where can I get the deb file from?

@flavioislima
Copy link
Member Author

I still would be on the side of a custom snapcraft yaml, as I think you'd have trouble getting GameMode and MangoHUD to work only by adding their packages via electron-builder. Maybe in the meantime, a message to users about functionality wouldn't be a bad idea, though.
We include GameMode and MangoHUD as separate parts in the Steam Snap if you're looking for inspiration there (https://github.com/canonical/steam-snap/blob/main/snap/snapcraft.yaml#L250-L310). Steam is a finicky case since we force the client to run in 32-bit, so functionality isn't very consistent. I'd be curious to see how well something similar would work for Heroic.

@flavioislima So, should I share a PR on the snap workflow?

sure

Problem, I want to test this pr, but where can I get the deb file from?

Sorry, missed your reply. I will change this PR so it will generate a .deb during build

@flavioislima
Copy link
Member Author

Added message with warning of missing features:
image

@soumyaDghosh
Copy link

@flavioislima sorry to inform you that heroic being a big project needs a lot of attention and problem is my sem exam is coming soon. So, I'm halting on this thing now. I think if you don't mind, we can restart again after December!

@flavioislima
Copy link
Member Author

@flavioislima sorry to inform you that heroic being a big project needs a lot of attention and problem is my sem exam is coming soon. So, I'm halting on this thing now. I think if you don't mind, we can restart again after December!

No problem. This is also getting more complicated. The app cannot even extract a tar.xz file since the core app is so limited on Snap. So I also need to change the extraction methods to use a npm package instead of native tar command.

@soumyaDghosh
Copy link

@flavioislima sorry to inform you that heroic being a big project needs a lot of attention and problem is my sem exam is coming soon. So, I'm halting on this thing now. I think if you don't mind, we can restart again after December!

No problem. This is also getting more complicated. The app cannot even extract a tar.xz file since the core app is so limited on Snap. So I also need to change the extraction methods to use a npm package instead of native tar command.

Those are possible, and can be done. What I suggest now is to hold any code changes just for the sake of snap. It'll later make a hell for the package.

@flavioislima
Copy link
Member Author

Those are possible, and can be done. What I suggest now is to hold any code changes just for the sake of snap. It'll later make a hell for the package.

Yeah, but at the same time having the extraction, download, etc. Done from Heroic itself will be better since we won't rely on external dependencies and packages. So in the end these are necessary refactors.

@flavioislima
Copy link
Member Author

Merging this one. Should at least fix part of the issues.
Will continue with more fixes afterwards. Perhaps only by using a snapcraft.yml build file we can have a 100% fully working app.

@flavioislima flavioislima changed the title [FIX-Linux] Snaps Issues [FIX-Linux] Snaps Issues Part 1 Nov 3, 2023
@flavioislima flavioislima merged commit ada9cdb into main Nov 3, 2023
13 checks passed
@flavioislima flavioislima deleted the fix/snaps branch November 3, 2023 11:23
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
pr:testing This PR is in testing, don't merge. pr:wip WIP, don't merge.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

6 participants