Syncthing - Update 0.14.44 - Generic Service approach #3094

Safihre · 2018-01-11T17:45:34Z

Checklist

Build rule all-supported completed successfully
Package upgrade completed successfully
New installation of package completed successfully

m4tt075 · 2018-01-12T18:22:15Z

@Safihre @ymartin59 I have just realized that Syncthing is a package which used a start-stop-daemon prior to the conversion. If I'm not mistaken (and @Diaoul might be able to comment here) start-stop-daemon was used for all services, which cannot be toggled to run as daemon themselves. Do you think it makes sense to keep this? And if so, would you implement this on package or spksrc.service script level?

Safihre · 2018-01-12T19:41:51Z

Ah, I thought you did the same for tvheaded? With the special service_prestart?
https://github.com/SynoCommunity/spksrc/wiki/Service-Support#run-service-process-in-background

But that's indeed not really deamon, for example when the service restarts itself it will be shown as Stopped in the Package Center.

Safihre · 2018-01-12T19:43:24Z

I kinda just improvised, maybe the real start-stop-deamon is better idea!

m4tt075 · 2018-01-12T21:36:53Z

Tvheadend has a daemon option (-f), so I did not need it. The special service_prestart vodoo was needed to workaround permission issues. I only encountered the start-stop-daemon stuff when I started looking into Syncthing. I grepped the dsm-control files within spk and realized quite a lot of packages utilize this approach, namely:

deluge
fengoffice
homeassistant
horde
icecast
museek-plus
owncloud
radarr
rutorrent
selfoss
shairport-sync
sonarr
snycthing-inotify
syncthing
tt-rss

This is why I was asking...

Safihre · 2018-01-12T22:26:55Z

I found that sonarr and radarr actually do have a PID file option, but just place it in a custom location.

But indeed for other ones, maybe a general service provider would be great!

ymartin59 · 2018-01-13T21:31:10Z

If application produces PID_FILE and command is "fixed", then Makefile SERVICE_COMMAND is enough.
If command has to be built, or PID_FILE has to be created thanks shell... then service_prestart is the option I proposed.
In that case, sourcing options.conf in service_prestart

ymartin59 · 2018-01-13T21:35:19Z

In fact start-stop-daemon comes from cross/busybox... and as far as it can be replaced by shell background execution and echo PID in a file, I prefer that later option as it avoid to make package arch.

ymartin59 · 2018-01-13T21:36:58Z

Please copy wizard french translations from spk/demoservice

Diaoul · 2018-01-13T23:13:01Z

It is also possible to patch the app so it produces a pidfile as we need. This is to be preferred if easy to do. There were benefits using the daemon thing over a simple shell but I can't remember exactly. Go with whatever that works, just make sure to test special cases like when the app restart itself. Le 13 janv. 2018 10:36 PM, "Yves Martin" <notifications@github.com> a écrit : Please copy wizard french translations from spk/demoservice — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#3094 (comment)>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AATe9EexKQunqQ0_puevoeHlv1sNEDPEks5tKSH6gaJpZM4RbM70> .

Safihre · 2018-01-14T06:45:36Z

I will investigate if start-stop-deamon actually handles self-restarts of the app. The shell method does not, as the app is still running but shown as Stopped in DSM.
But if that's the same with start-stop-deamon then it doesn't really matter 😊

ymartin59 · 2018-01-14T22:22:37Z

To solve automatic restart after update for jackett, someone asks how to integrated into DSM upstart system: #3027. It may be an option but I fear about documentation and support in case of troubles... What do you think about it ?

Safihre · 2018-01-15T07:23:42Z

@ymartin59 Discussion in #3027 is interesting. It seems you are also right that the .conf scripts don't survive an update of DSM.
So doesn't seem a viable solution, with the update-rate of DSM 😢

http://majikshoe.blogspot.nl/2014/12/starting-service-on-synology-dsm-5.html

Safihre · 2018-01-31T23:09:35Z

While this would benefit from #3123, it actually needs start-stop-deamon as discussed before.
@ymartin59 Shall I manually include that in this package or will you add that to #3123 also?

ymartin59 · 2018-02-03T08:31:23Z

