GNU bug report logs - #45712
[PATCHES] Improve Python package quality

Previous Next

Package: guix-patches;

Reported by: Lars-Dominik Braun <lars <at> 6xq.net>

Date: Thu, 7 Jan 2021 13:27:02 UTC

Severity: important

Tags: patch

Done: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>

Bug is archived. No further changes may be made.

To add a comment to this bug, you must first unarchive it, by sending
a message to control AT debbugs.gnu.org, with unarchive 45712 in the body.
You can then email your comments to 45712 AT debbugs.gnu.org in the normal way.

Toggle the display of automated, internal messages from the tracker.

View this report as an mbox folder, status mbox, maintainer mbox


Report forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Thu, 07 Jan 2021 13:27:02 GMT) Full text and rfc822 format available.

Acknowledgement sent to Lars-Dominik Braun <lars <at> 6xq.net>:
New bug report received and forwarded. Copy sent to guix-patches <at> gnu.org. (Thu, 07 Jan 2021 13:27:02 GMT) Full text and rfc822 format available.

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: guix-patches <at> gnu.org
Subject: [PATCHES] Improve Python package quality
Date: Thu, 7 Jan 2021 14:26:20 +0100
[Message part 1 (text/plain, inline)]
Hi,

as announced in
https://lists.gnu.org/archive/html/guix-devel/2021-01/msg00021.html I’ve
been working on adding an additional phase to Python packages to check
whether they actually work. I cleaned up my patch, added tests and now
I’m pretty confident it works as expected. The first patch in this
series adds this phase, while the other ones fix build failures caused
by it. All of this should go to core-updates (or a separate wip-*
branch?), since it causes a massive number of rebuilds.

You can also pull my git repo at https://github.com/PromyLOPh/guix.git
branch work-python-importcheck.

Cheers,
Lars

[0001-build-system-python-Validate-installed-package.patch (text/x-diff, attachment)]
[0002-gnu-pytest-6-Add-missing-propagated-input.patch (text/x-diff, attachment)]
[0003-gnu-python-pytest-xdist-Add-missing-input-relax-pyte.patch (text/x-diff, attachment)]
[0004-gnu-python-fixtures-bootstrap-Do-not-apply-loadable-.patch (text/x-diff, attachment)]
[0005-gnu-python-pytest-pep8-Fix-package.patch (text/x-diff, attachment)]
[0006-gnu-python-pyfakefs-Disable-unreliable-test.patch (text/x-diff, attachment)]
[0007-gnu-python-slugify-Add-missing-input.patch (text/x-diff, attachment)]
[0008-gnu-python-websockets-Fix-Python-package-name.patch (text/x-diff, attachment)]
[0009-gnu-python-black-Remove-blackd.patch (text/x-diff, attachment)]
[0010-gnu-python-traitlets-Add-missing-input.patch (text/x-diff, attachment)]
[0011-gnu-python-idna-ssl-Add-missing-input.patch (text/x-diff, attachment)]
[0012-gnu-python-twisted-Remove-broken-console-scripts.patch (text/x-diff, attachment)]
[0013-gnu-python-automat-Remove-broken-console-script.patch (text/x-diff, attachment)]
[0014-gnu-python-packaging-bootstrap-Remove-dependency.patch (text/x-diff, attachment)]
[0015-gnu-python-traceback2-Add-missing-dependency.patch (text/x-diff, attachment)]

Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Fri, 08 Jan 2021 11:38:02 GMT) Full text and rfc822 format available.

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

