Subject: Re: [bug#39841] [WIP PATCH] synapse
From: Marius Bakke <[email-masked]>
To: Alex ter Weele <[email-masked]>
Cc: [email-masked]
Date: Thu, 02 Apr 2020 20:49:40 +0200
List-Id: <debbugs-submit.debbugs.gnu.org>

Phew...  It took a while, but I've merged the whole series.  \o/

PS: Some of the later packages did not pass 'guix lint'.  There were
periods in synopses, lack of period or capitalizations in descriptions
and other minor issues.

I've also modified some of the descriptions to be more ... descriptive,
and use full sentences, instead of just a copy-paste of the synopsis.

I added a 'Co-authored-by:' line for the packages that were heavily
modified to save you some of the blame.  ;-)

A brief feedback on the patches:

Alex ter Weele <[email-masked]> writes:

> +(define-public python-signedjson
> +  (package
> +    (name "python-signedjson")
> +    (version "1.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri "signedjson" version))
> +       (sha256
> +        (base32 "13wfi50wzz38j6xsz7hmx26vf0zvk0fj7516wavzpsyfk9xy3g5v"))))
> +    (build-system python-build-system)
> +    (arguments
> +     ;; TODO "AttributeError: type object 'Callable' has no attribute
> +     ;; '_abc_registry'"
> +     `(#:tests? #f))

This was because of the inclusion of 'python-typing', which is part of
the Python standard library and apparently caused some conflict.
Removing it from propagated-inputs made all tests pass.

> +(define-public python-prometheus-client
> +  (package
> +    (name "python-prometheus-client")
> +    (version "0.7.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri "prometheus_client" version))
> +       (sha256
> +        (base32 "1ni2yv4ixwz32nz39ckia76lvggi7m19y5f702w5qczbnfi29kbi"))))
> +    (build-system python-build-system)
> +    (arguments
> +     ;; TODO "ModuleNotFoundError: No module named 'tests'"
> +     `(#:tests? #f))

This is because the PyPI tarball contains no tests.  I adjusted the
comment to reflect that.

> +    (propagated-inputs
> +     `(("python-twisted" ,python-twisted)))
> +    (native-inputs
> +     `(("python-tox" ,python-tox)))

...and also removed Tox, as it is pretty useless in Guix.  Whenever you
come across a package that uses Tox, you should read what tox.ini does
and do that directly in the check phase instead.

(Running Tox does not work because it wants to go on the internet and
create a virtualenv and does not play well with native Guix packages)

> +(define-public python-ldap3
> +  (package
> +    (name "python-ldap3")
> +    (version "2.6.1")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri "ldap3" version))
> +       (sha256
> +        (base32 "0ag5xqlki6pjk3f50b8ar8vynx2fmkna7rfampv3kdgwg8z6gjr7"))))
> +    (build-system python-build-system)
> +    (arguments
> +     ;; TODO "ImportError: Failed to import test module: ordDict"
> +     `(#:tests? #f))

There were two problems here: the pyasn1 version was too old, and
ultimately running the full suite requires an LDAP server, so I adjusted
the comment to reflect that (and also updated pyasn1 to prevent runtime
errors).

> +(define-public python-eliot
> +  (package
> +    (name "python-eliot")
> +    (version "1.12.0")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri "eliot" version))
> +       (sha256
> +        (base32 "0wabv7hk63l12881f4zw02mmj06583qsx2im0yywdjlj8f56vqdn"))))
> +    (build-system python-build-system)
> +    (arguments
> +     `(#:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'remove-journald-support
> +           (lambda _
> +             (invoke "rm"
> +                     "eliot/tests/test_journald.py"
> +                     "eliot/journald.py")
> +             #t))
> +         (add-after 'remove-journald-support 'remove-eliot-prettyprint-tests
> +           ;; remove command-line tool's tests. TODO eliot-prettyprint should
> +           ;; be installed and these tests should pass.
> +           (lambda _
> +             (invoke "rm" "eliot/tests/test_prettyprint.py")
> +             #t)))))

I changed the (invoke "rm" ...) stanzas to (delete-file ...) to avoid
the fork. 

> +(define-public python-pem
> +  (package
> +    (name "python-pem")
> +    (version "20.1.0")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri "pem" version))
> +       (sha256
> +        (base32
> +         "1xh88ss279fprxnzd10dczmqwjhppbyvljm33zrg2mgybwd66qr7"))))
> +    (build-system python-build-system)
> +    (native-inputs
> +     `(("python-certifi" ,python-certifi)
> +       ("python-coverage" ,python-coverage)
> +       ("python-pretend" ,python-pretend)
> +       ("python-pyopenssl" ,python-pyopenssl)
> +       ("python-pytest" ,python-pytest)
> +       ("python-sphinx" ,python-sphinx)
> +       ("python-twisted" ,python-twisted)))
> +    (home-page "https://pem.readthedocs.io/")
> +    (synopsis "Easy PEM file parsing in Python.")
> +    (description "Easy PEM file parsing in Python.")

I'm curious why all those inputs are just native, but did not
investigate.  I did fix the trailing dot in the synopsis, as well as
expanded a bit on the description.

> +(define-public python-txsni
> +  (package
> +    (name "python-txsni")
> +    (version "0.1.9")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri "TxSNI" version))
> +       (sha256
> +        (base32
> +         "1c72nbhx4z0v5djb89yzrcqlmjdlyryj7qgwl9ivnn816126z5kj"))))
> +    (build-system python-build-system)
> +    (arguments
> +     ;; TODO confusing test failure
> +     `(#:tests? #f))

So this turned out the be because the latest TxSNI release is
incompatible with newer OpenSSL.  I ended up pulling the package
straight from git, and made myself the package author.

> +    (propagated-inputs
> +     `(("python-pyopenssl" ,python-pyopenssl)
> +       ("python-service-identity" ,python-service-identity)
> +       ("python-twisted" ,python-twisted)))
> +    (home-page "https://github.com/glyph/txsni")
> +    (synopsis "easy-to-use SNI endpoint for twisted")
> +    (description
> +     "easy-to-use SNI endpoint for twisted")

...I also rewrote synopsis and description to be more Guix-y.

> +(define-public python-pysaml2
> +  (package
> +    (name "python-pysaml2")
> +    (version "5.0.0")
> +    (source
> +     (origin
> +       (method url-fetch)
> +       (uri (pypi-uri "pysaml2" version))
> +       (sha256
> +        (base32
> +         "1h8cmxh9cvxhrdfmkh92wg6zpxmhi2fixq1cy4hxismmaar7bsny"))))
> +    (build-system python-build-system)
> +    (propagated-inputs
> +     `(("python-cryptography" ,python-cryptography)
> +       ("python-dateutil" ,python-dateutil)
> +       ("python-defusedxml" ,python-defusedxml)
> +       ("python-pyopenssl" ,python-pyopenssl)
> +       ("python-pytz" ,python-pytz)
> +       ("python-requests" ,python-requests)
> +       ("python-six" ,python-six)))
> +    (home-page "https://idpy.org")
> +    (synopsis
> +     "Python implementation of SAML Version 2 Standard")
> +    (description
> +     "Python implementation of SAML Version 2 Standard")

Here I took the description from the README which was very informative.

> * gnu/packages/matrix.scm: New file. (python-matrix-synapse-ldap3) New
>   variable.
> * gnu/local.mk (GNU_SYSTEM_MODULES): Add matrix.scm.
> ---
>  gnu/local.mk            |  1 +
>  gnu/packages/matrix.scm | 33 +++++++++++++++++++++++++++++++++
>  2 files changed, 34 insertions(+)
>  create mode 100644 gnu/packages/matrix.scm
>
> diff --git a/gnu/local.mk b/gnu/local.mk
> index 7d8145b10d..8c50da9c10 100644
> --- a/gnu/local.mk
> +++ b/gnu/local.mk
> @@ -343,6 +343,7 @@ GNU_SYSTEM_MODULES =				\
>    %D%/packages/marst.scm			\
>    %D%/packages/mate.scm             \
>    %D%/packages/maths.scm			\
> +  %D%/packages/matrix.scm			\
>    %D%/packages/maven.scm			\
>    %D%/packages/mc.scm				\
>    %D%/packages/mcrypt.scm			\
> diff --git a/gnu/packages/matrix.scm b/gnu/packages/matrix.scm
> new file mode 100644
> index 0000000000..f399d2aabc
> --- /dev/null
> +++ b/gnu/packages/matrix.scm
> @@ -0,0 +1,33 @@
> +(define-module (gnu packages matrix)
> +  #:use-module ((guix licenses) #:prefix license:)
> +  #:use-module (gnu packages python-crypto)
> +  #:use-module (gnu packages python-xyz)
> +  #:use-module (guix build-system python)
> +  #:use-module (guix download)
> +  #:use-module (guix packages))

You forgot to add the license notice at the top of this file.  Fixed!