First of all, do you mind if I cherry-pick the important parts to avoid adding a bunch of ~rcs to the changelog (as well as other changelog conflicts)? I also already submitted an update for 1.7.1 about two hours ago.
I'm not sure I understand why the patch you've added is needed -- if we ship a default configuration file which uses /etc/matrix-synapse/... rather than /etc/synapse then it shouldn't be necessary to disable the configuration of those options within synapse itself. After all, users may wish to change it and store their configs and keys elsewhere -- and if you patch out the ability to configure them with hard-coded paths it will probably lead to confusing bug reports.
I do like the idea of having a basic "generate a config" script and removing that code from the service file.
I have mixed feelings about BuildRequiring all of the dependencies. It will fail builds if the dependencies aren't available, but we already get that information from matrix-synapse-test. Maybe it'll stop people from installing the package anyway, but it will also trigger rebuilds of the matrix-synapse package each time a dependency changes. But, if you feel it's a good idea I'd be happy to include it.
psutil is no longer required, they dropped the dependency a few versions ago.
Is it necessary to have a separate usermod in %post -- why not modify the useradd line?
Is it okay to use "python3" directly and drop that aspect of the python-packaging-related code? It was my understanding that Python packages are magically rewritten and we should keep using their packaging setup (even if we only support a single version). On that note, why Provides/Obsoletes python3-matrix-synapse (and dropping the Provides matrix-synapse)? Isn't the real name of the package python3-matrix-synapse and we need a Provides so users can install "matrix-synapse"?
Most of the %install and %build code seems reasonable.
All-in-all, I would like to see most of this get merged but I'm unclear on a few remaining topics.
Copying my comment from sr#757829
A few comments:
First of all, do you mind if I cherry-pick the important parts to avoid adding a bunch of ~rcs to the changelog (as well as other changelog conflicts)? I also already submitted an update for 1.7.1 about two hours ago.
I'm not sure I understand why the patch you've added is needed -- if we ship a default configuration file which uses /etc/matrix-synapse/... rather than /etc/synapse then it shouldn't be necessary to disable the configuration of those options within synapse itself. After all, users may wish to change it and store their configs and keys elsewhere -- and if you patch out the ability to configure them with hard-coded paths it will probably lead to confusing bug reports.
I do like the idea of having a basic "generate a config" script and removing that code from the service file.
I have mixed feelings about BuildRequiring all of the dependencies. It will fail builds if the dependencies aren't available, but we already get that information from matrix-synapse-test. Maybe it'll stop people from installing the package anyway, but it will also trigger rebuilds of the matrix-synapse package each time a dependency changes. But, if you feel it's a good idea I'd be happy to include it.
psutil is no longer required, they dropped the dependency a few versions ago.
Is it necessary to have a separate usermod in %post -- why not modify the useradd line?
Is it okay to use "python3" directly and drop that aspect of the python-packaging-related code? It was my understanding that Python packages are magically rewritten and we should keep using their packaging setup (even if we only support a single version). On that note, why Provides/Obsoletes python3-matrix-synapse (and dropping the Provides matrix-synapse)? Isn't the real name of the package python3-matrix-synapse and we need a Provides so users can install "matrix-synapse"?
Most of the %install and %build code seems reasonable.
All-in-all, I would like to see most of this get merged but I'm unclear on a few remaining topics.
@cyphar: review reminder