From: "Hartmut Goebel" <hartmut <at> goebel-consult.de>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: [bug#45712] [PATCHES] Improve Python package quality
Date: Fri, 08 Jan 2021 12:37:05 +0100
Hi Lars,

thanks for the patch set. Please please find some comments. (I did not
check all changes to the packages, assuming you did it right :-)


> +(define validate-script
> +  "from __future__ import print_function # Python 2 support.

Please but this int a new line - makeing it easier to copy & paste.
(Leading emptry lines doe nor effect "from __future__ import").

> +import pkg_resources, sys, importlib, traceback
> +try:
> +    from importlib.machinery import PathFinder
> +except ImportError:
> +    PathFinder = None



> +ws = pkg_resources.find_distributions(sys.argv[1])
> +for dist in ws:
> +    print('validating', repr(dist.project_name), dist.location)
> +    try:
> +        print('...checking requirements', end=': ')

This looks very uncommon (and make my mind hooking on it). Please use
this, which is more common and less mindbogling ;-)

print('...checking requirements: ', end='')


> +        req = str(dist.as_requirement())
> +        # dist.activate() is not enough to actually check requirements, we have to
> +        # .require() it.
> +        pkg_resources.require(req)

Any reason for converting the req into a string first? IC,
"`requirements` must be a string". So I'd move the "str()" to the
function call:

  req = dist.as_requirement()
  # dist.activate() is not enough to actually check requirements,
  # we have to .require() it.
  pkg_resources.require(str(req))


> +        print('OK')
> +    except Exception as e:
> +        print('ERROR:', req, e)
> +        ret = 1
> +        continue
> +    # Try to load entry points of console scripts too, making sure they work. They
> +    # should be removed if they don’t. Other groups may not be safe, as they can
> +    # depend on optional packages.
> +    for group, v in dist.get_entry_map().items():
> +       if group not in {'console_scripts', }:

    if group not in {'console_scripts', 'gui_scripts'}:

Why not gui-scripts?
If not adding gui-scripts, just use "if group != 'concolse_scrips':"

> +           continue
> +       for name, ep in v.items():
> +           try:
> +               print('...trying to load endpoint', group, name, end=': ')

Here it is fine ;-)

> +    # And finally try to load top level modules. This should not have any
> +    # side-effects.

Does it make sence to try loading the top-level modules *after* the the
entry-point? Chances are very high that the entry-points implicitly test
the top-level modules already.

IMHO it would make more sense to first try the top-level modules, and
even stop processing if this fails.

> +    for name in dist.get_metadata_lines('top_level.txt'):
> +        # Only available on Python 3.
> +        if PathFinder and PathFinder.find_spec(name) is None:
> +            # Ignore unavailable modules. Cannot use

Please add a short explanation why there can be unavailable top-level
modules.


> +    (add-after 'check 'validate-loadable validate-loadable)

While not having antoher idea, I'm not happy with this name. "loadable"
is not easy to get. (See also below, where this term is used in commit message.)

top-level-modules-are-importable?