My idea is to create a clean / alternate start-stop-status script in generic support (there are already two of them https://github.com/SynoCommunity/spksrc/blob/master/mk/spksrc.service.mk#L120) with a Makefile switch to declare package requires start-stop-daemon

Safihre · 2018-02-03T16:17:02Z

Sounds good! 👍

Safihre · 2018-03-01T19:29:41Z

@cytec Can you help me explain native/go has this special rules for arm? Why does it need Go-1.4?
And do we need to also add this for arm64?
https://github.com/SynoCommunity/spksrc/blob/master/native/go/Makefile#L26

ymartin59

Just a remark. I will fix check so that STARTABLE can be removed

ymartin59 · 2018-03-01T21:09:12Z

spk/syncthing/src/service-setup.sh

+fi
+
+# Add special options to start-stop-daemon command
+SERVICE_OPTIONS = SYNCTHING_OPTIONS


This should not work. I propose to rename SYNCTHING_OPTIONS into SERVICE_OPTIONS

Indeed it doesn't work, in my test-branch I already updated that but just didn't push it yet.
But the name needs to stay SYNCTHING_OPTIONS because users could have setup some custom options during previous versions via ${CONFIG_DIR}/options.conf.
That options.conf is kept during the upgrade, so for backwards-compatibility it needs this.

#!/bin/sh # This file will be added to start-stop-status script # For possible options see syncthing --help # Example: uncomment this to start syncthing with all devices paused # SYNCTHING_OPTIONS="${SYNCTHING_OPTIONS} -paused"

OK. Have not read inside options.conf. So I wait for your review request.

cytec · 2018-03-02T09:41:37Z

@Safihre i try my best: go 1.4 is needed because if you install from source and want to build the toolchains you need a go compiler installed (go 1.4 was the last version with a compiler written in c) so you need to bootstrap it with go 1.4

The rules for arm and i386 are needed to get valid go compilers for those plafroms too... basically what you have to do is: export GOARCH and run ./make.bash for each arch you want to use... --no-clean just keeps everything and builds the new stuff on top instead of clearing all previous builds.

More info about all that can be found on the install go from source site

So if you want to enable ppc64 you'll have to add the same line with ppc64 same goes for arm64 and every other ARCH go is able to compile to (see https://golang.org/doc/install/source#environment)

ymartin59 · 2018-03-03T15:13:01Z

Really instructive... Sounds like wiki would be the right place for this information.

ymartin59 · 2018-03-03T15:14:51Z

By the way, qoric last PPC architecture cannot be deprecated as it still has DSM 6.2 support.

Safihre · 2018-03-25T14:46:01Z

Aah yes, all permissions and deamon stuff is merged now so I can finish this.

Safihre · 2018-03-29T06:52:38Z

@ymartin59 I need help 😨
I adapted the files to be like this:
Makefile
service-setup.sh

Which then creates the service-setup-file below in the package.
But it doesn't work, probably because SERVICE_EXE is defined before SYNCTHING variables etc.
How do I fix this?

### Package specific variables and functions
# Base service USER to run background process prefixed according to DSM
USER="syncthing"
PRIV_PREFIX=sc-
SYNOUSER_PREFIX=svc-
if [ ${SYNOPKG_DSM_VERSION_MAJOR} -lt 6 ]; then EFF_USER="${SYNOUSER_PREFIX}${USER}"; else EFF_USER="${PRIV_PREFIX}${USER}"; fi
# Service port
SERVICE_PORT="7070"
# start-stop-status script redirect stdout/stderr to LOG_FILE
LOG_FILE="${SYNOPKG_PKGDEST}/var/${SYNOPKG_PKGNAME}.log"
# Service command has to deliver its pid into PID_FILE
PID_FILE="${SYNOPKG_PKGDEST}/var/${SYNOPKG_PKGNAME}.pid"
# Service command to execute with start-stop-daemon
SERVICE_EXE="env HOME=${CONFIG_DIR} ${SYNCTHING}"
SERVICE_OPTIONS="${SYNCTHING_OPTIONS}"

# Invoke shell function if available
call_func ()
{
    FUNC=$1
    if type "$FUNC" | grep -q 'function' 2>/dev/null; then
        echo "Invoke $FUNC" >> ${INST_LOG}
        eval ${FUNC}
    fi
}

# Setup environment
PATH="${SYNOPKG_PKGDEST}/bin:${PATH}"
SYNCTHING="${SYNOPKG_PKGDEST}/bin/syncthing"
CONFIG_DIR="${SYNOPKG_PKGDEST}/var"
SYNCTHING_OPTIONS="-home=${CONFIG_DIR}"

# Read additional startup options from /usr/local/syncthing/var/options.conf
if [ -f ${CONFIG_DIR}/options.conf ]; then
    source ${CONFIG_DIR}/options.conf
fi

LEGACY_GROUP="users"

service_postinst ()
{
    # Add also to "users" group in case it was there
    # This way it keeps any permissions it used to have
    syno_user_add_to_legacy_group "${EFF_USER}" "${USER}" "${LEGACY_GROUP}"

    # Discard legacy obsolete busybox user account
    BIN=${SYNOPKG_PKGDEST}/bin
    $BIN/busybox --install $BIN >> ${INST_LOG}
    $BIN/delgroup "${USER}" "users" >> ${INST_LOG}
    $BIN/deluser "${USER}" >> ${INST_LOG}
}

ymartin59 · 2018-03-30T05:48:19Z

@Safihre Please push your recent changes, it will be easier for me to understand.
I propose you set everything once from spk Makefile:

SERVICE_EXE="env HOME=$${SYNOPKG_PKGDEST}/var $${SYNOPKG_PKGDEST}/bin/syncthing"
SERVICE_OPTIONS="-home=$${SYNOPKG_PKGDEST}/var"

Cheers

Safihre · 2018-06-01T05:10:56Z

I don't think this is ready yet, never got it to work properly. That's why this was the only one I didn't put it in the checklist at the end of #3138.
Will try to do some work on it today to finish it!

Added wizard page explaining Syncthing permissions Also add new user to "users" group in case it was there, just to be sure

Safihre · 2018-06-02T10:47:09Z

@ymartin59 I pushed some updates, now the command is successfully passed by using a dummy that I then later overwrite in service-setup.sh
But it still won't start and I'm not sure why. Maybe you can take a look?

admin@DiskStation:/$ cat /volume1/@appstore/syncthing/var/syncthing.log                                                 
Sat Jun  2 12:39:32 CEST 2018
Starting syncthing command env HOME=/volume1/@appstore/syncthing/var /volume1/@appstore/syncthing/bin/syncthing -home=/volume1/@appstore/syncthing/var

ymartin59 · 2018-06-03T20:10:13Z

@Safihre From my point of view, there is no need for "dummy" as command and options are not dynamic. Setting these in Makefile instead of dummy should be exactly equivalent:

SERVICE_EXE = "env HOME=$${SYNOPKG_PKGDEST}/var $${SYNOPKG_PKGDEST}/bin/syncthing"
SERVICE_OPTIONS = "-home=$${SYNOPKG_PKGDEST}/var"

I will have to grab your branch to diagnose...

Safihre · 2018-06-03T20:21:56Z

I tried that, but that doesn't work, it results in:

SERVICE_EXE = ""env HOME=$${SYNOPKG_PKGDEST}/var $${SYNOPKG_PKGDEST}/bin/syncthing""

Because the .mk processing adds extra " when writing the files.

Safihre · 2018-06-03T20:24:38Z

Also still need to add SYNCTHING_OPTIONS that users can manually define in options.conf.

I am not good in Shell/Bash so maybe you know the fix :)

