syncthing: Upgrade to v1.15.1 and ignore folder permissions by default. #4527

acolomb · 2021-03-28T22:01:18Z

Motivation: Upstream v1.14.0 was released on 2021-03-02. Some option defaults are changed in this PR, which only applies for new installations, because config.xml is (and must be) kept during updates.

The new version introduces a "folder defaults" section in the configuration, which is used to preset the ignorePerms option to true. This will not touch any existing folders, but set the option when configuring a new folder to be synced. Specifically on DSM, the ignorePerms option should definitely be used with every Syncthing folder. Otherwise, it will possibly try to override the UNIX permissions on synced files, which will reset the ACL-based permissions. Possibly making them unusable for the rest of the DSM services.

Change the default for the cacheIgnoredFiles option to false (which is the upstream default). Many DiskStations are rather tight on RAM, so deviating from Syncthing's default here is probably a bad idea for most users. It only matters when many (or very complex) ignore patterns are used in a folder, so usually for power users. Those will certainly be able to change the option manually.

Drop the obsolete symlinksEnabled and weakHashSelectionMethod entries. Switch from minHomeDiskFreePct to the new syntax minHomeDiskFree unit="%".

Note that the previous package version should have already self-updated an installed Syncthing binary when the service was running long enough. So this PR should not really change anything for users updating the SPK.

Linked issues:
Configurable folder defaults: syncthing/syncthing#7131
symlinksEnabled: syncthing/docs@392dbd0b
weakHashSelectionMethod: syncthing/syncthing#4767

Checklist

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

(Sorry, I have no DSM installation to spare where Syncthing is not used in production, so testing is limited. Should be low impact though as mentioned above.)

Only package version and hashes updated. Built successfully for all-supported. Note that the previous package version should have already self-updated an installed Syncthing binary when the service was running long enough.

Utilize the newly introduced "folder default configuration" [1] feature to preset the ignorePerms option to true. This will not touch any existing folders, but set the option when configuring a new folder to be synced. Specifically on DSM, the ignorePerms option should definitely be used with every Syncthing folder. Otherwise, it will possibly try to override the UNIX permissions on synced files, which will reset the ACL-based permissions on the content. Possibly making them unusable for the rest of the DSM services. Note that this also only applies for new installations, because config.xml is (and must be) kept during updates. [1]: syncthing/syncthing#7131

Change the default for the cacheIgnoredFiles option to false. From the Syncthing documentation [1]: Whether to cache the results of ignore pattern evaluation. Performance at the price of memory. Defaults to false as the cost for evaluating ignores is usually not significant. The DiskStations are usually rather tight on RAM, so deviating from Syncthing's default here is probably a bad idea for most users. It only matters when many (or very complex) ignore patterns are used in a folder, so usually for power users. Those will certainly be able to change the option manually. Note that this also only applies for new installations, because config.xml is (and must be) kept during updates. [1]: https://docs.syncthing.net/users/config.html

Drop the symlinksEnabled and weakHashSelectionMethod entries from the default config file template. See [1] and [2]. Switch from the minHomeDiskFreePct option to the new syntax minHomeDiskFree unit="%". Note that this also only applies for new installations, because config.xml is (and must be) kept during updates. Syncthing migrates old configuration formats on its own. [1]: syncthing/docs@392dbd0b [2]: syncthing/syncthing#4767

acolomb · 2021-03-28T22:13:01Z

A side note about my work on spksrc: I've been involved with the last few updates to the Syncthing package here and tried to answer any related support requests. For that purpose, I have been following the spksrc repository on GitHub and received every notification. That is just becoming too much to handle, just for trying to filter what's related to this package.

So I'm planning to stop those notifications. If there are any further questions regarding Syncthing on DSM beyond this PR, feel free to mention me (@acolomb) on the issue and I will look into it. I want to stay involved with future PRs and support, just not get spammed with tens of e-mails a day about packages I never use.

publicarray · 2021-03-29T12:16:36Z

@acolomb Looks good, I would consider removing busybox from this package and start preparing for DSM7, https://github.com/SynoCommunity/spksrc/wiki/DSM-7.0-Beta

DSM 7 Build logs:

  ERROR: SERVICE_EXE (start-stop-daemon) is unsupported in DSM7
  Please migrate to SERVICE_COMMAND=
  SVC_BACKGROUND=y
  SVC_WRITE_PID=y

Please let me know if you need guidance.

