• Review of python-bonsai

    From =?UTF-8?Q?Louis-Philippe_V=c3=a9ron@21:1/5 to All on Mon Dec 13 23:10:01 2021
    Hi!

    Here's my review of python-bonsai:

    1. d/control:

    * You don't need the "(>= 3.6)" restriction for python3-all-dev, as that version isn't even in oldstable.

    * sphinx-common isn't needed for the source package, as python3-sphinx
    depends on it

    * "python3 (>= 3.6)" is not required for python3-bonsai,
    ${python3:Depends} should take care of that for you.

    * IMO, python3-bonsai should recommend or suggest python3-bonsai-doc,
    but that's up to you.

    ---------

    2. d/copyright:

    * you forgot to add a debian/* section. AFAIU, noirello isn't the one
    who wrote d/rules :)

    * .appveyor/run_with_env.cmd is licensed CC0. You probably don't need
    those files, so you can exclude them from the source package using Files-Excluded in d/copyright

    * the MIT license in Debian is named "Expat", for historical reasons.

    ---------

    4. d/rules:

    * You left "export DH_VERBOSE = 1" uncommented.

    * I'm curious to why you need to set "export LC_ALL = C.UTF-8".

    ---------

    5. d/tests:

    I don't have an autopkgtests setup that has machine-level isolation. You
    ran that code and it works?

    ---------

    6. d/watch:

    You left "<project>" in there instead of replacing it by the actual
    project's name (have a look at Lintian) :) Note you can use the "git
    tag" mode to simplify this file (not that it's required, your file works as-is): [1]

    ---------

    7. Upstream code

    Have you read the upstream code? It's something you should do (and you
    should read all the changes for each new update). Not that you have to
    do a proper security audit, but you should go through the code to be
    sure there's no obvious or dangerous things in there.

    Otherwise, good job! Fix those, ping me and if it's OK, I'll read the
    upstream code myself and sponsor it.

    Cheers,

    [1]: https://salsa.debian.org/python-team/packages/python-mediafile/-/blob/debian/master/debian/watch


    --
    ⢀⣴⠾⠻⢶⣦⠀
    ⣾⠁⢠⠒⠀⣿⡁ Louis-Philippe Véronneau
    ⢿⡄⠘⠷⠚⠋ pollo@debian.org / veronneau.org
    ⠈⠳⣄

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)
  • From Robin Jarry@21:1/5 to See my on Tue Dec 14 01:10:02 2021
    Hi Louis-Philippe,

    I think I have addressed all your comments. I have pushed fixes in
    a temporary branch. I'll integrate them on debian/master once the review
    is finished:

    https://salsa.debian.org/python-team/packages/python-bonsai/-/commits/review/

    See my replies below:

    Louis-Philippe Véronneau, Dec 13, 2021 at 23:07:
    * I'm curious to why you need to set "export LC_ALL = C.UTF-8".

    This was a leftover from another package. It is useless.

    5. d/tests:

    I don't have an autopkgtests setup that has machine-level isolation. You
    ran that code and it works?

    Yes, here is the build log:

    http://files.diabeteman.com/juoghahf0teG3Phi/python-bonsai_1.3.0-1_amd64.build.txt

    I tried to reproduce the test environment which is prepared for the
    github CI actions. This involves generating a docker container with an
    LDAP server installed and configured, running that container with
    elevated privileges and run the test suite to talk to that container.
    It also does some tc voodoo to force timeouts on the TCP connections
    (see .ci/delay.py).

    I did not manage to run autopkgtests with containers but since the test
    needs to run multiple services, a virtual machine made more sense to me.
    Also, the use of tc may be a bit too much for a container (I'm not sure
    about the limitations, depending on the host).

    Some tests are explicitly disabled because I could not get them to work.
    I will ping the upstream developer with this when I get a chance.

    7. Upstream code

    Have you read the upstream code? It's something you should do (and you
    should read all the changes for each new update). Not that you have to
    do a proper security audit, but you should go through the code to be
    sure there's no obvious or dangerous things in there.

    I have read most of the python code and it looks well written,
    documented and tested. C python extensions code is always rather obscure
    but I did not see anything suspicious. I am no expert in libldap2
    however. The project has been around for some years now. It looks stable
    enough to go in Debian in my opinion.

    Thanks a lot for your time!

    --- SoupGate-Win32 v1.05
    * Origin: fsxNet Usenet Gateway (21:1/5)