synapse: minor changes.

[freshprince]

Don't patch sample config.

Make run script configurable so that admin can change path to config file and set locale if necessary.

Create /etc/synapse again.

Log to daemon.notice. Logging daemon is not really necessary for this.

Testing the changes

• I tested the changes in this PR: YES

[TinfoilSubmarine]

Hi, just saw this PR. I've been maintaining the package and added the /usr/bin/vlogger -> /etc/sv/synapse/log/run symlink. Was curious what benefit directly execing logger has? I have this running on my system and synapse is already logging to daemon.notice.

The configurable config file location looks good to me.

For patching config files, I'd like to have the config file updates installed so that e.g. xdiff from xtools can be used to easily manage updates/changes to the synapse configuration. Currently that's waiting on void-linux/xbps#426, then hopefully we can install to /etc/synapse instead of vsconf.

[TinfoilSubmarine]

Should also mention that #34682 has already been merged.

[freshprince]

Was curious what benefit directly execing logger has? I have this running on my system and synapse is already logging to daemon.notice.

I was used to see error output in the logging space of runsvdir, when I saw that there was a logging service running for synapse there was no easy way for me to see where it is logging to. All other services I use do have explicitly set where they are logging to and in this case explicit is better than implicit because if you want to check logs you shouldn't need to know what's the default behaviour of vlogger.

For patching config files, I'd like to have the config file updates installed so that e.g. xdiff from xtools can be used to easily manage updates/changes to the synapse configuration. Currently that's waiting on void-linux/xbps#426, then hopefully we can install to /etc/synapse instead of vsconf.

If you read the first three paragraphs of the config file that you want to install you see that this config file is not intended to be copied into /etc/synapse, patched or otherwise. The config does contain several secrets that should be randomly generated and for this the correct and documented way is to use generate_config. Regardless of the behaviour of xbps that resulted in void-linux/xbps#441.

Please test your patches before you create pull requests.

[TinfoilSubmarine]

explicit is better than implicit because if you want to check logs you shouldn't need to know what's the default behaviour of vlogger

We'll need to have a Void team member weigh in on this because the history of the logging service (introduced in #31263) was what made me start using vlogger instead of the older exec logger ... pattern.

If you read the first three paragraphs of the config file that you want to install you see that this config file is not intended to be copied into /etc/synapse, patched or otherwise. The config does contain several secrets that should be randomly generated and for this the correct and documented way is to use generate_config

I compared the output of generate_config with the patched sample_config.yaml and the only difference was that the header was included and the log_config location differed slightly. You are correct that this is confusing, so likely these paragraphs should be patched out as well. As far as secret generation, that is optional for having a working server, I specifically tested this. The comments above the three secrets that would be generated (registration_shared_secret, macaroon_secret_key, and form_secret) indicate that these are either optional or only needed for advanced functionality such as consent forms.

[freshprince]

explicit is better than implicit because if you want to check logs you
shouldn't need to know what's the default behaviour of vlogger