Regarding too many messages, Thanks for supporting the package. I understand and do the same thing. I visit and filter issue tracker on occasion as well. I hope I remember to ping you on related issues. Maybe add yourself as the package MAINTAINER in the makefile ? I don't think @Safihre would mind.

Edit. Usually my process (if I can't fix/diagnose a package issue myself) involves looking at the maintainer field in the makefile to ask for assistance. Then git history if I have to.

Safihre · 2021-03-29T12:27:14Z

I don't mind 🥳

acolomb · 2021-03-29T12:28:13Z

Please let me know if you need guidance.

I saw the warning about SERVICE_EXE, but decided not to touch this yet. I'd rather get this update out the door quickly, then focus on the DSM7 stuff in a separate effort. Are there any similar packages I could use as reference for the transition? Or a Wiki page describing the different requirements? Given my limited time and testing capacity right now, I'd rather not delay this update by a quest to make the DSM7 stuff work.

publicarray · 2021-03-29T12:43:08Z

I understand the desire for a seperate PR and that's fine. It's just from my understanding that every release eats up diskspace on our server. So we don't like releases in quick sucession at the moment. I still leave it up to you though.

There are sample packages in https://github.com/SynoCommunity/spksrc/commits/dsm7-packages (I suggest to look at the Transmission package). I posted the DSM7-beta wiki link above. You may find Generic Service-Support and Variables in SPK Makefiles helpful too.

I would help test for DSM7 but my NAS died (won't turn on anymore), waiting to get it replaced 😢

acolomb · 2021-03-30T18:56:57Z

I really don't see a chance on my end to test anything related to DSM7 at the moment. It will be some time before I even think about upgrading one of my NASes to that.

So I can investigate the needed changes, but not really go through with it. On the other hand, this PR fixes a common pitfall with the Syncthing package on DSM (not enabling ignorePerms for a folder), which is only made possible with the recent upstream release. So please accept this fix and update first. I'll look into the DSM7 stuff if no one beats me to it as part of a larger package DSM7 refactoring.

publicarray · 2021-03-31T08:16:45Z

Ok no problem. Would you still consider removing busybox in this update?

acolomb · 2021-03-31T08:19:50Z

Ok no problem. Would you still consider removing busybox in this update?

I thought it was still needed for DSM6?

publicarray · 2021-03-31T08:21:45Z

No it was needed for DSM5 but since then we have a generic script - service that should work for all 3 versions or at least better in DSM6 and DSM7

Edit: I might be wrong about the history but going forward I strongly recommend ditching busybox

Required to prepare for DSM 7.0 support

publicarray · 2021-04-03T02:20:33Z

@acolomb I've made some changes would you mind telling me if you think they are OK?

Technically it should still run in DSM5 but the user migration script to DSM6 is removed. (I think users still on DSM5 would have upgraded by now if there are any users left on DSM5)

acolomb · 2021-04-04T19:23:42Z

Sorry I tried to build with your changes applied, but cannot even get the build to start anymore :-(

root@barnov:/spksrc/spk/syncthing# make all-supported
===>  Pre-build native dependencies for parallel build
make supported-arch-88f6281 supported-arch-aarch64 supported-arch-armv7 supported-arch-evansport supported-arch-hi3535 supported-arch-qoriq supported-arch-x64
make[1]: Entering directory '/spksrc/spk/syncthing'
===>  BUILDING package for arch 88f6281 with SynoCommunity toolchain
make[2]: Entering directory '/spksrc/spk/syncthing'
/spksrc/spk/syncthing/work-88f6281-6.1/tc_vars.mk:1: *** An error occured while setting up the toolchain, please check the messages above.  Stop.
make[2]: Leaving directory '/spksrc/spk/syncthing'
make[1]: [../../mk/spksrc.spk.mk:430: supported-arch-88f6281] Error 2 (ignored)
===>  BUILDING package for arch aarch64 with SynoCommunity toolchain
make[2]: Entering directory '/spksrc/spk/syncthing'
/spksrc/spk/syncthing/work-aarch64-6.1/tc_vars.mk:1: *** An error occured while setting up the toolchain, please check the messages above.  Stop.
make[2]: Leaving directory '/spksrc/spk/syncthing'
make[1]: [../../mk/spksrc.spk.mk:430: supported-arch-aarch64] Error 2 (ignored)

... and so on for all supported archs.

I've pulled the latest Docker image, but still no luck.

acolomb · 2021-04-04T19:38:07Z

Sorry, a manual rm -rf work-* solved the build problems.

acolomb · 2021-04-04T20:34:10Z

Installation and upgrade worked on my DS418j (rtd1296 / aarch64) with DSM 6.2.3-25426 Update 3. Unfortunately I can't help test any DSM7 changes yet.

So the busybox removal looks good to go I guess.

publicarray · 2021-04-04T20:57:48Z

Thanks @acolomb! FYI the make clean command does the same thing (usually good idea to rebuild when there are changes in Makefiles or cross/{package}).

I'm waiting on #4539 and #4539 before I merge any PRs at the moment, since they fix issues with the framework that where introduced when the DSM7 branch was merged.

Only package version and hashes updated. Built successfully for all-supported. Note that the previous package version should have already self-updated an installed Syncthing binary when the service was running long enough.

acolomb · 2021-04-08T16:05:20Z

Syncthing v1.15.1 was released in the meantime, so I updated this PR. Otherwise it would have updated itself right after installation anyway. Regarding the packaging, nothing else changed.

hgy59 · 2021-04-11T22:43:01Z

as #4539 is now on the master branch, you have to remove >> ${INST_LOG} in service-setup.sh for service_postinst to work.

* redesign installer log file - create install_log function for DSM version independent installation logs - enhance log function to work with multi-line piped output - prefix all log lines with timestamp - avoid usage of INST_LOG in spksrc/mk files - apply the new installer log in spk/python/src/service-setup.sh - fix dsm7 installer to copy configurations to permanent storage * write all output of specific installer functions to installer log - capture output of call of service_* function in installer log - use different call_func functions for start-stop-status and installer as start-stop-status must write to service-log - adjust python2/src/service-setup.sh for this new approach - introduce validate_preinst, validate_preupgrade and validate_preuninst for service specific installers that must provide error notification. - remove the service_*_links calls in dsm7 installer as dsm7 supports only configuration by resource linkers - uninstall: avoid remove of ${SYNOPKG_PKGTMP} in dsm7 as it is deleted by the system before postuninst * update installer logs - only the "global generict functions" (preinst, postinst, ...) should redirect to installer_log - all other functions use echo or command execution * replace INST_VAR by SYNOPKG_PKGVAR - define SYNOPKG_PKGVAR for compatibility with DSM7 * Framework DSM7: fix firewall installation * DSM7 fix firewall rule * optimize call_func * DSM7 fix firewall rule Since 6.0-5936 the port-config resource worker .sc file's relative path is under /var/package/{$package}/target/ * use `/var/log/packages/{package-name}.log` for DSM 6 too * add dedicated noarch build for TCVERSION>=6.0 - to use DSM6 installer log we need different noarch packages for DSM<6 and DSM>=6 * installer cleanup - ignore resource linker for evaluation of STARTABLE - use 'else if' to simplify nesting in makefile - add begin/end to installer log for custom service functions - add begin/end to service log for custom functions - add INST_LOG variable to dsm6 and dsm7 installer for reference only * update demoservice - update for installer log - make independent of default python (2 or 3) - fix service_postuninst - add examples for validate functions * update packages for installer log and to avoid use of /usr/local/{package-name} - mono - python3 and python38 - synocli-monitor - beets - couchpotato - dnscrypt-proxy - borgbackup - fishnet - flexget - gateone - headphones - homeassistant - itools - lazylibrarian - lidarr - minio - mosquitto - mutt - mylar - nzbget - nzbhydra - plexpy-custom - pyload - rabbitmq - rdiff-backup - rsnapshot - rutorrent - salt-minion - sickbeard-custom and sickrage - sonarr - stockfish - umurmur * Update spk/dnscrypt-proxy/src/service-setup.sh Co-authored-by: Sebastian Schmidt <[email protected]> * adjust nzbget installation - make wget less verbose to avoid download progress in log file * log full install log name to service log - dsm5: rename INST_LOG to INST_LOG_TEMP for unified INST_LOG Co-authored-by: Sebastian Schmidt <[email protected]>

publicarray · 2021-04-15T04:19:53Z

Thanks @acolomb 👍 you should see it in the package center soon!

guidorb79 · 2021-04-18T23:16:36Z

syncthing 1.15.1-21 installed right now on dsm7 from synocommunity repo thank you all!!!!

DigitalBox98 · 2021-04-20T14:13:37Z

Well done :)

acolomb added 4 commits March 28, 2021 23:14

syncthing: Incorporate upstream release v1.14.0.

52e9ad5

Only package version and hashes updated. Built successfully for all-supported. Note that the previous package version should have already self-updated an installed Syncthing binary when the service was running long enough.

syncthing: Add myself (@acolomb) as maintainer.

5370217

Syncthing: Drop busybox and DSM 5 support

574f8e2

Required to prepare for DSM 7.0 support

publicarray added 2 commits April 3, 2021 13:43

Syncthing: fixes

e7aeccd

Framework: log the actual command after service_prestart()

4c97688

publicarray mentioned this pull request Apr 3, 2021

Meta: DSM7 package status #4524

Open

publicarray added the status/ready-to-merge label Apr 3, 2021

publicarray mentioned this pull request Apr 8, 2021

redesign installer logs #4539

Merged

2 tasks

syncthing: Incorporate upstream release v1.15.1.

6eb9ce7

Only package version and hashes updated. Built successfully for all-supported. Note that the previous package version should have already self-updated an installed Syncthing binary when the service was running long enough.

acolomb changed the title ~~syncthing: Upgrade to v1.14.0 and ignore folder permissions by default.~~ syncthing: Upgrade to v1.15.1 and ignore folder permissions by default. Apr 13, 2021

publicarray merged commit 2f93f5c into SynoCommunity:master Apr 15, 2021

publicarray added status/published Published and activated (may take up to 48h until visible in DSM package manager) and removed status/ready-to-merge labels Apr 15, 2021

acolomb deleted the syncthing-1.14-ignorePerms branch April 19, 2021 20:56

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

syncthing: Upgrade to v1.15.1 and ignore folder permissions by default. #4527

syncthing: Upgrade to v1.15.1 and ignore folder permissions by default. #4527

acolomb commented Mar 28, 2021 •

edited

Loading

acolomb commented Mar 28, 2021

publicarray commented Mar 29, 2021 •

edited

Loading

Safihre commented Mar 29, 2021

acolomb commented Mar 29, 2021

publicarray commented Mar 29, 2021 •

edited

Loading

acolomb commented Mar 30, 2021 •

edited

Loading

publicarray commented Mar 31, 2021

acolomb commented Mar 31, 2021

publicarray commented Mar 31, 2021 •

edited

Loading

publicarray commented Apr 3, 2021 •

edited

Loading

acolomb commented Apr 4, 2021

acolomb commented Apr 4, 2021

acolomb commented Apr 4, 2021 •

edited

Loading

publicarray commented Apr 4, 2021 •

edited

Loading

acolomb commented Apr 8, 2021

hgy59 commented Apr 11, 2021

publicarray commented Apr 15, 2021

guidorb79 commented Apr 18, 2021

DigitalBox98 commented Apr 20, 2021

syncthing: Upgrade to v1.15.1 and ignore folder permissions by default. #4527

syncthing: Upgrade to v1.15.1 and ignore folder permissions by default. #4527

Conversation

acolomb commented Mar 28, 2021 • edited Loading

Checklist

acolomb commented Mar 28, 2021

publicarray commented Mar 29, 2021 • edited Loading

Safihre commented Mar 29, 2021

acolomb commented Mar 29, 2021

publicarray commented Mar 29, 2021 • edited Loading

acolomb commented Mar 30, 2021 • edited Loading

publicarray commented Mar 31, 2021

acolomb commented Mar 31, 2021

publicarray commented Mar 31, 2021 • edited Loading

publicarray commented Apr 3, 2021 • edited Loading

acolomb commented Apr 4, 2021

acolomb commented Apr 4, 2021

acolomb commented Apr 4, 2021 • edited Loading

publicarray commented Apr 4, 2021 • edited Loading

acolomb commented Apr 8, 2021

hgy59 commented Apr 11, 2021

publicarray commented Apr 15, 2021

guidorb79 commented Apr 18, 2021

DigitalBox98 commented Apr 20, 2021

acolomb commented Mar 28, 2021 •

edited

Loading

publicarray commented Mar 29, 2021 •

edited

Loading

publicarray commented Mar 29, 2021 •

edited

Loading

acolomb commented Mar 30, 2021 •

edited

Loading

publicarray commented Mar 31, 2021 •

edited

Loading

publicarray commented Apr 3, 2021 •

edited

Loading

acolomb commented Apr 4, 2021 •

edited

Loading

publicarray commented Apr 4, 2021 •

edited

Loading