Update Jackett to v0.8.490 and to use generic DSM 5+6 services #3070

m4tt075 · 2018-01-01T11:29:08Z

Motivation

Contains @kaso17 's Jackett updates and fixes of #3032 and adapts the package to use generic DSM 5+6 services

Linked issues

#3015, #2986, #2881, #1995, Jackett/Jackett#1985, Jackett/Jackett#2217

Notes

~~Depends on merging of @kaso 's Jackett update PR jackett: update to v0.8.490 #3032.~~ Squashed into this PR on request of @ymartin59 and with agreement of @kaso17
~~Depends on merging of Changes to Generic service #3064. Do NOT merge without it or old configurations will be overwritten on upgrades.~~ Changes to Generic service #3064 merged in the meantime.
~~Further testing needed.~~ Outdated.

Checklist

Build rule all-supported completed successfully on XPE 5.2 and XPE 6.1 systems.
Package upgrade completed successfully
New installation of package completed successfully

m4tt075 · 2018-01-01T11:44:49Z

@kaso17 As to your hints / notes in #3032:

I have tested the package on an XPE 5.2 and XPE 6.1 system. It installs and runs as far as I can judge. At least I can access the web interface in both cases.
User premissions for the share folder seem correct, because I could succesfully upgrade the package using the internal upgrade process on both systems.
I could not test a migration from a legacy installation as I don't have one, but would rely on the generic service scripts to reset user permissions correctly as this applies to all packages.
I could verify that all environment variables are set and used correctly.
The only piece I could not implement from your approach was the logging functionality as this is driven through the generic service scripts. As such, standard logging to var/jackett.log is used for now. It works to the extend that both DSM and jackett log into that file correctly. It can be accessed through the DSM web-UI. However, your (very nice) log rotation functionality is lost unfortunately. I don't know how much information is usually written to the logs, but if you foresee an issue here, please let me know.

Edit: I have cherry-picked your commits from PR #3032 . As soon as #3032 is merged, I will drop them from my PR here...

kianrafiee · 2018-01-01T20:26:49Z

newly installed jackett package for rtd1296 from @Safihre repo will not start. it was starting fine yesterday and updated. but now it wont start. not sure if there was a jackett update yesterday that broke it

m4tt075 · 2018-01-01T20:32:34Z

Not related to this PR or spksrc development. Therefore, please report this in the Issues section instead, either in one of the old threads or open a new one.

ymartin59

@m4tt075 May you please review related issues I have listed and apply minor changes I proposed

ymartin59 · 2018-01-01T21:38:13Z

spk/jackett/Makefile

-MAINTAINER = SynoCommunity
-DESCRIPTION = "Jackett works as a proxy server: it translates queries from apps into tracker-site-specific http queries, parses the html response, then sends results back to the requesting software. Jackett is a single repository of maintained indexer scraping & translation logic - removing the burden from other apps."
-ADMIN_PORT = 9117
+MAINTAINER = 'kaso17 (Jackett), m4tt075 (spksrc integration)'


I wonder if spkrepo will accept packages with such maintainer. As far as I understood/remember, it has to match a user account registered in registry

ymartin59 · 2018-01-01T21:40:03Z

spk/jackett/Makefile


-INSTALL_PREFIX = /usr/local/$(SPK_NAME)
+# Admin link
+ADMIN_PORT = $(SERVICE_PORT)


Is it relevant to consider SERVICE_PORT as ADMIN_PORT ? Probably not, but I prefer to keep it for "backward compatibility"

Sorry, not sure I understand. Do you want me to remove the comment and the ADMIN_PORT definition or do you want me to keep it?

It depends if link generated in package view is really usable (often http://ip:ADMIN_PORT/) whereas SERVICE_PORT is used to provide application link in app/config which allows finer settings like https and context path in url.
So if "admin link" in package view is not usable as-is, please remove it. If it works, let it there.

It is usable and works correctly, so I kept it.

ymartin59 · 2018-01-01T21:43:28Z

spk/jackett/src/service-setup.sh

+    COMMAND="env HOME=${HOME_DIR} PATH=${PATH} LD_LIBRARY_PATH=${SYNOPKG_PKGDEST}/lib ${MONO} --debug ${JACKETT} --PIDFile ${PID_FILE}"
+
+    if [ $SYNOPKG_DSM_VERSION_MAJOR -lt 6 ]; then
+        su ${EFF_USER} -s /bin/sh -c "cd ${SYNOPKG_PKGDEST}; ${COMMAND}" >> ${LOG_FILE} 2>&1 &


Is it really needed to change process working directory to SYNOPKG_PKGDEST ? (I did it on demoservice to browse files...)
I think HOME_DIR may be more relevant, and it is already done by su

ymartin59 · 2018-01-01T21:44:19Z

spk/jackett/src/service-setup.sh

+    HOME_DIR="${SYNOPKG_PKGDEST}/var"
+
+    echo "Starting Jackett as user ${EFF_USER}" >> ${LOG_FILE}
+    COMMAND="env HOME=${HOME_DIR} PATH=${PATH} LD_LIBRARY_PATH=${SYNOPKG_PKGDEST}/lib ${MONO} --debug ${JACKETT} --PIDFile ${PID_FILE}"


Is --debug flag really relevant ?

ymartin59 · 2018-01-01T21:46:16Z

spk/jackett/src/service-setup.sh

+    fi
+}
+
+service_preinst ()


