GNU bug report logs - #59239
[PATCH] gnu: Add python-synapseclient.

Previous Next

Full log

Message #14 received at 59239 <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Ricardo Wurmus <rekado <at> elephly.net>
Cc: 59239 <at> debbugs.gnu.org,
 Mădălin Ionel Patrașcu
 <madalinionel.patrascu <at> mdc-berlin.de>
Subject: Re: bug#59239: [PATCH] gnu: Add python-synapseclient.
Date: Tue, 21 Mar 2023 09:06:09 -0400

Hello Mădălin,

Ricardo Wurmus <rekado <at> elephly.net> writes:

> Hi Mădălin,
>
>> * gnu/packages/python-xyz.scm (python-synapseclient): New variable.
> […]
>
>> +              ;;(method url-fetch)
>> +              ;;(uri (pypi-uri "synapseclient" version))
>> +              ;;(sha256
>> +              ;; (base32
>> +              ;;  "1g2n5bbq5vkvprk4ap3sbz2q39cjwl8igy3krlskg34c1q7if7r4"))))
>
> Please remove this commented code.
>
>> +    (build-system python-build-system)
>> +    (arguments
>> +     '(#:phases
>> +       (modify-phases %standard-phases
>> +         (add-after 'unpack 'relax-version-requirements
>> +           (lambda _
>> +             (substitute* "setup.py"
>> +               (("keyring>=15,<23\\.5") "keyring>=15")
>
> Is this really a good idea?  Upper bounds usually exist for a reason.
> We currently have version 23.9.  Can you please clear this with upstream
> first?
>
>> +               (("keyrings\\.alt==3\\.1") "keyrings.alt>=3.1"))))
>> +         (add-before 'build 'set-homeless-shelter
>> +           (lambda _
>> +             ;; could not create '/homeless-shelter': Permission denied
>> +             (setenv "HOME" "/tmp")))
>
> We’re setting HOME, not homeless-shelter.
>
>> +         (replace 'check
>> +           (lambda* (#:key tests? #:allow-other-keys)
>> +             (when tests?
>> +               ;; these tests require server authentification  SynapseNoCredentialsError
>> +               (delete-file-recursively "tests/integration/synapseclient")
>> +               (delete-file-recursively "tests/integration/synapseutils")
>> +               (invoke "pytest")))))))
>
> typo: “authentication”.  What exactly does this error mean?  Does it
> talk to the internet?  How are these tests supposed to be run?
>
> Is it possible to disable these tests with an option to pytest?  That
> would seem better than conditionally deleting them.
>
> Finally, I think the test files should be deleted unconditionally, so
> that “tests?” only governs whether the tests are *run* instead of also
> causing other side effects.

Genle ping.  Could you please send a v2 with the above suggested
changes?

-- 
Thanks,
Maxim

Previous Next

GNU bug tracking system
Copyright (C) 1999 Darren O. Benham, 1997,2003 nCipher Corporation Ltd, 1994-97 Ian Jackson.