> +(define python-dummy-ok

AFAIKS the packages only differ by some requirements. So I suggest
using a function to return the packages. This makes it easier to spot
the actull differences.

> +           (replace 'unpack
> +             (lambda _
> +               (mkdir-p "src")
> +               (chdir "src")
> +               (mkdir-p "dummy")

(mkdir-p "src/dummy")
(chdir "src")



> +setup(
> +     name='dummy-fail-import',
> +     version='0.1',
> +     packages=['dummy'],
> +     )

I would strip this down (version is not required AFAIK) and put it one
line (her assuming you use (format) for creating the code within a function:

setup(name='~a', packages=~a, install_requires=~a)


> Subject: [PATCH 03/15] gnu: python-pytest-xdist: Add missing input, relax
>  pytest requirement.

> +         (add-after 'unpack 'patch
> +           (lambda* (#:key inputs #:allow-other-keys)

Arguments are not used?

> +             ;; Relax pytest requirement.
> +             (substitute* "setup.py"
> +               (("pytest>=6\\.0\\.0") "pytest"))
> +             #t))

Any reason for relaxing this? Why not use python-pytest-6 as input?

> Subject: [PATCH 04/15] gnu: python-fixtures-bootstrap: Do not apply loadable
>  check.

Please rephrase into something like

Do not validate loadability
Do not validate whetehr packag is loadable
…


> Subject: [PATCH 05/15] gnu: python-pytest-pep8: Fix package.

> -     `(#:tests? #f)) ; Fails with recent pytest and pep8. See upstream issues #8 and #12.
> +     `(#:tests? #t ; Fails with recent pytest and pep8. See upstream issues #8 and #12.

Dosn't this change fix the checks? So this comment would be obsoltes and
"#:tests #t" can be removed.


> Subject: [PATCH 06/15] gnu: python-pyfakefs: Disable unreliable test.

> -         (add-after 'check 'check-pytest-plugin
> +         (replace 'check
>             (lambda _
> -             (invoke
> -              "python" "-m" "pytest"
> -              "pyfakefs/pytest_tests/pytest_plugin_test.py")
> -             #t)))))
> +             (invoke "pytest"
> +               "pyfakefs/tests"
> +               ;; The default test suite does not run these extra tests.
> +               ;"pyfakefs/pytest_tests/pytest_plugin_test.py"
> +               ;; atime difference is larger than expected.
> +               "-k" "not test_copy_real_file"))))))

I suggest keeping the old way, as this is will automatically update to
upstream changes.




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Fri, 08 Jan 2021 12:20:01 GMT) Full text and rfc822 format available.

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

From: Ricardo Wurmus <rekado <at> elephly.net>
To: Hartmut Goebel <hartmut <at> goebel-consult.de>
Cc: 45712 <at> debbugs.gnu.org, Lars-Dominik Braun <lars <at> 6xq.net>,
 guix-patches <at> gnu.org
Subject: Re: [bug#45712] [PATCHES] Improve Python package quality
Date: Fri, 08 Jan 2021 13:19:29 +0100
Hartmut Goebel <hartmut <at> goebel-consult.de> writes:

> Hi Lars,
>
> thanks for the patch set. Please please find some comments. (I did not
> check all changes to the packages, assuming you did it right :-)
>
>
>> +(define validate-script
>> +  "from __future__ import print_function # Python 2 support.
>
> Please but this int a new line - makeing it easier to copy & paste.
> (Leading emptry lines doe nor effect "from __future__ import").

You can also escape the line break:

--8<---------------cut here---------------start------------->8---
(define validate-script "\
from __future__ import print_function # Python 2 support.
…")
--8<---------------cut here---------------end--------------->8---

-- 
Ricardo




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Fri, 08 Jan 2021 12:20:02 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Tue, 12 Jan 2021 09:38:02 GMT) Full text and rfc822 format available.

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Hartmut Goebel <hartmut <at> goebel-consult.de>,
 Ricardo Wurmus <rekado <at> elephly.net>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: [bug#45712] [PATCHES] Improve Python package quality
Date: Tue, 12 Jan 2021 10:37:07 +0100
[Message part 1 (text/plain, inline)]
Hi Hartmut,

> Please but this int a new line - makeing it easier to copy & paste.
> (Leading emptry lines doe nor effect "from __future__ import").
adopted Ricardo’s suggestion here.

> This looks very uncommon (and make my mind hooking on it). Please use
> this, which is more common and less mindbogling ;-)
Done.

> Any reason for converting the req into a string first? IC,
> "`requirements` must be a string". So I'd move the "str()" to the
> function call:
Yes, the string is used in the error message.

>     if group not in {'console_scripts', 'gui_scripts'}:
> Why not gui-scripts?
> If not adding gui-scripts, just use "if group != 'concolse_scrips':"
I wasn’t aware this exists. Added, thanks!

> Does it make sence to try loading the top-level modules *after* the the
> entry-point? Chances are very high that the entry-points implicitly test
> the top-level modules already.
> IMHO it would make more sense to first try the top-level modules, and
> even stop processing if this fails.
True, I reversed the order. Still, I think continuing with the entry
points might be worth, if it points out more (different) errors. For
small packages this might not be the case, but larger ones often only
re-export specific names in the top-level module, thus testing might
reveal more issues.

> Please add a short explanation why there can be unavailable top-level
> modules.
Done.

> While not having antoher idea, I'm not happy with this name. "loadable"
> is not easy to get. (See also below, where this term is used in commit message.)
> top-level-modules-are-importable?
I’m also not happy with the name, but can’t think of a better one right
now. Anyone?

> AFAIKS the packages only differ by some requirements. So I suggest
> using a function to return the packages. This makes it easier to spot
> the actull differences.
> […]
> (mkdir-p "src/dummy")
> (chdir "src")
Done.

> I would strip this down (version is not required AFAIK) and put it one
> line (her assuming you use (format) for creating the code within a function:
Not sure whether it’s optional or not, [1] does not say it is. No harm
in keeping it?

[1] https://setuptools.readthedocs.io/en/latest/references/keywords.html?highlight=version#keywords

> Arguments are not used?
Fixed.

> Any reason for relaxing this? Why not use python-pytest-6 as input?
Yes, our pytest <at> 6 package reports its version as 0.0.0, so this patch is
required with either package. Or we figure out how to fix pytest <at> 6
(works fine with PEP 517 compliant build system).

> Please rephrase into something like
> Do not validate loadability
Done.

> Dosn't this change fix the checks? So this comment would be obsoltes and
> "#:tests #t" can be removed.
Should’ve been #f, sorry, fixed.

> I suggest keeping the old way, as this is will automatically update to
> upstream changes.
Okay, I’ve added another phase that disables the test manually, because
it fails for me every time.

See attached patchset (v2) or git repository for updated patches.

I’ve also rebuilt all 2284 packages depending on python-build-system.
Currently 426 of them fail to build for any reason (not necessarily
caused by this patchset; I’ve seen some unrelated issues). Due to the
large number of packages failing I suggest moving forward with applying
this first batch and then fixing everything else incrementally, unless
some major package is broken (see attached list). WDYT?

Cheers,
Lars

[0001-build-system-python-Validate-installed-package.patch (text/x-diff, attachment)]
[0002-gnu-pytest-6-Add-missing-propagated-input.patch (text/x-diff, attachment)]
[0003-gnu-python-pytest-xdist-Add-missing-input-relax-pyte.patch (text/x-diff, attachment)]
[0004-gnu-python-fixtures-bootstrap-Do-not-validate-loadab.patch (text/x-diff, attachment)]
[0005-gnu-python-pytest-pep8-Fix-package.patch (text/x-diff, attachment)]
[0006-gnu-python-pyfakefs-Disable-unreliable-test.patch (text/x-diff, attachment)]
[0007-gnu-python-slugify-Add-missing-input.patch (text/x-diff, attachment)]
[0008-gnu-python-websockets-Fix-Python-package-name.patch (text/x-diff, attachment)]
[0009-gnu-python-black-Remove-blackd.patch (text/x-diff, attachment)]
[0010-gnu-python-traitlets-Add-missing-input.patch (text/x-diff, attachment)]
[0011-gnu-python-idna-ssl-Add-missing-input.patch (text/x-diff, attachment)]
[0012-gnu-python-twisted-Remove-broken-console-scripts.patch (text/x-diff, attachment)]
[0013-gnu-python-automat-Remove-broken-console-script.patch (text/x-diff, attachment)]
[0014-gnu-python-packaging-bootstrap-Remove-dependency.patch (text/x-diff, attachment)]
[0015-gnu-python-traceback2-Add-missing-dependency.patch (text/x-diff, attachment)]
[0016-gnu-python2-packaging-bootstrap-Add-missing-dependen.patch (text/x-diff, attachment)]
[failing-packages.txt (text/plain, attachment)]

Severity set to 'important' from 'normal' Request was from Ludovic Courtès <ludo <at> gnu.org> to control <at> debbugs.gnu.org. (Wed, 13 Jan 2021 14:33:01 GMT) Full text and rfc822 format available.

Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Mon, 25 Jan 2021 14:44:01 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Mon, 25 Jan 2021 09:43:40 -0500
Hi!

Lars-Dominik Braun <lars <at> 6xq.net> writes:

> Hi,
>
> as announced in
> https://lists.gnu.org/archive/html/guix-devel/2021-01/msg00021.html Ive
> been working on adding an additional phase to Python packages to check
> whether they actually work. I cleaned up my patch, added tests and now
> Im pretty confident it works as expected. The first patch in this
> series adds this phase, while the other ones fix build failures caused
> by it. All of this should go to core-updates (or a separate wip-*
> branch?), since it causes a massive number of rebuilds.
>
> You can also pull my git repo at https://github.com/PromyLOPh/guix.git
> branch work-python-importcheck.

Thanks for the initiative!  It looks good, on first sight.  One question
I have is this: does it rely on the Python package having been built
with setuptools/distutils?  The Python world is moving toward a
plurality of PEP 517 compliant build systems; any idea if the checker
will continue working for these new packages?

We have currently only one such package in our collection, on
core-updates.  It's python-isort, added with commit
812a2931de553d12c01b0a4d53d03613b09adaaf on the core-updates branch.

Thank you,

Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Mon, 25 Jan 2021 14:49:01 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Mon, 25 Jan 2021 09:48:17 -0500
Hi again,

[...]

>>From cf9ae80b59e86a60c27734c8cc27637757490d70 Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <lars <at> 6xq.net>
> Date: Thu, 7 Jan 2021 13:25:56 +0100
> Subject: [PATCH 02/15] gnu: pytest <at> 6: Add missing propagated-input.
>
> * gnu/packages/check.scm (python-pytest-6) [native-inputs]: Remove
> python-iniconfig.
> [propagated-inputs]: Move it here.
> ---
>  gnu/packages/check.scm | 6 +++---
>  1 file changed, 3 insertions(+), 3 deletions(-)
>
> diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
> index 1300f9e1a6..9d1e0b8173 100644
> --- a/gnu/packages/check.scm
> +++ b/gnu/packages/check.scm
> @@ -964,13 +964,13 @@ and many external plugins.")
>      (propagated-inputs
>       (append (alist-delete "python-py"
>                             (package-propagated-inputs python-pytest))
> -             `(("python-py" ,python-py-next))))
> +             `(("python-py" ,python-py-next)
> +               ("python-iniconfig" ,python-iniconfig))))
>      (native-inputs
>       (append (alist-delete "python-pytest"
>                             (package-native-inputs python-pytest))
>               `(("python-pytest" ,python-pytest-6-bootstrap)
> -               ("python-toml" ,python-toml)
> -               ("python-iniconfig" ,python-iniconfig))))))
> +               ("python-toml" ,python-toml))))))
>  
>  ;; Pytest 4.x are the last versions that support Python 2.
>  (define-public python2-pytest

I hadn't seen this patch but I fixed that problem on core-updates with
be7061cea30b59676fc473d6bd4a56a0f2fbd7cf on the 14 of January.  Note
that python-pytest-6 became just 'python-pytest' there.




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Mon, 25 Jan 2021 19:30:02 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: Hartmut Goebel <hartmut <at> goebel-consult.de>,
 Ricardo Wurmus <rekado <at> elephly.net>, 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Mon, 25 Jan 2021 14:29:06 -0500
[Message part 1 (text/plain, inline)]
Hi Lars-Dominik,

Lars-Dominik Braun <lars <at> 6xq.net> writes:

> Adds a new phase validating usalibity of installed Python packages.
>
> * guix/build/python-build-system.scm (validate-script): Add script.
> (validate-loadable): New phase.
> (%standard-phases): Use it.
> * tests/builders.scm (make-python-dummy): Add test package generator.
> (check-build-{success,failure}): Add build helper functions.
> (python-dummy-*): Add test packages.
> ("python-build-system: &"): Add tests.

Attached is a small rework of your original patch.  I've made the Python
script standalone, which should make it easier to maintain.  I've also
refactored the tests somewhat and added your copyright information.

Is this OK with you?

Thanks!

Maxim
[0001-build-system-python-Add-a-sanity-check-phase.patch (text/x-patch, attachment)]

Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Mon, 25 Jan 2021 19:43:02 GMT) Full text and rfc822 format available.

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Mon, 25 Jan 2021 20:42:26 +0100
Hi Maxim,

> Thanks for the initiative!  It looks good, on first sight.  One question
> I have is this: does it rely on the Python package having been built
> with setuptools/distutils?  The Python world is moving toward a
> plurality of PEP 517 compliant build systems; any idea if the checker
> will continue working for these new packages?
yes and no. pkg_resources is part of setuptools, so my patch depends on
setuptools. But I think dependencies are specified in a standardized
format[1], which any tool can (and should?) write.

top_level.txt seems to be part of python eggs, which are deprecated[2].
I guess we could just scan for top-level directories under
site-packages, which have a __init__.py and try to load them.

I can’t find any standard for the entry points, so I’m guessing they’re
setuptools-specific too.

My assumption is that if no metadata is found the checks are just
skipped and nothing bad happens, but we may have to verify this.

As for PEP 517, I’m working on updating python-build-system, see
https://lists.gnu.org/archive/html/guix-devel/2021-01/msg00294.html
I’ll send an updated patch to guix-patches@ as soon as I have the 'check
phase working.

Cheers,
Lars

[1] https://packaging.python.org/specifications/core-metadata/#requires-dist-multiple-use
[2] https://setuptools.readthedocs.io/en/latest/deprecated/python_eggs.html#top-level-txt-conflict-management-metadata




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Tue, 26 Jan 2021 08:40:01 GMT) Full text and rfc822 format available.

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Tue, 26 Jan 2021 09:39:22 +0100
Hi Maxim,

> Attached is a small rework of your original patch.  I've made the Python
> script standalone, which should make it easier to maintain.  I've also
> refactored the tests somewhat and added your copyright information.
> Is this OK with you?
sure, no problem with the refactoring in gerenal, but I think you used
an old patchset. I sent a v2 to this issue, which had some changes to
the scripts and tests.

I can’t test your patch properly unfortunately, because `make check`
does not work on core-updates and trying to build any Python package on
core-updates triggers the requirements checker immediately: 

validating 'attrs' /gnu/store/hdjip92izsf9anfhd6ijgc9glvbi4dzv-python-attrs-bootstrap-19.3.0/lib/python3.9/site-packages
...checking requirements: ERROR: attrs==19.3.0 The 'attrs==19.3.0' distribution was not found and is required by the application”)

Maybe that’s an issue with Python 3.9?

Cheers,
Lars





Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Thu, 28 Jan 2021 15:41:02 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Thu, 28 Jan 2021 10:40:26 -0500
Hi Lars,

Lars-Dominik Braun <lars <at> 6xq.net> writes:

> Hi Maxim,
>
>> Attached is a small rework of your original patch.  I've made the Python
>> script standalone, which should make it easier to maintain.  I've also
>> refactored the tests somewhat and added your copyright information.
>> Is this OK with you?
> sure, no problem with the refactoring in gerenal, but I think you used
> an old patchset. I sent a v2 to this issue, which had some changes to
> the scripts and tests.
>
> I can’t test your patch properly unfortunately, because `make check`
> does not work on core-updates and trying to build any Python package on
> core-updates triggers the requirements checker immediately: 
>
> validating 'attrs'
> /gnu/store/hdjip92izsf9anfhd6ijgc9glvbi4dzv-python-attrs-bootstrap-19.3.0/lib/python3.9/site-packages
> ...checking requirements: ERROR: attrs==19.3.0 The 'attrs==19.3.0'
> distribution was not found and is required by the application”)
>
> Maybe that’s an issue with Python 3.9?
>
> Cheers,
> Lars

Doesn't that mean that the attrs requirement should be lifted from the
bootstrap version? As its purpose is exactly this: breaking the cyclic
dependency with itself.

Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Thu, 28 Jan 2021 16:19:02 GMT) Full text and rfc822 format available.

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Thu, 28 Jan 2021 17:18:39 +0100
Hi Maxim,

> Doesn't that mean that the attrs requirement should be lifted from the
> bootstrap version? As its purpose is exactly this: breaking the cyclic
> dependency with itself.
ah, now I see the problem, there’s a (add-installed-pythonpath inputs
outputs) missing and so it can’t find the installed packages. Maybe
we’re working on different branches? I applied your patch directly to
core-updates.

Cheers,
Lars





Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Fri, 29 Jan 2021 14:15:02 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Fri, 29 Jan 2021 09:14:40 -0500
Lars-Dominik Braun <lars <at> 6xq.net> writes:

[...]

>>From 9bc53a8e9706440668ec70d88db1ebc7d5e2c71d Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <lars <at> 6xq.net>
> Date: Thu, 7 Jan 2021 13:27:55 +0100
> Subject: [PATCH 03/15] gnu: python-pytest-xdist: Add missing input, relax
>  pytest requirement.
>
> * gnu/packages/check.scm: (python-pytest-xdist)
> [arguments]: Relax pytest version requirements.
> [propagated-inputs]: Add python-pytest-forked.
> ---
>  gnu/packages/check.scm | 25 ++++++++++++++++---------
>  1 file changed, 16 insertions(+), 9 deletions(-)
>
> diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
> index 9d1e0b8173..32a1a2d6a3 100644
> --- a/gnu/packages/check.scm
> +++ b/gnu/packages/check.scm
> @@ -1216,20 +1216,27 @@ same arguments.")
>             #t))))
>      (build-system python-build-system)
>      (arguments
> -     '(#:tests? #f)) ;FIXME: Some tests are failing.
> -       ;; #:phases
> -       ;; (modify-phases %standard-phases
> -       ;;   (delete 'check)
> -       ;;   (add-after 'install 'check
> -       ;;     (lambda* (#:key inputs outputs #:allow-other-keys)
> -       ;;       (add-installed-pythonpath inputs outputs)
> -       ;;       (zero? (system* "py.test" "-v")))))
> +     '(#:tests? #f ; Lots of tests fail.
> +       #:phases
> +       (modify-phases %standard-phases
> +         (add-after 'unpack 'patch
> +           (lambda* (#:key inputs #:allow-other-keys)
> +             ;; Relax pytest requirement.
> +             (substitute* "setup.py"
> +               (("pytest>=6\\.0\\.0") "pytest"))

I'm not sure what the above was caused by, but its'
been fixed on core-updates (the version of pytest as per its metadata is
reported correctly there).




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Fri, 29 Jan 2021 14:27:02 GMT) Full text and rfc822 format available.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Fri, 29 Jan 2021 09:26:18 -0500
Hi Lars,

Lars-Dominik Braun <lars <at> 6xq.net> writes:

> Hi Maxim,
>
>> Doesn't that mean that the attrs requirement should be lifted from the
>> bootstrap version? As its purpose is exactly this: breaking the cyclic
>> dependency with itself.
> ah, now I see the problem, there’s a (add-installed-pythonpath inputs
> outputs) missing and so it can’t find the installed packages. Maybe
> we’re working on different branches? I applied your patch directly to
> core-updates.
>
> Cheers,
> Lars

Oh yes, sorry I had failed to mention it, this is on the
cu/farewell-to-pythonpath branch, which is the integration of
GUIX_PYTHONPATH and this work of yours.  I've included your v2 patches,
with light editing: fixed the script indentation (spot via flake8), I
removed the python2 tests as Python 2 is obsolete, and removed the space
between the (procedure)[area/conditional] part of the GNU changelog
commit messages.

Another note to help with review; when sending v2 patches, make sure the
title of your mail reply mentions [PATCH v2]; this helps to spot the
later versions in email threads.

The branch is shaping up nicely; I encourage you to try it.  If no major
problem is found with it, I'll merge it in core-updates soon.

Thank you,

Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Mon, 01 Feb 2021 07:22:02 GMT) Full text and rfc822 format available.

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Mon, 1 Feb 2021 08:20:45 +0100
Hi Maxim,

> Oh yes, sorry I had failed to mention it, this is on the
> cu/farewell-to-pythonpath branch, which is the integration of
> GUIX_PYTHONPATH and this work of yours.  I've included your v2 patches,
> with light editing: fixed the script indentation (spot via flake8), I
> removed the python2 tests as Python 2 is obsolete, and removed the space
> between the (procedure)[area/conditional] part of the GNU changelog
> commit messages.
thanks!

> Another note to help with review; when sending v2 patches, make sure the
> title of your mail reply mentions [PATCH v2]; this helps to spot the
> later versions in email threads.
Sorry about that, will do next time.

> The branch is shaping up nicely; I encourage you to try it.  If no major
> problem is found with it, I'll merge it in core-updates soon.
I’ve been rebuilding packages on my list that use python-build-system
using this branch and quite a few fail, but in a random sample I found
none that fails due to this patch and also I imagine some issues are
already fixed on core-updates? Is there anything else I can do?

Cheers,
Lars





Reply sent to Maxim Cournoyer <maxim.cournoyer <at> gmail.com>:
You have taken responsibility. (Mon, 01 Feb 2021 17:03:02 GMT) Full text and rfc822 format available.

Notification sent to Lars-Dominik Braun <lars <at> 6xq.net>:
bug acknowledged by developer. (Mon, 01 Feb 2021 17:03:02 GMT) Full text and rfc822 format available.

Message #54 received at 45712-done <at> debbugs.gnu.org (full text, mbox):

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: 45712-done <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Mon, 01 Feb 2021 12:02:44 -0500
Hi Lars,

Lars-Dominik Braun <lars <at> 6xq.net> writes:

> Hi Maxim,
>
>> Oh yes, sorry I had failed to mention it, this is on the
>> cu/farewell-to-pythonpath branch, which is the integration of
>> GUIX_PYTHONPATH and this work of yours.  I've included your v2 patches,
>> with light editing: fixed the script indentation (spot via flake8), I
>> removed the python2 tests as Python 2 is obsolete, and removed the space
>> between the (procedure)[area/conditional] part of the GNU changelog
>> commit messages.
> thanks!
>
>> Another note to help with review; when sending v2 patches, make sure the
>> title of your mail reply mentions [PATCH v2]; this helps to spot the
>> later versions in email threads.
> Sorry about that, will do next time.
>
>> The branch is shaping up nicely; I encourage you to try it.  If no major
>> problem is found with it, I'll merge it in core-updates soon.
> I’ve been rebuilding packages on my list that use python-build-system
> using this branch and quite a few fail, but in a random sample I found
> none that fails due to this patch and also I imagine some issues are
> already fixed on core-updates? Is there anything else I can do?

I guess we can be bold and merge the branch to core-updates, and fix any
breakage that may occur there!

I've done so, the last commit of the series is
1b9186828867e77af1f2ee6741063424f8256398.

Let's make Python on Guix great again!

Thank you :-)

Closing,

Maxim




Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Sun, 07 Feb 2021 17:00:03 GMT) Full text and rfc822 format available.

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

From: Hartmut Goebel <hartmut <at> goebel-consult.de>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>,
 Lars-Dominik Braun <lars <at> 6xq.net>
Cc: Ricardo Wurmus <rekado <at> elephly.net>, 45712 <at> debbugs.gnu.org
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Sun, 7 Feb 2021 17:59:02 +0100
[Message part 1 (text/plain, inline)]
Hi Maxim,
>
> Attached is a small rework of your original patch.  I've made the Python
> script standalone, which should make it easier to maintain.  I've also
> refactored the tests somewhat and added your copyright information.
>
> Is this OK with you?

I had discussed some change to his original patch with Lars. Can't 
remember all point, just these:

+ if group not in {'console_scripts', }:

"gui_scripts"m are missing. Using a set there is uncommon, since it is a 
constant value anyway.

+ # And finally try to load top level modules. This should not have any

+ # side-effects.

I'd try loading the top level module first. As this is a pre-condition 
for loading the entry-points.

-- 
+++hartmut

| Hartmut Goebel            |                       |
| hartmut <at> goebel-consult.de | www.goebel-consult.de |

[Message part 2 (text/html, inline)]

Information forwarded to guix-patches <at> gnu.org:
bug#45712; Package guix-patches. (Mon, 08 Feb 2021 08:03:01 GMT) Full text and rfc822 format available.

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Hartmut Goebel <hartmut <at> goebel-consult.de>
Cc: 45712 <at> debbugs.gnu.org, Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: Re: bug#45712: [PATCHES] Improve Python package quality
Date: Mon, 8 Feb 2021 09:02:37 +0100
Hi Hartmut,

> I had discussed some change to his original patch with Lars. Can't 
> remember all point, just these:
these points were addressed, before Maxim merged the change into
core-updates.

Cheers,
Lars





bug archived. Request was from Debbugs Internal Request <help-debbugs <at> gnu.org> to internal_control <at> debbugs.gnu.org. (Mon, 08 Mar 2021 12:24:04 GMT) Full text and rfc822 format available.

This bug report was last modified 3 years and 20 days ago.

Previous Next


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