Please discard all other hook functions with only logging... I only wrote them for demo, I consider them not relevant for a package

m4tt075 · 2018-01-01T22:08:57Z

@ymartin59 Thanks for your review. I have addressed your comments, except the ADMIN_PORT one (see my question above). Will drop and squash once #3032 and #3064 are merged.

kaso17 · 2018-01-02T17:48:57Z

@m4tt075 thank you for taking care of this.

EDIT: sorry, didn't see that you tested the update mechanism.

I don't see any obvious issues.

m4tt075 · 2018-01-02T20:16:54Z

@kaso17 All fine. Thanks for reviewing...

ymartin59

Still some details but soon ready to go

ymartin59 · 2018-01-05T23:19:56Z

spk/jackett/src/service-setup.sh

+    if [ $SYNOPKG_DSM_VERSION_MAJOR -lt 6 ]; then
+        su ${EFF_USER} -s /bin/sh -c "${COMMAND}" >> ${LOG_FILE} 2>&1 &
+    else
+        cd ${SYNOPKG_PKGDEST}


You removed "cd" in COMMAND for DSM 5 and so it should be removed here for DSM 6 too.

ymartin59 · 2018-01-05T23:23:33Z

spk/jackett/Makefile


-INSTALL_PREFIX = /usr/local/$(SPK_NAME)
+# Admin link
+ADMIN_PORT = $(SERVICE_PORT)


