-
Notifications
You must be signed in to change notification settings - Fork 146
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 plex-core to support synology packages #247
base: master
Are you sure you want to change the base?
Conversation
Is it even possible to install Plex from the command-line on a Synology NAS? I have one that I've got root SSH access on, but I don't know of any way to install the .spk packages from the command-line. |
@demonbane |
That's great! Ok, so this can be added because it is an officially supported platform from Plex, but there are a couple of issues that need to be addressed first. In addition to downloading the Synology package, this should also support:
|
Do you mean installing the package for the first time? or using your plexupdate installer script? It does handle installing the package using manual setup (but the user does have to follow the instructions on this page first to trust plex packages). I haven't modified installer.sh to detect the synology distro, so installer.sh wont work. synopkg automatically starts and stops the package for installation. I don't have an older version of dsm to test with. However, the changes I made don't use any bash functions that weren't already present in the script, so I'd assume it works fine in bash 3.x. |
I did a bit of digging and it looks like most (all?) versions of DSM 6 already include bash 4 so I think that should be ok. The existing code is already pretty heavily dependent on bash 4 which is why I mentioned it. For installation, it would be great if you update installer.sh to support initial setup as well, but the only requirement would be that plexupdate can auto-install the update after downloading it (presumably using the There should also be some logic added to try to determine if we're running on a synology automatically so that it doesn't have to be specified via an environment variable: Lines 298 to 315 in 293e6d5
I'm excited to see this added, so I'll try to be as responsive as possible to any questions you have so we can get this merged soon. Thanks for working on this! |
I patched plexupdate.sh to detect synology and to work with the new 5.json naming scheme. I updated installer.sh too. Where does installer.sh set the BUILD variable? or is it not necessary? I define DISTRO and DISTRO_INSTALL inside of check_distro(). |
any update on this? I am still receiving an issue: #259 |
@d1slact0r no. you can manually download plexupdate-core and plexupdate.sh from my pull request and replace the versions you have on your synology now |
I believe you also need to update the AUTOSTART section to add handling for using synopkg to start the job after the update completes. |
This fork is from an older release which still has the old API endpoints and build designations. It should be rebased to the current HEAD from upstream. |
checks DISTRO variable in getPlexVersion and parseVersion
change cut to return field 2 and 3 instead of just 2
update plexupdate.sh to automatically determine if we're running on a synology
add ability to detect if running on synology in installer.sh
add spaces before closing brackets
changed spaces to tabs
It's been working for me without that, but I'll double check
I just rebased it. |
remove check for plexpass version as now public versions also use new build versions.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks pretty good to me. Once the changes are done I'll try installing Plex on my Synology at home so I can test it locally and then we can get it merged upstream. Thanks very much for working on this!
@@ -228,13 +228,17 @@ isNewerVersion() { | |||
parseVersion() { | |||
if [ "${REDHAT}" = "yes" ]; then | |||
cut -f2- -d- <<< "$1" | cut -f1-4 -d. | |||
elif [ "${DISTRO}" = "synology" ]; then | |||
cut -f2-3 -d- <<< "$1" | |||
else | |||
cut -f2 -d_ <<< "$1" | |||
fi | |||
} | |||
|
|||
getPlexVersion() { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For readability, functions like these should generally start with the "default" option (Ubuntu/Debian) and then go on to the other cases. It's just a bit confusingly written right now because of how the REDHAT
variable is used. I'd suggest swapping the two first cases and adding a comment (which I really should have done when I wrote the function 🤦♂ ), about what the "${REDHAT}" != "yes"
does.)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The way GitHub is rendering this is a bit confusing. The comment is about the getPlexVersion
function, not isNewerVersion
.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so if I just switch the cases like so
if [ "${REDHAT}" != "yes" ]; then
dpkg-query --showformat='${Version}' --show plexmediaserver 2>/dev/null
elif [ "${DISTRO}" = "synology" ]; then
synopkg version "Plex Media Server" 2>/dev/null
I don't think the switch ever reaches the synology case because "${REDHAT}" != "yes"
is true for synology devices too.
I'm not sure how best to change it.
Now that we're going to have 3 supported platforms, I think it's also time to retire the |
BUILD="linux-${ARCH}" is already done at line 56 Co-Authored-By: Alex Malinovich <alexmalinovich@gmail.com>
Before you install it on your synology, you'll have to follow the instructions in How to add Plex’s package signing public key to Synology NAS Package Center. That link should be added to the readme or the wiki. re: redhat, I don't have any linux boxes to test with, and I'm not very familiar with them anyways. So unfortunately, I'm not really comfortable making those changes. |
Add link to "How to add Plex’s package signing public key to Synology NAS Package Center"
I added it to the README since you need to do that step whether you use automatic or manual setup. |
Can the package signing key stuff be done via the command line? That would be a great thing to add to |
The installer only knows how to put a file into What I'd love would be if the installer can just create a Task Scheduler task from the command line, but I'm not having much luck in finding a way to do it. That would still be my preferred solution, but if we can't find a way to do it then we can at least print a link to a (soon to be written) wiki article on how to set up a scheduled task on Synology. (just need to short-circuit the logic at the top of |
The GUI task scheduler just adds the tasks to /etc/crontab using synoschedtask. In my crontab We should be able to add a line to /etc/crontab, link cronwrapper inside of /etc/cron.d, or create a /etc/cron.daily folder to use. |
Do you know if cron job output will still get sent to the admin? I have a couple of scheduled tasks on my NAS and when they run I get an HTML email from Synology showing the output which leads me to believe that notifications are handled by synoschedtask, not the usual cron mail configuration. |
Oh, and the system crontab has Ok, so in that case we can still add the job, we just need to provide a warning that on Synology you won't get any output. Any thoughts on a good flow to use in the installer that'll also be portable to other Linux systems that may not use the cron.daily setup? |
I think if there's no cron.daily, it should check for cron.d. If it finds cron.d, change CRONWRAPPER to /etc/cron.d/plexupdate, then create a crontab file named plexupdate in cron.d with one line that just calls the cronwrapper as a script like So add some logic around line 235 and around line 277 to check CRONWRAPPER and switch to something like |
I agree with everything except:
At the time, using cron.daily was just the simplest way of implementing this, but if we're going to just use cron.d might as well use it by default and get rid of cron.daily altogether. Since installer.sh is meant to be run multiple times without problems, there should be some code to migrate an old cron.daily setup to cron.d. (for bonus points, grab the cron.daily time in crontab and use that in the cron.d entry. And if |
Use new crontab file file in cron.d instead of cron.daily to support more devices
I changed the cron support to use cron.d instead of cron.daily. There's not anything to migrate since it still uses /etc/plexupdate.cron.conf for settings, and that file gets overwritten every time you run installer.sh anyways. But it does remove the old file in cron.daily if its a symlink. I look for a cron.daily job in crontab for the time, otherwise I just picked 4 am. May be something we prompt the user for. I'm having some weird trouble with it running though. The cron job runs at the right time, and it does run the cronwrapper, and that does run plexupdate.sh, but it doesn't actually update. However if I manually run cronwrapper, it works fine. Maybe something with permissions or something weird with cron. So I'm curious if it's just my machine it happens on. |
On second look, it might be because I don't include PATH in the new cron file... |
yeah, that fixed it. Should be all good now. |
Easier to see what /etc/cron.d/plexupdate will look like
AUTOSTART is needed on synology
So, is this about to be merged, or..? |
Hopefully late next week. I’ve been traveling the last few weeks and haven’t had a chance to review the latest updates, but it’s on my list as soon as I get back. |
I've checked out this pull request and it still seems that PMS isn't started again after an update. |
@deepfriedmind Which Synology and DSM version are you using? It’s been working for me on a ds916+ and dsm 6. |
I'm using a DS916+ on DSM 6.2.2-24922 Update 4. |
Here are my troubleshooting questions:
|
I was able to install and configure this branch successfully on a DS418 with DSM 6.2.3-25426 Update 2 |
This no longer appears to work on DSM 7; it looks like there is a new |
Checks DISTRO variable in getPlexVersion and parseVersion to see if script is running on synology.
Fixes #245