ymartin59 · 2018-06-03T20:25:19Z

@Safihre OK I will give it a try soon...

ymartin59 · 2018-07-18T20:17:30Z

@Safihre I am in trouble with start-stop-daemon support... it works properly from command line but not from Package Center. Still investigating...

ymartin59 · 2018-07-30T21:45:21Z

Superseed by #3391

Safihre force-pushed the just-syncthing branch from adc3cc4 to 1a98e35 Compare January 12, 2018 08:21

m4tt075 mentioned this pull request Jan 12, 2018

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

Closed

ymartin59 added the status/work-in-progress label Jan 13, 2018

Safihre added status/needs-review and removed status/work-in-progress labels Feb 19, 2018

Safihre force-pushed the just-syncthing branch from 1a98e35 to d13a324 Compare February 28, 2018 09:29

Safihre changed the title ~~Syncthing - Update 0.14.43 - Generic Service approach~~ Syncthing - Update 0.14.44 - Generic Service approach Feb 28, 2018

Safihre mentioned this pull request Feb 28, 2018

Service Support: add background command execution #3123

Merged

Safihre force-pushed the just-syncthing branch from d13a324 to 58d83e8 Compare February 28, 2018 09:33

Safihre mentioned this pull request Mar 1, 2018

Correct permissions of Sonarr/Radarr backup upon upgrade #3190

Merged

3 tasks

ymartin59 reviewed Mar 1, 2018

View reviewed changes

Safihre force-pushed the just-syncthing branch from 58d83e8 to 34ae391 Compare March 25, 2018 15:24

Safihre force-pushed the just-syncthing branch from 34ae391 to 6947ba2 Compare March 30, 2018 10:25

ymartin59 approved these changes May 31, 2018

