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

kodi: service addon wrapper call fix #3191

Merged
merged 1 commit into from Jan 4, 2019

Conversation

@vpeter4
Copy link
Contributor

commented Dec 27, 2018

Currently when addon is installed it's service is enabled and started.
But immediately service is stopped, disabled, enabled and started again.
This second part should be executed only on addon's update.

Discussion in PR #1835 #1835 (comment)

@vpeter4 vpeter4 requested a review from HiassofT Dec 27, 2018

@HiassofT

This comment has been minimized.

Copy link
Member

commented Dec 28, 2018

Thanks, this looks good to me!

Could you also rename LE_ADDON_POST_INSTALL to LE_ADDON_POST_UPDATE and change the post-install option to post-update (both in kodi patch and in the service-addon-wrapper script)? The first comment in the post-install case in the script ("post-install is triggered on update as well,") should probably be dropped.

I had a quick look if we could also get a pre-update notification from kodi (then pre-update handling could be merged with disable and post-update with enable) but didn't have much luck. When AddonInstaller sends the PreInstall notiification addon->Path() still points to the ZIP file, not the addon directory. Maybe I'll have a look at it again sometimes later, but for now your changes are good enough.

kodi: service addon wrapper call fix
Currently when addon is installed it's service is enabled and started.
But immediately service is stopped, disabled, enabled and started again.
This second part should be executed only on addon's update.

@vpeter4 vpeter4 force-pushed the vpeter4:addon_service_fix branch from 9e12b78 to 3fc3656 Dec 28, 2018

@vpeter4

This comment has been minimized.

Copy link
Contributor Author

commented Dec 28, 2018

Done.

@HiassofT
Copy link
Member

left a comment

thanks!

@vpeter4

This comment has been minimized.

Copy link
Contributor Author

commented Jan 4, 2019

Is something wrong with this PR? It fixes a bug but still ignored :(

@CvH

This comment has been minimized.

Copy link
Member

commented Jan 4, 2019

The pr is at the @MilhouseVH builds for a while, this gets merged for B3 if no problems appear.

@MilhouseVH MilhouseVH merged commit df9076a into LibreELEC:master Jan 4, 2019

@vpeter4 vpeter4 deleted the vpeter4:addon_service_fix branch Jan 4, 2019

@aptalca

This comment has been minimized.

Copy link
Contributor

commented Feb 7, 2019

@HiassofT @vpeter4 I noticed that my systemd service addons are no longer restarting the services when addon settings change. Could this PR be the reason? My addons use the following:

   def onSettingsChanged(self):
      subprocess.call(['systemctl', 'restart', self.id])

Here's the full code: https://github.com/linuxserver/libreelec-addon-repo/blob/letsencrypt2/docker.linuxserver.letsencrypt/default.py

Thanks

EDIT: Actually, I confirmed that the PR is unrelated to the issue, sorry for the noise

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
5 participants
You can’t perform that action at this time.