It depends if link generated in package view is really usable (often http://ip:ADMIN_PORT/) whereas SERVICE_PORT is used to provide application link in app/config which allows finer settings like https and context path in url.
So if "admin link" in package view is not usable as-is, please remove it. If it works, let it there.

m4tt075 · 2018-01-06T21:56:12Z

@ymartin59 I have addressed your latest comments. Anything else?

ward0 · 2018-01-08T11:41:03Z

When can we see an update pls? @m4tt075

m4tt075 · 2018-01-08T14:11:15Z

@ymartin59 @kaso17 Following up with your recent comments in #3032, I have...

rebased
squashed all commits (including @kaso17 's) into one
Adapted the PR title
Adapted the PR description to keep the linked issues from @kaso17 's PR description
Re-compiled Jackett for bromolow architecture 5.2 and 6.1, just to make sure
Successfully upgraded the package on XPE 6.1 within the package manager and then again, using Jackett's internal upgrading process
Successfully installed the package on XPE 5.2 and upgraded it using Jackett's internal upgrading process

Should be ready to go now...

ymartin59 · 2018-01-08T17:43:26Z

Marvelous. Merging.
By the way, online repository rejects mono 5.4 package for x64... so it may not be published in a short term.

ymartin59

After @Safihre feedback with other packages, I have still some "improvements" to propose

ymartin59 · 2018-01-16T08:24:35Z

spk/jackett/src/service-setup.sh

+{
+    # Replace generic service startup
+
+    MONO_PATH="${SYNOPKG_PKGDEST}/../mono/bin"


Bad idea... Prefer to use /usr/local/mono/bin

ymartin59 · 2018-01-16T08:26:08Z

spk/jackett/src/service-setup.sh

+    echo "Starting Jackett as user ${EFF_USER}" >> ${LOG_FILE}
+    COMMAND="env HOME=${HOME_DIR} PATH=${PATH} LD_LIBRARY_PATH=${SYNOPKG_PKGDEST}/lib ${MONO} ${JACKETT} --PIDFile ${PID_FILE}"
+
+    if [ $SYNOPKG_DSM_VERSION_MAJOR -lt 6 ]; then


These if statements are same as generic script.
I think moving all variables settings out of service_prestart (so removing this function) and provide SERVICE_COMMAND at the begging of this script should work.

No that won't work because the process never detaches: no & at the end when using SERVICE_COMMAND for processes that don't support deamon.
So DSM will stay in a constant state of Starting since the start never returns (since iets running the actual application).
That's why we need this special function. Or integrated in generic approach :)

LOL Just came here to write the same! Agree with @Safihre . This is another candidate for the start-stop-daemon approach. I propose to keep this package as is for the time being and update once that approach is established...

m4tt075 · 2018-02-23T17:40:57Z

@ymartin59 This has been merged 6 weeks ago. Anything preventing this from being published?

ymartin59 · 2018-02-24T08:38:40Z

@m4tt075 Yes... Mono update. The fix I designed to relocate cert-sync seems to work. My DSM has a pending update, I will test everything is ok with mono+jackett upgrade

jdeluyck · 2018-02-25T14:20:07Z

Having just pulled in Mono, and just wanted to pull in Jackett, it fails to upgrade from 0.7.1483-4 to the latest. Complains about corrupt package?

Safihre · 2018-02-25T14:22:48Z

@jdeluyck Could you restart your NAS and try again?
What model NAS do you use?

jdeluyck · 2018-02-25T14:43:43Z

@Safihre I'm using a DS916+.
Rebooted NAS, retried upgrade, same story.

ymartin59 · 2018-02-25T17:51:18Z

Hum hum. Corrupt package ? It remembers me trouble with signing process in repository... I have already fixed it (and there was no issue with recent published packages like mono, ffmpeg...) so probably there is a specific failure with Jackett. I am investigating....

ymartin59 · 2018-02-25T21:43:20Z

@jdeluyck As an immediate work-around, open Package Center "Settings" and select "Any publishers" in Trust Level section... sorry for the inconvenience

Safihre · 2018-02-26T07:41:59Z

Simple re-publish is not fixing it? I indeed also see this error..

jdeluyck · 2018-02-26T14:59:49Z

@ymartin59 that indeed works. So the package is ok, signing is borked.

m4tt075 mentioned this pull request Jan 1, 2018

jackett: update to v0.8.490 #3032

Closed

3 tasks

This was referenced Jan 1, 2018

jackett wont start (rtd1296) #3071

Closed

Compiling jackett for rtd1296 #3015

Closed

[Package Request] Update Jackett #2986

Closed

Provider t411.si don't work in Jackett #2881

Closed

Jacket Works but Crashes Frequently #1995

Closed

ymartin59 reviewed Jan 1, 2018

View reviewed changes

m4tt075 mentioned this pull request Jan 2, 2018

Update Tvheadend to 4.2.6 (maintenance release) for DSM 5+6 #3050

Merged

4 tasks

ymartin59 reviewed Jan 5, 2018

View reviewed changes

m4tt075 force-pushed the jackett-service branch from f44fc9c to 0550633 Compare January 6, 2018 21:53

m4tt075 changed the title ~~[WIP] Update Jackett to use generic DSM 5+6 services~~ Update Jackett to use generic DSM 5+6 services Jan 6, 2018

kaso17 mentioned this pull request Jan 8, 2018

Synology service not starting Jackett/Jackett#2337

Closed

Update Jackett to v0.8.490 and to use generic DSM 5+6 services

835ef61

m4tt075 force-pushed the jackett-service branch from 0550633 to 835ef61 Compare January 8, 2018 13:39

m4tt075 changed the title ~~Update Jackett to use generic DSM 5+6 services~~ Update Jackett to v0.8.490 and to use generic DSM 5+6 services Jan 8, 2018

ymartin59 merged commit 4104452 into SynoCommunity:master Jan 8, 2018

m4tt075 deleted the jackett-service branch January 8, 2018 20:17

m4tt075 mentioned this pull request Jan 12, 2018

Meta: DSM6 compatibility of packages with user privilege issues #2904

Closed

ymartin59 added the status/to-publish label Jan 14, 2018

ymartin59 reviewed Jan 16, 2018

View reviewed changes

ymartin59 removed the status/to-publish label Feb 24, 2018

Update Jackett to v0.8.490 and to use generic DSM 5+6 services #3070

Update Jackett to v0.8.490 and to use generic DSM 5+6 services #3070

Conversation

m4tt075 commented Jan 1, 2018 • edited Loading

Motivation

Linked issues

Notes

Checklist

m4tt075 commented Jan 1, 2018 • edited Loading

kianrafiee commented Jan 1, 2018 • edited Loading

m4tt075 commented Jan 1, 2018

ymartin59 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m4tt075 commented Jan 1, 2018

kaso17 commented Jan 2, 2018 • edited Loading

m4tt075 commented Jan 2, 2018

ymartin59 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m4tt075 commented Jan 6, 2018

ward0 commented Jan 8, 2018

m4tt075 commented Jan 8, 2018

ymartin59 commented Jan 8, 2018

ymartin59 left a comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Safihre Jan 16, 2018 • edited Loading

Choose a reason for hiding this comment

Choose a reason for hiding this comment

m4tt075 commented Feb 23, 2018

ymartin59 commented Feb 24, 2018

jdeluyck commented Feb 25, 2018

Safihre commented Feb 25, 2018

jdeluyck commented Feb 25, 2018 • edited Loading

ymartin59 commented Feb 25, 2018

ymartin59 commented Feb 25, 2018

Safihre commented Feb 26, 2018

jdeluyck commented Feb 26, 2018

m4tt075 commented Jan 1, 2018 •

edited

Loading

m4tt075 commented Jan 1, 2018 •

edited

Loading

kianrafiee commented Jan 1, 2018 •

edited

Loading

kaso17 commented Jan 2, 2018 •

edited

Loading

Safihre Jan 16, 2018 •

edited

Loading

jdeluyck commented Feb 25, 2018 •

edited

Loading