View reviewed changes

ymartin59 added status/ready-to-merge status/to-publish translation and removed status/needs-review labels May 31, 2018

ymartin59 added status/work-in-progress and removed status/ready-to-merge labels Jun 1, 2018

Safihre force-pushed the just-syncthing branch from 6947ba2 to c72ca32 Compare June 2, 2018 09:54

Syncthing - Update 0.14.44 - Generic Service approach

daf23d9

Added wizard page explaining Syncthing permissions Also add new user to "users" group in case it was there, just to be sure

Safihre force-pushed the just-syncthing branch from 1939cf6 to daf23d9 Compare June 2, 2018 10:44

This was referenced Jul 19, 2018

framework: fix busybox start-stop-daemon support for service #3390

Merged

Fix syncthing startup and use default webui port #3391

Merged

ymartin59 removed status/to-publish status/work-in-progress translation labels Jul 30, 2018

ymartin59 closed this Jul 30, 2018

Safihre deleted the just-syncthing branch March 30, 2021 10:24

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Syncthing - Update 0.14.44 - Generic Service approach #3094

Syncthing - Update 0.14.44 - Generic Service approach #3094

Safihre commented Jan 11, 2018

m4tt075 commented Jan 12, 2018

Safihre commented Jan 12, 2018

Safihre commented Jan 12, 2018

m4tt075 commented Jan 12, 2018

Safihre commented Jan 12, 2018

ymartin59 commented Jan 13, 2018

ymartin59 commented Jan 13, 2018

ymartin59 commented Jan 13, 2018

Diaoul commented Jan 13, 2018 via email

Safihre commented Jan 14, 2018

ymartin59 commented Jan 14, 2018

Safihre commented Jan 15, 2018

Safihre commented Jan 31, 2018

ymartin59 commented Feb 3, 2018

Safihre commented Feb 3, 2018

Safihre commented Mar 1, 2018

ymartin59 left a comment

ymartin59 Mar 1, 2018

Safihre Mar 1, 2018

ymartin59 Mar 1, 2018

cytec commented Mar 2, 2018

ymartin59 commented Mar 3, 2018

ymartin59 commented Mar 3, 2018 •

edited

Loading

Safihre commented Mar 25, 2018

Safihre commented Mar 29, 2018

ymartin59 commented Mar 30, 2018

Safihre commented Jun 1, 2018

Safihre commented Jun 2, 2018

ymartin59 commented Jun 3, 2018

Safihre commented Jun 3, 2018

Safihre commented Jun 3, 2018

ymartin59 commented Jun 3, 2018

ymartin59 commented Jul 18, 2018

ymartin59 commented Jul 30, 2018

Syncthing - Update 0.14.44 - Generic Service approach #3094

Syncthing - Update 0.14.44 - Generic Service approach #3094

Conversation

Safihre commented Jan 11, 2018

Checklist

m4tt075 commented Jan 12, 2018

Safihre commented Jan 12, 2018

Safihre commented Jan 12, 2018

m4tt075 commented Jan 12, 2018

Safihre commented Jan 12, 2018

ymartin59 commented Jan 13, 2018

ymartin59 commented Jan 13, 2018

ymartin59 commented Jan 13, 2018

Diaoul commented Jan 13, 2018 via email

Safihre commented Jan 14, 2018

ymartin59 commented Jan 14, 2018

Safihre commented Jan 15, 2018

Safihre commented Jan 31, 2018

ymartin59 commented Feb 3, 2018

Safihre commented Feb 3, 2018

Safihre commented Mar 1, 2018

ymartin59 left a comment

Choose a reason for hiding this comment

ymartin59 Mar 1, 2018

Choose a reason for hiding this comment

Safihre Mar 1, 2018

Choose a reason for hiding this comment

ymartin59 Mar 1, 2018

Choose a reason for hiding this comment

cytec commented Mar 2, 2018

ymartin59 commented Mar 3, 2018

ymartin59 commented Mar 3, 2018 • edited Loading

Safihre commented Mar 25, 2018

Safihre commented Mar 29, 2018

ymartin59 commented Mar 30, 2018

Safihre commented Jun 1, 2018

Safihre commented Jun 2, 2018

ymartin59 commented Jun 3, 2018

Safihre commented Jun 3, 2018

Safihre commented Jun 3, 2018

ymartin59 commented Jun 3, 2018

ymartin59 commented Jul 18, 2018

ymartin59 commented Jul 30, 2018

ymartin59 commented Mar 3, 2018 •

edited

Loading