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

[Adam <adam%netbsd.org@localhost>]

>> matrix-synapse: allow py-pydantic v2; provide correct DEPENDs; clean up
> 
> 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.

DEPENDs were wrong. You didn't read pyproject.toml.


>> # 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.)

poetry.lock is not the place to look for dependencies, but pyproject.toml.


>> +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).

Right, rust is needed anyway, so there's no point in insisting on non-rust py-cryptography.


>> -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?

idna and lxml are optional dependencies, but not only these.
I couldn't find a trace of nacl in the source code (grep -r nacl work/synapse-1.98.0).


>> -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.

Synapse marks this as optional. Do you think it's necessary to depend on it? If so, I can bring it back. But maybe, someone doesn't prefer to depend on PostgreSQL.


>> -TEST_DEPENDS+= ${PYPKGPREFIX}-test-[0-9]*:../../devel/py-test
>> 
>> @@ -112,12 +109,12 @@ post-install:
>> # \todo Grok upstream's new test scheme and port to it.
>> # As of 1.98.0 all tests fail, most of them failing to import
>> # synapse.synapse_rust.
>> -do-test:
>> - cd ${WRKSRC} && ${SETENV} ${MAKE_ENV} pytest-${PYVERSSUFFIX}
>> +# Quick fix before running tests: rm -r ${WRKSRC}/synapse
> 
> This looks like an improvement, but:
> 
>  Is the package now running tests the way upstream wants tests run?  If
>  so, should the todo comment be removed?
> 
>  Tests all fail, but with the "quick fix", things seem better:
>    2166 failed, 616 passed, 292 skipped, 198 warnings, 4 errors in 199.39s (0:03:19)
> 
>  Do you understand what's going on?  It seems like it should be
>  explained in a comment.

Actually, it's sufficient to copy (or link) ${WRKSRC}/build/lib.*/synapse/synapse_rust.abi3.so to ${WRKSRC}/synapse/, but the lib.* directory name is tricky to guess.

I got: 38 failed, 2675 passed, 361 skipped, 919 warnings, 4 errors in 131.44s (0:02:11) on NetBSD-current. Most error involve invoking `openssl` command.


Kind regards,
Adam