We'll need to have a Void team member weigh in on this because the history of
the logging service (introduced in #31263) was what made me start using
vlogger instead of the older exec logger ... pattern.

For synapse, logging of stderr doesn't really make sense anyway since it only
produces output when there's a problem with the configuration.

You are correct that this is confusing, so likely these paragraphs should be
patched out as well.

Sure you can ignore what the authors of the software and people that use it
tell you and do it anyway.

these are either optional or only needed for advanced functionality

If registering a user in the command line (which you do when you follow the
installation instructions) is advanced functionality for you.

I've rearranged my patches. Feel free to do with them as you please. I'll just

echo -e "preserve=/etc/synapse/*\npreserve=/etc/sv/synapse/*" >> /etc/xbps.d/synapse.conf

and be done with it.

[TinfoilSubmarine]

For synapse, logging of stderr doesn't really make sense anyway since it only
produces output when there's a problem with the configuration.

Being able to see those configuration problems raised in syslog makes debugging easier.

Sure you can ignore what the authors of the software and people that use it
tell you and do it anyway.

The sample_config.yaml file is generated by running generate_config --header-file docs/.sample_config_header.yaml -o docs/sample_config.yaml, so this is just removing the header of an otherwise valid config file.

If registering a user in the command line (which you do when you follow the
installation instructions) is advanced functionality for you.

I missed this when I was testing, thanks for pointing it out. I was struggling to find a "beginner" use-case that would be a blocker requiring that the secrets be generated and couldn't come up with one.

However, this isn't necessarily a bad thing. The Debian package maintained by the authors does the same thing -- they generate a default config without generated secrets and the user must manually supply them, see https://github.com/matrix-org/synapse/blob/develop/debian/build_virtualenv.

[freshprince]

Being able to see those configuration problems raised in syslog makes debugging easier.

Syslog is not installed by default on void so you won't see the problems raised anywhere. If you think that installing socklog-void and knowing where vlogger logs to by default is easier than using ps and pgrep runsvdir | xargs -I{} cat /proc/{}/cmdline you should consider reading up on runit. Which is the init system that void is using.

The Debian package maintained by the authors does the same thing

That's a nice form of cargo-cult that begs for the statement that if you like how it's done in distribution X maybe you should switch to distribution X. If I remember correctly void is trying to provide packages as 'vanilla' as possible.

If one seriously wants to set up a synapse homeserver, they have to install synapse, get valid ssl certificates, set up a reverse proxy, install postgresql and set it up, add well-known files for clients and federation, have a working e-mail setup, set up a TURN server and start running different worker processes to do load balancing. I don't think that copying sample config files so that the service starts without failing after a fresh install helps a beginner doing any of that. But it might have wrong defaults like the name of the homeserver (which makes the generated sqlite-db unusable if you want to switch the name later) and missing randomly generated secrets as already mentioned. It also introduced problems for people that were already having a working setup.

[TinfoilSubmarine]

Syslog is not installed by default on void so you won't see the problems raised anywhere. If you think that installing socklog-void and knowing where vlogger logs to by default is easier than using ps and pgrep runsvdir | xargs -I{} cat /proc/{}/cmdline you should consider reading up on runit. Which is the init system that void is using.

Not trying to strawman here, but by this logic, it seems like no service in Void should be shipping a $service/log since we can just dig logs out of /proc? I'm legitimately curious what constitutes a valid reason for adding a logging service then.

That's a nice form of cargo-cult that begs for the statement that if you like how it's done in distribution X maybe you should switch to distribution X. If I remember correctly void is trying to provide packages as 'vanilla' as possible.

I wasn't really pointing to "how Debian does it" as much as "the authors are using this approach as a way to ship their own software so maybe we can learn from it", but you are right that this may be a too vendored approach for Void.

If one seriously wants to set up a synapse homeserver, they have to install synapse, get valid ssl certificates, set up a reverse proxy, install postgresql and set it up, add well-known files for clients and federation, have a working e-mail setup, set up a TURN server and start running different worker processes to do load balancing. I don't think that copying sample config files so that the service starts without failing after a fresh install helps a beginner doing any of that. But it might have wrong defaults like the name of the homeserver (which makes the generated sqlite-db unusable if you want to switch the name later) and missing randomly generated secrets as already mentioned. It also introduced problems for people that were already having a working setup.

I agree that making a few changes to a default config to get the synapse service running is not representative of the amount of work needed to actually stand-up a feature-complete synapse from scratch. The original reason I attempted to have a default config in place was to remove the existing INSTALL telling the user what to do, since that vendors setup instructions/info. Indeed, there's a lot more that the user starting from scratch will need to do than just run:

sudo -u synapse python3 -m synapse.app.homeserver \
      --server-name my.domain.name \
      --config-path /etc/synapse/homeserver.yaml \
      --generate-config \
      --report-stats=yes

which is all that INSTALL was telling new users to do. But, including this in INSTALL may be more in line with upstream documentation, since the upstream installation instructions don't have an explicit step telling the user to generate their config file except if following the PyPI/virtualenv instructions. And this page on configuration just says that the configuration should have been generated when you installed synapse.

[freshprince]

but by this logic, it seems like no service in Void should be shipping a $service/log since we can just dig logs out of /proc? I'm legitimately curious what constitutes a valid reason for adding a logging service then.

synapse has it's own logging facilities. By default it logs to a logfile in /var/log/synapse. It can log to syslog or stdout too if the user wants that and configures it to. synapse only spews stuff out to stderr when there is a problem with the configuration and the service fails to start of if you explicitly tell it to.

[TinfoilSubmarine]

It can log to syslog

Oh, I didn't realize logging.handlers.SysLogHandler existed, TIL, thanks!

[TinfoilSubmarine]

Anyways, I'm OK with these commits. I think they will need to be squashed into one commit, or I can add them to the next version.

[freshprince]

Superseded by #35109.