Update Tvheadend to 4.2.6 (maintenance release) for DSM 5+6 #3050
Conversation
|
@ymartin59 Could you take a look at this, please? I think the package is created correctly. It installs fine on XPE5.2, but the service is not executed after the package has been installed. I'm pretty sure that something is wrong with EDIT: I cherry-picked your Mosquitto PRs as they seem to contain amendments to the generic service routines. Will drop them from this PR when they have been merged into master... tvheadend_install.log: tvheadend.log: |
m4tt075
changed the title
|
@ymartin59 I could track the problem down for now, but still some way to go. Here my observations:
|
|
@m4tt075 Correct, I have configured "home directories" and so have not detected this requirement. As If as I guessed, I know it is more complex but "generic" support cannot fit all situations. What do you think about this proposal ? |
|
@ymartin59 Cool. Thanks. Yes, that would work, but I might have found a different solution: It seems |
|
@ymartin59 I have just pushed some fixes, which utilize a combination of your |
|
@ymartin59 I have installed an XPE 6.1 testing system this morning, and the package installs and runs with correct permissions as well. The only problem I'm still facing is that for some reason the |
|
@ymartin59 Yikes! See last commit. Took me hours to find that nasty error. It's the same in your |
|
@ymartin59 I have adapted the TVH pacakge using your There is one issue left: Upgrading the package with itself continues to nuke the old configuration. I have found the Otherwise, the migration should be done. Your review will be appreciated. |
|
@ymartin59 Contingent to merging of @Safihre 's fixes in PR #3064 , this package now works correctly and upgrades without nuking previous configurations. Without PR #3064 it stays dangerous and should not be merged. I have cleaned up some additional minors and squashed all changes into a single commit. With this, for your review, please. |
m4tt075
changed the title
|
@ymartin59 Minor cleanup in line with your review comments to the analogous Jackett PR #3070 ; squashed and force-pushed. |
There was a problem hiding this comment.
Good. I propose you to add:
- download location as
wizard_download_dirin wizard for user to provide share path (like indemoserviceand Update SABnzbd to use Generic service and version 2.3.2 #3065) SERVICE_WIZARD_SHARE=wizard_download_dirinMakefileto enable share creation and supportSERVICE_PORTinMakefileso thatapp/configis generated by https://github.com/SynoCommunity/spksrc/blob/master/mk/spksrc.service.mk#L190- enforce group name thanks to
GROUP=tvheadendas first line ofsrc/service-setup.sh, to get group created andEFF_USERenlisted as member
spk/tvheadend/src/service-setup.sh
Outdated
| echo "service_postinst ${SYNOPKG_PKG_STATUS}" >> $INST_LOG | ||
| } | ||
|
|
||
| service_preuninst () |
There was a problem hiding this comment.
Please remove functions that are useless for your package. I have only written them for demonstration / logging of DSM call sequence.
spk/tvheadend/Makefile
Outdated
| POST_STRIP_TARGET = tvheadend_extra_install | ||
|
|
||
| BUSYBOX_CONFIG = usrmng | ||
| ENV += BUSYBOX_CONFIG="$(BUSYBOX_CONFIG)" | ||
|
|
||
| include ../../mk/spksrc.spk.mk | ||
|
|
||
| .PHONY: tvheadend_install |
There was a problem hiding this comment.
Please remove this target. In demonstration service it is required as these was no source package with "make install" available.
spk/tvheadend/Makefile
Outdated
| mkdir --parents $(STAGING_INSTALL_PREFIX) | ||
| install -m 755 -d $(STAGING_INSTALL_PREFIX)/bin | ||
| install -m 755 src/tvheadend.sh $(STAGING_INSTALL_PREFIX)/bin | ||
|
|
||
| .PHONY: tvheadend_extra_install | ||
| tvheadend_extra_install: | ||
| install -m 755 -d $(STAGING_DIR)/app |
There was a problem hiding this comment.
I wonder why this additional install invocation is really required as DSM_UI_DIR is expected to trigger standard copy in spksrc makefiles.
spk/tvheadend/src/tvheadend.sh
Outdated
| HOME_DIR="${SYNOPKG_PKGDEST}/var" | ||
| LOG_FILE="${HOME_DIR}/tvheadend.log" | ||
| PID_FILE="${HOME_DIR}/tvheadend.pid" | ||
| ${SYNOPKG_PKGDEST}/bin/tvheadend -f -u ${USRN} -g ${USRG} --http_port ${HTTPP} --htsp_port ${HTSPP} -c ${HOME_DIR} -l ${LOG_FILE} -p ${PID_FILE} |
There was a problem hiding this comment.
You should use $GROUP group which is expected to be sc-media by default
There was a problem hiding this comment.
I have just tried to use ${GROUP} instead of ${USRG} but it is an empty variable when used from within service-setup.sh. I will retain ${USRG} for the time being.
There was a problem hiding this comment.
You have to hardcode it as one of first lines in service-setup.sh like @ymartin59 says a bit up here in the other comments and like I did for SABnzbd: 2ac3e53#diff-5d6bd92e6f38c326c2344d53c1dfb20aR10
There was a problem hiding this comment.
Ahh, now I see. Misunderstood it. Thanks for clarifying! Will put it in sc-media then...
There was a problem hiding this comment.
Just tried this, but it screws up all the user permissions again. I will revert to the old tvheadend goup and force push to get a cleaner version. I also think you have been reviewing outdated commits, as the tvheadend.sh file has already been removed some days ago.
spk/tvheadend/Makefile
Outdated
| rm -fr $(STAGING_DIR) | ||
| mkdir $(STAGING_DIR) | ||
| mkdir --parents $(STAGING_INSTALL_PREFIX) | ||
|
|
||
| .PHONY: tvheadend_extra_install | ||
| tvheadend_extra_install: |
There was a problem hiding this comment.
I really wonder if this target is really usefull too... With DSM_UI_DIR, folder is copied without such install command.
|
In fact, DSM 6 framework creates service account with a default group with same name - you have used as process argument. |
|
@ymartin59 Thanks. Mostly implemented. See
|
|
OK sorry for my mistake. As DrBean has set "GROUP=sc-media" in installer script, I thought there was a download directory. So forgot about this point about share create and group name. Your shell requesting current user group required as service parameter is perfect so. |
|
As a consequence, please clean wizard steps about permission management, as this is not relevant either. |
|
... except if |
|
@ymartin59 You are right. I'm hardcoding However, this leaves me in similar situation to what @Safihre encountered for |
m4tt075
changed the title
|
@m4tt075 I didn't test anything on DSM5.. Do you have an indication what is going wrong in DSM5? |
|
@Safihre Nope, I don't. I tried recording but did no have permissions. I have checked the group permission settings in DSM and could verify that they were not set correctly. I checked the |
|
@Safihre we are testing DSM 5 with XPEnology virtual machine... but it is quite complex to set up, and there too proper instructions to succeed at first trial are lacking in wiki. |
spk/tvheadend/src/service-setup.sh
Outdated
| HTSPP=9982 | ||
|
|
||
| # Need to enforce correct permissions for recording directories on upgrades | ||
| UPGRADE_CFG_DIR="${TMP_DIR}/var/dvr/config" |
There was a problem hiding this comment.
Probably it is a better place in service_preupgrade ?
|
@ymartin59 @Safihre Thanks, guys, but in the meantime 4.2.6 has been published. In addition the upgrade procedures are dependent on the actual volume number, which can be avoided. I have got the upgrade and the fixes, but still need to compile and push them. Have been drowning recently. Will try to do it over the weekend... |
|
No hurry... was just a reminder... backlog is huge ! |
…upport for DSM 5+6
… to new generic scripts
… in package system directory
…standard installations
m4tt075
changed the title
|
OK, pushed the fixes and the update, adapted PR title and description, travis all green (link), new test builds released (link) and advertised in @Safihre 's test build thread. Since this is the latest release, I'd expect a substantial number of downloads relatively quickly. Let's see how it goes... |
|
Unless there's spk specific comments, let's merge and publish by mid next week (18 April). The PR has been open for 4 months.. |
|
No comments received, so time to get this out! Do you have time to build and publish @ymartin59 ? :) |
|
Hello, Thanks, noncon66 |
|
Tvheadend should be running on ports 9981 and 9982. Not sure what is blocking it from installing. Any other package that would come to your mind and which you can remove? |
|
@ymartin59 No time to build? |
|
@ymartin59 Possible, as there have been several packages from different sources flying around, but I don't believe in some systematic problem. I have had this as well with another package some time ago, but could not reproduce it. The 4.2.4 version and the 4.2.5 and 4.2.6 test builds together must have had thousands of downloads in the meantime. I'm just aware of this single report and the reporter did not come back either. |
|
@m4tt075 Sorry for not reporting back. I could not find any other package blocking the port. So I tried a restart. After that the package installed without any problem. |
|
@ymartin59 I think something went wrong in the publishing process. |
|
The code of |
|
This is really a media-package,s o the |
|
Yes, that was my understanding as well. It should not interfere with the |
…unity#3050) * Update Tvheadend to 4.2.5 (maintenance release) and generic service support for DSM 5+6 * [TVH] Adapt to run as background service and adapt permission setting to new generic scripts * [TVH] Upgrade fixes: Permissions and handling of potential recordings in package system directory * [TVH] Improved recording recovery from package root directory * [TVH] Remove explicit volume dependencies in install scripts for non-standard installations * [TVH] Update TVH to 4.2.6 (maintenance release) * [TVH] Remove version patch (no longer required)
Motivation
Update Tvheadend to 4.2.6 (latest maintenance release), update libhdhomerun to 20171221 and adapt Tvheadend to use generic DSM 5+6 service
Linked issues
#2904
Checklist
all-supportedcompleted successfully with5.2toolchainsall-supportedcompleted successfully with6.1toolchains (exceptions above)XPE 5.2andXPE 6.1)XPE 5.2andXPE 6.1)