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

Update usage instructions for Flatpak #184

Open
wants to merge 1 commit into
base: main
Choose a base branch
from

Conversation

guihkx
Copy link
Contributor

@guihkx guihkx commented Sep 15, 2024

This simplifies things regarding the Flatpak set up. This also adds:

  • Flatpak-specific build instructions.
  • Remove inaccurate comment about the Flatpak package.
  • Remove every $ at the beginning of each command for easier copying and pasting.

Rendered page: https://github.com/guihkx/spotify-adblock/blob/readme-flatpak/README.md

Closes #112

@guihkx guihkx force-pushed the readme-flatpak branch 2 times, most recently from 82f9f19 to 0288679 Compare September 16, 2024 16:41
@guihkx
Copy link
Contributor Author

guihkx commented Sep 16, 2024

Apologies for the extra force-pushes. It should be ready for review now.

@axelsimon
Copy link

@guihkx small mistake in your instructions.

mkdir -p ~/.var/app/com.spotify.Client/config/spotify-adblock
but then:
cp config.toml ~/.var/app/com.spotify.Client/spotify-adblock

/config is missing, the right command is:
cp config.toml ~/.var/app/com.spotify.Client/config/spotify-adblock

Also, i'm not sure i like telling people they just need to use podman to build. At least we should give people the option: either you need cargo + rust, or if you don't want to install that you can run a container using podman (why Ubuntu 23.10 though, at least use a LTS or a recent version). But then again you can also use nix and nix-shell -p cargo so, i'm not sure we should push flatpak users to a specific solution.

Copy link

@axelsimon axelsimon left a comment

Choose a reason for hiding this comment

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

Needed change.

README.md Outdated Show resolved Hide resolved
@guihkx
Copy link
Contributor Author

guihkx commented Oct 18, 2024

Also, i'm not sure i like telling people they just need to use podman to build.

That's necessary because building this library on a distro that has a different glibc version than the one provided by the Freedesktop Runtime (which Spotify for Flatpak runs on), may cause the library not to work (see #24 and #28).

And that's the reason why Ubuntu 23.10 was chosen: It has glibc 2.38, which is the same version present in the 23.08 Freedesktop Runtime, which was used by Spotify when I created this pull request, but now they've updated to 24.08, so I'll need to find another distro with the same glibc version...

i'm not sure we should push flatpak users to a specific solution.

I'm done because it's the solution that I tested and it's guaranteed work. People are, of course, free to choose whatever build methods they like. Or they can avoid the build process completely and try their luck with the pre-built library, but again, I can't guarantee that will work...

@guihkx
Copy link
Contributor Author

guihkx commented Oct 18, 2024

Updates:

  • Fixed the cp config.toml ... command not targeting the right location.
  • Updated the Flatpak-specific build instructions to use a Ubuntu 24.10 container image, since it has glibc 2.40, which matches the version used by the runtime Spotify for Flatpak currently runs on (24.08).

@guihkx guihkx closed this Oct 18, 2024
@guihkx guihkx deleted the readme-flatpak branch October 18, 2024 15:41
@guihkx guihkx restored the readme-flatpak branch October 18, 2024 15:41
@guihkx
Copy link
Contributor Author

guihkx commented Oct 18, 2024

I'm so sorry, I deleted the branch by mistake while I was pushing changes to this other branch I'm working on.

@guihkx guihkx reopened this Oct 18, 2024
And also add Flatpak-specific build instructions.
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.

Spotify flatpak fix
2 participants