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

Feature request: Recognize type of VLC install on the system (package manager vs. Snap) #222

Closed
RogueScholar opened this issue Feb 15, 2019 · 3 comments

Comments

Projects
None yet
3 participants
@RogueScholar
Copy link

commented Feb 15, 2019

With VLC having bought-in wholesale to Canonical's Snap packaging ecosystem (I'm decidedly not a fan for a host of reason including [but not limited to]: increased storage overhead, atrocious integration with AppArmor, old snapshots lingering after updates with no garbage collection, the eighth circle of app theming hell) it might be time to build an awareness into the install process for both types so that it can correctly locate the Lua script where it will be seen and used. I've been doing this manually with each SyncPlay install/upgrade and it's no biggie, but the project can't gain more widespread adoption without supporting less geeky users.

The two installation folders being:

  • /usr/lib/x86_64-linux-gnu/vlc/lua/intf
  • $HOME/snap/vlc/current/.local/share/vlc/lua/intf

I would think a simple if statement for each would suffice. My first thought for those is to use which vlc to trigger the install inside /usr since snaps don't place binaries inside the environment's PATH variable (can you hear my grumbling across the internet over that?) the which command will exit with code 1 if the Snap package was used and that location isn't polled by the app.

For testing if the Snap package is present the solution seems less elegant, requiring piping the output of snap list to grep -q -E "^vlc" to produce a Boolean response to a query for its presence.

Anyhow, thanks for making a nifty open-source tool and maintaining it for all these years and I hope my thoughts here don't come across as presumptuous or too dumb. I don't even recall how I found Syncplay to begin with (TWiT show, maybe?) but it's a handy thing to have in my bag of tricks when the need arises.

@Et0h

This comment has been minimized.

Copy link
Contributor

commented Feb 15, 2019

The GIT HEAD of Syncplay moves from installing the syncplay.lua script when you install Syncplay to copying it into the user folder if it does not exist when you actually try to run VLC through Syncplay. If the current GIT HEAD does not work then please make a pull request which detects /snap/ in the VLC directory and copies to the appropriate User INTF directory.

How to do this: Change the portion of https://github.com/Syncplay/syncplay/blob/master/syncplay/players/vlc.py for the line which reads playerController.vlcIntfUserPath = os.path.join(os.getenv('HOME', '.'), ".local/share/vlc/lua/intf/"). Make it check the contents of playerPath. If the variable contains /snap/ then you can set playerController.vlcIntfUserPath to the appropriate folder for snap. If it does not then it sets it to ".local/share/vlc/lua/intf/" as normal.

You can also add the relevant VLC path to VLC_PATHS in constants.py so that Syncplay will locate the snap version of VLC by default.

@Et0h Et0h added the help wanted label Feb 16, 2019

@albertosottile

This comment has been minimized.

Copy link
Member

commented May 20, 2019

@RogueScholar Do you plan to work on this? Otherwise, I could give it a try.

@albertosottile

This comment has been minimized.

Copy link
Member

commented May 21, 2019

I investigated the issue and wrote a fix for it, I hope you won't mind @RogueScholar. If you could let us know if this commit fixes the issue also on your system, that would be great. Thanks for reporting this.

albertosottile added a commit to albertosottile/syncplay that referenced this issue May 27, 2019

Support for Snap VLC on Linux
Add the snap VLC path to the discovery list and automatically copy
our lua script in the correct path for snap installations.

Fixes: Syncplay#222
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.