Re: CVS commit: pkgsrc/chat/matrix-synapse

[Jonathan Schleifer <js%netbsd.org@localhost>]

Am 31.01.24 um 19:31 schrieb Greg Troxel:

> Did you ask js@ to review this change?  It seems major enough that it should have
> been tested in production, which is what js@ and I have been doing
> before committing updates.   I am guessing not, because I probably would
> have been asked to look at it and/or try it too.

I have not been contacted.

> >   # Dependencies as defined by synapse's build system (in theory):
> > -# \todo Go over poetry.lock
>
> You dropped this comment.  But the commit message does not say that you
> have rigorously gone over the upstream documented-in-build depends.  (It
> has seemed messy.)

I'll defer to gdt@ here, as he added that comment. Synapse migrated to 
poetry at some point and it was messy and thankfully, gdt@ handled that 
migration.

> > +DEPENDS+=      ${PYPKGPREFIX}-cryptography>=3.4.7:../../security/py-cryptography
>
> It seems this was missing, but it was js@'s intent to not require recent
> cryptography.  That may not longer make sense as the package needs rust
> anyway, but if you didn't check with MAINTAINER it's not ok to drop such
> support (such as the patch to pyproject.toml).

That was indeed dropped intentionally, but indeed no longer needs to be 
dropped, so this change is fine. And yes, the reason was to not require 
Rust when it wasn't needed -- that is unfortunately no longer the case, 
as Synapse itself needs Rust these days :(.

> > -DEPENDS+=      ${PYPKGPREFIX}-idna>=2.5:../../www/py-idna
> > -DEPENDS+=      ${PYPKGPREFIX}-lxml>=3.5.0:../../textproc/py-lxml
> > -DEPENDS+=      ${PYPKGPREFIX}-nacl>=1.2.1:../../security/py-nacl
>
> These show up in the sources.  Why did you remove them?

+1, unclear why those were removed. Did you try if it even runs without 
those installed on the system?

> > -DEPENDS+=      ${PYPKGPREFIX}-psycopg2>=2.7:../../databases/py-psycopg2
>
> Why is this dropped?  synapse needs a postgres database and we as
> maintainers have more or less given up on sqlite3.

Please don't drop this. sqlite3 isn't only given up on by us - it's by 
upstream as well. To the point where if you use sqlite3 and update 
Synapse, there is no guarantee that your DB is still intact. It only 
exists for testing purposes and should never be used in any use case 
that would be covered by pkgsrc. It's only for synapse development 
itself at this point, where you don't mind just wiping the entire DB 
regularly.

--
Jonathan