[PATCHES] Improve Python package quality

  • Done
  • quality assurance status badge
Details
5 participants
  • Hartmut Goebel
  • Lars-Dominik Braun
  • Ludovic Courtès
  • Maxim Cournoyer
  • Ricardo Wurmus
Owner
unassigned
Submitted by
Lars-Dominik Braun
Severity
important
L
L
Lars-Dominik Braun wrote on 7 Jan 2021 14:26
(address . guix-patches@gnu.org)
X/cL/EAsHw0UNXXy@noor.fritz.box
Hi,

as announced in
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
Attachment: 0001-build-system-python-Validate-installed-package.patch
From cf9ae80b59e86a60c27734c8cc27637757490d70 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:25:56 +0100
Subject: [PATCH 02/15] gnu: pytest@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(-)

Toggle diff (23 lines)
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
--
2.26.2
From 9bc53a8e9706440668ec70d88db1ebc7d5e2c71d Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@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(-)

Toggle diff (43 lines)
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"))
+ #t))
+ (replace 'check
+ (lambda* (#:key tests? inputs outputs #:allow-other-keys)
+ (when tests?
+ (add-installed-pythonpath inputs outputs)
+ (invoke "py.test" "-v")))))))
(native-inputs
`(("python-setuptools-scm" ,python-setuptools-scm)))
(propagated-inputs
`(("python-execnet" ,python-execnet)
("python-pytest" ,python-pytest)
- ("python-py" ,python-py)))
+ ("python-py" ,python-py)
+ ("python-pytest-forked" ,python-pytest-forked)))
(home-page
"https://github.com/pytest-dev/pytest-xdist")
(synopsis
--
2.26.2
From 0974fc1c98ae1554ddd37a1ca40127bd5df95179 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:29:59 +0100
Subject: [PATCH 04/15] gnu: python-fixtures-bootstrap: Do not apply loadable
check.

* gnu/packages/check.scm (python-fixtures-bootstrap) [arguments]: Delete
'validate-loadable.
---
gnu/packages/check.scm | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

Toggle diff (20 lines)
diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index 32a1a2d6a3..fba408d88f 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -1541,7 +1541,12 @@ protocol.")))
(base32
"1vxj29bzz3rd4pcy51d05wng9q9dh4jq6wx92yklsm7i6h1ddw7w"))))
(build-system python-build-system)
- (arguments `(#:tests? #f))
+ (arguments
+ `(#:tests? #f
+ #:phases
+ (modify-phases %standard-phases
+ ;; Package is not loadable on its own at this stage.
+ (delete 'validate-loadable))))
(propagated-inputs
`(("python-pbr-minimal" ,python-pbr-minimal)
("python-six" ,python-six)))
--
2.26.2
From d2272e00c42e3ddde1b0532a4b1ad187816dc98f Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:35:11 +0100
Subject: [PATCH 05/15] gnu: python-pytest-pep8: Fix package.

* gnu/packages/check.scm (python-pytest-pep8) [arguments]: Remove
dependency on pytest-cache and add proper 'check phase.
---
gnu/packages/check.scm | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

Toggle diff (27 lines)
diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index fba408d88f..796635e012 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -2043,7 +2043,19 @@ failures.")
"06032agzhw1i9d9qlhfblnl3dw5hcyxhagn7b120zhrszbjzfbh3"))))
(build-system python-build-system)
(arguments
- `(#: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.
+ #:phases
+ (modify-phases %standard-phases
+ (add-after 'unpack 'fix-dependencies
+ (lambda _
+ (substitute* "setup.py"
+ (("'pytest-cache', ") "")) ; Included in recent pytest
+ #t))
+ (replace 'check
+ (lambda* (#:key tests? inputs outputs #:allow-other-keys)
+ (when tests?
+ (add-installed-pythonpath inputs outputs)
+ (invoke "pytest" "-v")))))))
(native-inputs
`(("python-pytest" ,python-pytest)))
(propagated-inputs
--
2.26.2
From a252a9d180804a6cb0d2829acd99a1010b4e1984 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:42:40 +0100
Subject: [PATCH 06/15] gnu: python-pyfakefs: Disable unreliable test.

* gnu/packages/check.scm (python-pyfakefs) [arguments]: Disable test.
---
gnu/packages/check.scm | 13 +++++++------
1 file changed, 7 insertions(+), 6 deletions(-)

Toggle diff (27 lines)
diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index 796635e012..0535a1b2c4 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -2858,13 +2858,14 @@ grew out of the @dfn{Vc} project.")
(arguments
`(#:phases
(modify-phases %standard-phases
- ;; The default test suite does not run these extra tests.
- (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"))))))
(native-inputs
`(("python-pytest" ,python-pytest)))
(build-system python-build-system)
--
2.26.2
From ced04daa680ef5f261b6f23e0500aca60032d323 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:48:36 +0100
Subject: [PATCH 07/15] gnu: python-slugify: Add missing input.

* gnu/packages/python-web.scm (python-slugify) [propagated-inputs]: Add
python-text-unidecode.
---
gnu/packages/python-web.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/python-web.scm b/gnu/packages/python-web.scm
index 622f5fc6e2..e3cf25a687 100644
--- a/gnu/packages/python-web.scm
+++ b/gnu/packages/python-web.scm
@@ -4398,7 +4398,8 @@ Python.")
(sha256
(base32 "0w22fapghmzk3xdasc4dn7h8sl58l08d1h5zbf72dh80drv1g9b9"))))
(propagated-inputs
- `(("python-unidecode" ,python-unidecode)))
+ `(("python-unidecode" ,python-unidecode)
+ ("python-text-unidecode" ,python-text-unidecode)))
(arguments
`(#:phases
(modify-phases %standard-phases
--
2.26.2
From 4412f706693caa7fd888a637ea66b65afd34a15b Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:49:01 +0100
Subject: [PATCH 08/15] gnu: python-websockets: Fix Python package name.

* gnu/packages/python-web.scm (python-websockets) [arguments]: Add new
phase to fix package name.
---
gnu/packages/python-web.scm | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

Toggle diff (24 lines)
diff --git a/gnu/packages/python-web.scm b/gnu/packages/python-web.scm
index e3cf25a687..e3318a18ce 100644
--- a/gnu/packages/python-web.scm
+++ b/gnu/packages/python-web.scm
@@ -5086,7 +5086,16 @@ Plus all the standard features of requests:
(base32
"03s3ml6sbki24aajllf8aily0xzrn929zxi84p50zkkbikdd4raw"))))
(build-system python-build-system)
- (arguments '(#:tests? #f)) ; Tests not included in release tarball.
+ (arguments
+ '(#:tests? #f ; Tests not included in release tarball.
+ #:phases
+ (modify-phases %standard-phases
+ (add-after 'unpack 'patch
+ (lambda* (#:key inputs #:allow-other-keys)
+ ;; Python package names use dot as separator.
+ (substitute* "setup.py"
+ (("websockets/extensions") "websockets.extensions"))
+ #t)))))
(home-page "https://github.com/aaugustin/websockets")
(synopsis
"Python implementation of the WebSocket Protocol (RFC 6455 & 7692)")
--
2.26.2
From f1b8ada28652fa85fa62075afb462a0611a50a50 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:50:18 +0100
Subject: [PATCH 09/15] gnu: python-black: Remove blackd.

* gnu/packages/python-xyz.scm (python-black) [arguments]: Add new phase
to prevent installation of blackd.
---
gnu/packages/python-xyz.scm | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

Toggle diff (22 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 818a244378..a02960f1c6 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -4277,7 +4277,14 @@ matching of file paths.")
(substitute* "tests/test_black.py"
(("( *)def test_python38" match indent)
(string-append indent "@unittest.skip(\"guix\")\n" match)))
- #t)))))
+ #t))
+ ;; Remove blackd, because it depends on python-aiohttp and
+ ;; python-aiohttp-cors.
+ (add-after 'unpack 'remove-entrypoint
+ (lambda _
+ (substitute* "setup.py"
+ (("\\s*\"blackd=blackd:patched_main \\[d\\]\",\n") "")
+ (("\"blackd\", ") "")))))))
(propagated-inputs
`(("python-click" ,python-click)
("python-attrs" ,python-attrs)
--
2.26.2
From df86a15fef25655c78faed179b25cd3c06e6b396 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:53:25 +0100
Subject: [PATCH 10/15] gnu: python-traitlets: Add missing input.

* gnu/packages/python-xyz.scm (python-traitlets) [propagated-inputs]:
Add python-six.
---
gnu/packages/python-xyz.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index a02960f1c6..4b7fff5750 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -6973,7 +6973,8 @@ cluster down and deletes the throwaway profile.")
(replace 'check (lambda _ (invoke "pytest" "-vv" "traitlets"))))))
(propagated-inputs
`(("python-ipython-genutils" ,python-ipython-genutils)
- ("python-decorator" ,python-decorator)))
+ ("python-decorator" ,python-decorator)
+ ("python-six" ,python-six)))
(native-inputs
`(("python-pytest" ,python-pytest)))
(properties `((python2-variant . ,(delay python2-traitlets))))
--
2.26.2
From f41d09b6acf31fc36b37f8fb895fb9ded52fe825 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:12:56 +0100
Subject: [PATCH 11/15] gnu: python-idna-ssl: Add missing input.

* gnu/packages/python-xyz.scm (python-idna-ssl) [propagated-inputs]: Add
python-idna.
---
gnu/packages/python-xyz.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (14 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 4b7fff5750..58ad3b6e55 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -9517,6 +9517,7 @@ is binding LibSass.")
(build-system python-build-system)
(arguments
`(#:tests? #f)) ;circular dependency with python-aiohttp
+ (propagated-inputs `(("python-idna" ,python-idna)))
(home-page "https://github.com/aio-libs/idna-ssl")
(synopsis "Patch @code{ssl.match_hostname} for Unicode(idna) domains support")
(description "Patch @code{ssl.match_hostname} for Unicode(idna)
--
2.26.2
From fe272f9e3a35a9a3b6b9994f3b6881fbe8facb7b Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:13:53 +0100
Subject: [PATCH 12/15] gnu: python-twisted: Remove broken console scripts.

* gnu/packages/python-xyz.scm (python-twisted) [arguments]: Patch
setup.py.
---
gnu/packages/python-xyz.scm | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

Toggle diff (22 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 58ad3b6e55..d8b61cd1d2 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -12716,7 +12716,14 @@ format.")
"17d3hnxv9qndagzz63mdpyk99xj63p9gq586vjn0rxk8cl197nym"))))
(build-system python-build-system)
(arguments
- '(#:tests? #f)) ; FIXME: some tests fail
+ '(#:tests? #f ; FIXME: some tests fail
+ #:phases
+ (modify-phases %standard-phases
+ ;; Remove scripts, because they depend on [conch]
+ (add-after 'unpack 'remove-entrypoint
+ (lambda _
+ (substitute* "src/twisted/python/_setup.py"
+ (("\".+ = twisted\\.conch\\.scripts\\..+\",") "")))))))
(propagated-inputs
`(("python-zope-interface" ,python-zope-interface)
("python-pyhamcrest" ,python-pyhamcrest)
--
2.26.2
From fef86cb5fc9dede96cc458c3801818f7cd6912cd Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:15:12 +0100
Subject: [PATCH 13/15] gnu: python-automat: Remove broken console script.

* gnu/packages/python-xyz.scm (python-automat) [arguments]: Patch
setup.py.
---
gnu/packages/python-xyz.scm | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

Toggle diff (23 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index d8b61cd1d2..acd674878a 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -15151,7 +15151,15 @@ instead of servers and network commands.")
;; python-twisted depends on python-automat. Twisted is optional, but the
;; tests fail if it is not available. Also see
;; <https://github.com/glyph/automat/issues/71>.
- (arguments '(#:tests? #f))
+ (arguments
+ `(#:tests? #f
+ #:phases
+ (modify-phases %standard-phases
+ ;; Remove script, because it depends on python-twisted.
+ (add-after 'unpack 'remove-entrypoint
+ (lambda _
+ (substitute* "setup.py"
+ (("\"automat-visualize = automat._visualize:tool\"") "")))))))
(native-inputs
`(("python-m2r" ,python-m2r)
("python-setuptools-scm" ,python-setuptools-scm)
--
2.26.2
From 9c0eb0a2ede51bc234cd5c4d7a1347fa3f83e843 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:16:05 +0100
Subject: [PATCH 14/15] gnu: python-packaging-bootstrap: Remove dependency.

* gnu/packages/python-xyz.scm (python-packaging-bootstrap) [arguments]:
Remove dependency from setup.py, which we do not provide for this
variant.
---
gnu/packages/python-xyz.scm | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

Toggle diff (27 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index acd674878a..e78016221f 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -16049,10 +16049,18 @@ information.")
(package/inherit
python-packaging
(name "python-packaging-bootstrap")
+ (arguments
+ (substitute-keyword-arguments (package-arguments python-packaging)
+ ((#:phases phases)
+ `(modify-phases ,phases
+ (add-after 'unpack 'fix-dependencies
+ (lambda* (#:key tests? #:allow-other-keys)
+ (substitute* "setup.py" (("\"six\"") ""))
+ #t))))
+ ((#:tests? _ #f) #f)))
(native-inputs '())
(propagated-inputs
- `(("python-pyparsing" ,python-pyparsing)))
- (arguments '(#:tests? #f)))))
+ `(("python-pyparsing" ,python-pyparsing))))))
(define-public python2-packaging-bootstrap
(hidden-package
--
2.26.2
From ecc8eebadbeb8c75ac5025d7bce581423d4d1894 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:17:20 +0100
Subject: [PATCH 15/15] gnu: python-traceback2: Add missing dependency.

* gnu/packages/python-xyz.scm (python-traceback2) [propagated-inputs]:
Add python-six.
---
gnu/packages/python-xyz.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index e78016221f..1df9807626 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -17266,7 +17266,8 @@ lines are read from a single file.")
(native-inputs
`(("python-pbr" ,python-pbr-minimal)))
(propagated-inputs
- `(("python-linecache2" ,python-linecache2)))
+ `(("python-linecache2" ,python-linecache2)
+ ("python-six" ,python-six)))
(home-page
"https://github.com/testing-cabal/traceback2")
(synopsis "Backports of the traceback module")
--
2.26.2
H
H
Hartmut Goebel wrote on 8 Jan 2021 12:37
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 45712@debbugs.gnu.org)
87mtxj1wym.fsf@lenashee.goebel-consult.de
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 :-)


Toggle quote (3 lines)
> +(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").

Toggle quote (14 lines)
> +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='')


Toggle quote (5 lines)
> + 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))


Toggle quote (11 lines)
> + 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':"

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

Here it is fine ;-)

Toggle quote (3 lines)
> + # 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.

Toggle quote (5 lines)
> + 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.


Toggle quote (2 lines)
> + (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?

Toggle quote (2 lines)
> +(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.

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

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



Toggle quote (6 lines)
> +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)


Toggle quote (6 lines)
> 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?

Toggle quote (5 lines)
> + ;; 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?

Toggle quote (3 lines)
> 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
…


Toggle quote (5 lines)
> 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.


Toggle quote (16 lines)
> 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.
R
R
Ricardo Wurmus wrote on 8 Jan 2021 13:19
(name . Hartmut Goebel)(address . hartmut@goebel-consult.de)
87pn2ffwoe.fsf@elephly.net
Hartmut Goebel <hartmut@goebel-consult.de> writes:

Toggle quote (12 lines)
> 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:

Toggle snippet (5 lines)
(define validate-script "\
from __future__ import print_function # Python 2 support.
…")

--
Ricardo
L
L
Lars-Dominik Braun wrote on 12 Jan 2021 10:37
(address . 45712@debbugs.gnu.org)
X/1tw1fRLS6JYVwu@noor.fritz.box
Hi Hartmut,

Toggle quote (2 lines)
> 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.

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

Toggle quote (3 lines)
> 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.

Toggle quote (3 lines)
> 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!

Toggle quote (5 lines)
> 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.

Toggle quote (2 lines)
> Please add a short explanation why there can be unavailable top-level
> modules.
Done.

Toggle quote (3 lines)
> 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?

Toggle quote (6 lines)
> 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.

Toggle quote (2 lines)
> 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?


Toggle quote (1 lines)
> Arguments are not used?
Fixed.

Toggle quote (1 lines)
> Any reason for relaxing this? Why not use python-pytest-6 as input?
Yes, our pytest@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@6
(works fine with PEP 517 compliant build system).

Toggle quote (2 lines)
> Please rephrase into something like
> Do not validate loadability
Done.

Toggle quote (2 lines)
> 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.

Toggle quote (2 lines)
> 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
Attachment: 0001-build-system-python-Validate-installed-package.patch
From e99707fb4cea9db88721192e84044dbdf1b1ca9b Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:25:56 +0100
Subject: [PATCH v2 02/16] gnu: pytest@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(-)

Toggle diff (23 lines)
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
--
2.26.2
From 23077674975a0194f38ed70d0a912ab52c4af3fe Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:27:55 +0100
Subject: [PATCH v2 03/16] 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(-)

Toggle diff (43 lines)
diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index 9d1e0b8173..2136b8fc4f 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-setup-py
+ (lambda _
+ ;; Relax pytest requirement.
+ (substitute* "setup.py"
+ (("pytest>=6\\.0\\.0") "pytest"))
+ #t))
+ (replace 'check
+ (lambda* (#:key tests? inputs outputs #:allow-other-keys)
+ (when tests?
+ (add-installed-pythonpath inputs outputs)
+ (invoke "py.test" "-v")))))))
(native-inputs
`(("python-setuptools-scm" ,python-setuptools-scm)))
(propagated-inputs
`(("python-execnet" ,python-execnet)
("python-pytest" ,python-pytest)
- ("python-py" ,python-py)))
+ ("python-py" ,python-py)
+ ("python-pytest-forked" ,python-pytest-forked)))
(home-page
"https://github.com/pytest-dev/pytest-xdist")
(synopsis
--
2.26.2
From 478a266a4299a05835bbfc19099a1f0afc318d8a Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:29:59 +0100
Subject: [PATCH v2 04/16] gnu: python-fixtures-bootstrap: Do not validate
loadability

* gnu/packages/check.scm (python-fixtures-bootstrap) [arguments]: Delete
'validate-loadable.
---
gnu/packages/check.scm | 7 ++++++-
1 file changed, 6 insertions(+), 1 deletion(-)

Toggle diff (20 lines)
diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index 2136b8fc4f..e4761eb66a 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -1541,7 +1541,12 @@ protocol.")))
(base32
"1vxj29bzz3rd4pcy51d05wng9q9dh4jq6wx92yklsm7i6h1ddw7w"))))
(build-system python-build-system)
- (arguments `(#:tests? #f))
+ (arguments
+ `(#:tests? #f
+ #:phases
+ (modify-phases %standard-phases
+ ;; Package is not loadable on its own at this stage.
+ (delete 'validate-loadable))))
(propagated-inputs
`(("python-pbr-minimal" ,python-pbr-minimal)
("python-six" ,python-six)))
--
2.26.2
From ac7d664c3748afb9059ed3d147c13a9033fff45b Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:35:11 +0100
Subject: [PATCH v2 05/16] gnu: python-pytest-pep8: Fix package.

* gnu/packages/check.scm (python-pytest-pep8) [arguments]: Remove
dependency on pytest-cache and add proper 'check phase.
---
gnu/packages/check.scm | 14 +++++++++++++-
1 file changed, 13 insertions(+), 1 deletion(-)

Toggle diff (27 lines)
diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index e4761eb66a..c12cb98a6c 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -2043,7 +2043,19 @@ failures.")
"06032agzhw1i9d9qlhfblnl3dw5hcyxhagn7b120zhrszbjzfbh3"))))
(build-system python-build-system)
(arguments
- `(#:tests? #f)) ; Fails with recent pytest and pep8. See upstream issues #8 and #12.
+ `(#:tests? #f ; Fails with recent pytest and pep8. See upstream issues #8 and #12.
+ #:phases
+ (modify-phases %standard-phases
+ (add-after 'unpack 'fix-dependencies
+ (lambda _
+ (substitute* "setup.py"
+ (("'pytest-cache', ") "")) ; Included in recent pytest
+ #t))
+ (replace 'check
+ (lambda* (#:key tests? inputs outputs #:allow-other-keys)
+ (when tests?
+ (add-installed-pythonpath inputs outputs)
+ (invoke "pytest" "-v")))))))
(native-inputs
`(("python-pytest" ,python-pytest)))
(propagated-inputs
--
2.26.2
From e76db5088bafd7a3788a80a834d4983e0eed0e6a Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Mon, 11 Jan 2021 14:44:02 +0100
Subject: [PATCH v2 06/16] gnu: python-pyfakefs: Disable unreliable test

* gnu/packages/check.scm (python-pyfakefs) [arguments]: Add new phase to
skip single test.
---
gnu/packages/check.scm | 10 ++++++++++
1 file changed, 10 insertions(+)

Toggle diff (23 lines)
diff --git a/gnu/packages/check.scm b/gnu/packages/check.scm
index c12cb98a6c..9ddf656324 100644
--- a/gnu/packages/check.scm
+++ b/gnu/packages/check.scm
@@ -2858,6 +2858,16 @@ grew out of the @dfn{Vc} project.")
(arguments
`(#:phases
(modify-phases %standard-phases
+ (add-after 'unpack 'patch-testsuite
+ (lambda _
+ ;; Time difference is larger than expected.
+ (substitute* "pyfakefs/tests/fake_filesystem_unittest_test.py"
+ (("(\\s+)def test_copy_real_file" all indent)
+ (string-append
+ indent
+ "@unittest.skip('disabled by guix')\n"
+ all)))
+ #t))
;; The default test suite does not run these extra tests.
(add-after 'check 'check-pytest-plugin
(lambda _
--
2.26.2
From 203cd356f44081a753b1f67ec14ab00ec1bd8520 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:48:36 +0100
Subject: [PATCH v2 07/16] gnu: python-slugify: Add missing input.

* gnu/packages/python-web.scm (python-slugify) [propagated-inputs]: Add
python-text-unidecode.
---
gnu/packages/python-web.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/python-web.scm b/gnu/packages/python-web.scm
index 622f5fc6e2..e3cf25a687 100644
--- a/gnu/packages/python-web.scm
+++ b/gnu/packages/python-web.scm
@@ -4398,7 +4398,8 @@ Python.")
(sha256
(base32 "0w22fapghmzk3xdasc4dn7h8sl58l08d1h5zbf72dh80drv1g9b9"))))
(propagated-inputs
- `(("python-unidecode" ,python-unidecode)))
+ `(("python-unidecode" ,python-unidecode)
+ ("python-text-unidecode" ,python-text-unidecode)))
(arguments
`(#:phases
(modify-phases %standard-phases
--
2.26.2
From fe3ec027ce3fe198a2cdfe3e1329094f2525fa42 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:49:01 +0100
Subject: [PATCH v2 08/16] gnu: python-websockets: Fix Python package name.

* gnu/packages/python-web.scm (python-websockets) [arguments]: Add new
phase to fix package name.
---
gnu/packages/python-web.scm | 11 ++++++++++-
1 file changed, 10 insertions(+), 1 deletion(-)

Toggle diff (24 lines)
diff --git a/gnu/packages/python-web.scm b/gnu/packages/python-web.scm
index e3cf25a687..e3318a18ce 100644
--- a/gnu/packages/python-web.scm
+++ b/gnu/packages/python-web.scm
@@ -5086,7 +5086,16 @@ Plus all the standard features of requests:
(base32
"03s3ml6sbki24aajllf8aily0xzrn929zxi84p50zkkbikdd4raw"))))
(build-system python-build-system)
- (arguments '(#:tests? #f)) ; Tests not included in release tarball.
+ (arguments
+ '(#:tests? #f ; Tests not included in release tarball.
+ #:phases
+ (modify-phases %standard-phases
+ (add-after 'unpack 'patch
+ (lambda* (#:key inputs #:allow-other-keys)
+ ;; Python package names use dot as separator.
+ (substitute* "setup.py"
+ (("websockets/extensions") "websockets.extensions"))
+ #t)))))
(home-page "https://github.com/aaugustin/websockets")
(synopsis
"Python implementation of the WebSocket Protocol (RFC 6455 & 7692)")
--
2.26.2
From d8199ab0353aa215d193ca511a63d4eea355b6c6 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:50:18 +0100
Subject: [PATCH v2 09/16] gnu: python-black: Remove blackd.

* gnu/packages/python-xyz.scm (python-black) [arguments]: Add new phase
to prevent installation of blackd.
---
gnu/packages/python-xyz.scm | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

Toggle diff (22 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 818a244378..a02960f1c6 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -4277,7 +4277,14 @@ matching of file paths.")
(substitute* "tests/test_black.py"
(("( *)def test_python38" match indent)
(string-append indent "@unittest.skip(\"guix\")\n" match)))
- #t)))))
+ #t))
+ ;; Remove blackd, because it depends on python-aiohttp and
+ ;; python-aiohttp-cors.
+ (add-after 'unpack 'remove-entrypoint
+ (lambda _
+ (substitute* "setup.py"
+ (("\\s*\"blackd=blackd:patched_main \\[d\\]\",\n") "")
+ (("\"blackd\", ") "")))))))
(propagated-inputs
`(("python-click" ,python-click)
("python-attrs" ,python-attrs)
--
2.26.2
From b7ec4d0705bab46fa682a79de530336e2ff46775 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 13:53:25 +0100
Subject: [PATCH v2 10/16] gnu: python-traitlets: Add missing input.

* gnu/packages/python-xyz.scm (python-traitlets) [propagated-inputs]:
Add python-six.
---
gnu/packages/python-xyz.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index a02960f1c6..4b7fff5750 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -6973,7 +6973,8 @@ cluster down and deletes the throwaway profile.")
(replace 'check (lambda _ (invoke "pytest" "-vv" "traitlets"))))))
(propagated-inputs
`(("python-ipython-genutils" ,python-ipython-genutils)
- ("python-decorator" ,python-decorator)))
+ ("python-decorator" ,python-decorator)
+ ("python-six" ,python-six)))
(native-inputs
`(("python-pytest" ,python-pytest)))
(properties `((python2-variant . ,(delay python2-traitlets))))
--
2.26.2
From e17eb838b9d77588200ac6db652e87bcd73e3f87 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:12:56 +0100
Subject: [PATCH v2 11/16] gnu: python-idna-ssl: Add missing input.

* gnu/packages/python-xyz.scm (python-idna-ssl) [propagated-inputs]: Add
python-idna.
---
gnu/packages/python-xyz.scm | 1 +
1 file changed, 1 insertion(+)

Toggle diff (14 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 4b7fff5750..58ad3b6e55 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -9517,6 +9517,7 @@ is binding LibSass.")
(build-system python-build-system)
(arguments
`(#:tests? #f)) ;circular dependency with python-aiohttp
+ (propagated-inputs `(("python-idna" ,python-idna)))
(home-page "https://github.com/aio-libs/idna-ssl")
(synopsis "Patch @code{ssl.match_hostname} for Unicode(idna) domains support")
(description "Patch @code{ssl.match_hostname} for Unicode(idna)
--
2.26.2
From 3372011b15cd28b203590981f5e1c561f977e31f Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:13:53 +0100
Subject: [PATCH v2 12/16] gnu: python-twisted: Remove broken console scripts.

* gnu/packages/python-xyz.scm (python-twisted) [arguments]: Patch
setup.py.
---
gnu/packages/python-xyz.scm | 9 ++++++++-
1 file changed, 8 insertions(+), 1 deletion(-)

Toggle diff (22 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 58ad3b6e55..d8b61cd1d2 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -12716,7 +12716,14 @@ format.")
"17d3hnxv9qndagzz63mdpyk99xj63p9gq586vjn0rxk8cl197nym"))))
(build-system python-build-system)
(arguments
- '(#:tests? #f)) ; FIXME: some tests fail
+ '(#:tests? #f ; FIXME: some tests fail
+ #:phases
+ (modify-phases %standard-phases
+ ;; Remove scripts, because they depend on [conch]
+ (add-after 'unpack 'remove-entrypoint
+ (lambda _
+ (substitute* "src/twisted/python/_setup.py"
+ (("\".+ = twisted\\.conch\\.scripts\\..+\",") "")))))))
(propagated-inputs
`(("python-zope-interface" ,python-zope-interface)
("python-pyhamcrest" ,python-pyhamcrest)
--
2.26.2
From 7f644bb845249c32a2504fad0625dd75a6c01ce1 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:15:12 +0100
Subject: [PATCH v2 13/16] gnu: python-automat: Remove broken console script.

* gnu/packages/python-xyz.scm (python-automat) [arguments]: Patch
setup.py.
---
gnu/packages/python-xyz.scm | 10 +++++++++-
1 file changed, 9 insertions(+), 1 deletion(-)

Toggle diff (23 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index d8b61cd1d2..acd674878a 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -15151,7 +15151,15 @@ instead of servers and network commands.")
;; python-twisted depends on python-automat. Twisted is optional, but the
;; tests fail if it is not available. Also see
;; <https://github.com/glyph/automat/issues/71>.
- (arguments '(#:tests? #f))
+ (arguments
+ `(#:tests? #f
+ #:phases
+ (modify-phases %standard-phases
+ ;; Remove script, because it depends on python-twisted.
+ (add-after 'unpack 'remove-entrypoint
+ (lambda _
+ (substitute* "setup.py"
+ (("\"automat-visualize = automat._visualize:tool\"") "")))))))
(native-inputs
`(("python-m2r" ,python-m2r)
("python-setuptools-scm" ,python-setuptools-scm)
--
2.26.2
From ff4f539c9c4cd18e7f5c9bea4f87eec42ee81394 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:16:05 +0100
Subject: [PATCH v2 14/16] gnu: python-packaging-bootstrap: Remove dependency.

* gnu/packages/python-xyz.scm (python-packaging-bootstrap) [arguments]:
Remove dependency from setup.py, which we do not provide for this
variant.
---
gnu/packages/python-xyz.scm | 12 ++++++++++--
1 file changed, 10 insertions(+), 2 deletions(-)

Toggle diff (27 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index acd674878a..e78016221f 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -16049,10 +16049,18 @@ information.")
(package/inherit
python-packaging
(name "python-packaging-bootstrap")
+ (arguments
+ (substitute-keyword-arguments (package-arguments python-packaging)
+ ((#:phases phases)
+ `(modify-phases ,phases
+ (add-after 'unpack 'fix-dependencies
+ (lambda* (#:key tests? #:allow-other-keys)
+ (substitute* "setup.py" (("\"six\"") ""))
+ #t))))
+ ((#:tests? _ #f) #f)))
(native-inputs '())
(propagated-inputs
- `(("python-pyparsing" ,python-pyparsing)))
- (arguments '(#:tests? #f)))))
+ `(("python-pyparsing" ,python-pyparsing))))))
(define-public python2-packaging-bootstrap
(hidden-package
--
2.26.2
From 65d2ab65d4dd73e1bdb0248cc41848c55859f98d Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Thu, 7 Jan 2021 14:17:20 +0100
Subject: [PATCH v2 15/16] gnu: python-traceback2: Add missing dependency.

* gnu/packages/python-xyz.scm (python-traceback2) [propagated-inputs]:
Add python-six.
---
gnu/packages/python-xyz.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index e78016221f..1df9807626 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -17266,7 +17266,8 @@ lines are read from a single file.")
(native-inputs
`(("python-pbr" ,python-pbr-minimal)))
(propagated-inputs
- `(("python-linecache2" ,python-linecache2)))
+ `(("python-linecache2" ,python-linecache2)
+ ("python-six" ,python-six)))
(home-page
"https://github.com/testing-cabal/traceback2")
(synopsis "Backports of the traceback module")
--
2.26.2
From 55aab0eba972c6672be9d8de5e0ec9340b10c243 Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Mon, 11 Jan 2021 15:19:01 +0100
Subject: [PATCH v2 16/16] gnu: python2-packaging-bootstrap: Add missing
dependency

* gnu/packages/python-xyz.scm (python2-packaging-bootstrap)
[propagated-inputs]: Add python2-six-bootstrap.
---
gnu/packages/python-xyz.scm | 3 ++-
1 file changed, 2 insertions(+), 1 deletion(-)

Toggle diff (16 lines)
diff --git a/gnu/packages/python-xyz.scm b/gnu/packages/python-xyz.scm
index 1df9807626..434946acfd 100644
--- a/gnu/packages/python-xyz.scm
+++ b/gnu/packages/python-xyz.scm
@@ -16069,7 +16069,8 @@ information.")
(name "python2-packaging-bootstrap")
(native-inputs '())
(propagated-inputs
- `(("python-pyparsing" ,python2-pyparsing)))
+ `(("python-pyparsing" ,python2-pyparsing)
+ ("python-six" ,python2-six-bootstrap)))
(arguments
`(#:tests? #f
,@(package-arguments python2-packaging))))))
--
2.26.2
Attachment: failing-packages.txt
L
L
Ludovic Courtès wrote on 13 Jan 2021 15:32
control message for bug #45712
(address . control@debbugs.gnu.org)
87turkaovl.fsf@gnu.org
severity 45712 important
quit
M
M
Maxim Cournoyer wrote on 25 Jan 2021 15:43
Re: bug#45712: [PATCHES] Improve Python package quality
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 45712@debbugs.gnu.org)
8735yp3wmb.fsf@gmail.com
Hi!

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

Toggle quote (14 lines)
> 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
M
M
Maxim Cournoyer wrote on 25 Jan 2021 15:48
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 45712@debbugs.gnu.org)
87v9bl2hu6.fsf@gmail.com
Hi again,

[...]

Toggle quote (34 lines)
>>From cf9ae80b59e86a60c27734c8cc27637757490d70 Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Thu, 7 Jan 2021 13:25:56 +0100
> Subject: [PATCH 02/15] gnu: pytest@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.
M
M
Maxim Cournoyer wrote on 25 Jan 2021 20:29
(name . Lars-Dominik Braun)(address . lars@6xq.net)
87czxs3jel.fsf_-_@gmail.com
Hi Lars-Dominik,

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

Toggle quote (10 lines)
> 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
Attachment: 0001-build-system-python-Add-a-sanity-check-phase.patch
L
L
Lars-Dominik Braun wrote on 25 Jan 2021 20:42
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 45712@debbugs.gnu.org)
YA8fImGp75KEIhuj@noor.fritz.box
Hi Maxim,

Toggle quote (5 lines)
> 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
I’ll send an updated patch to guix-patches@ as soon as I have the 'check
phase working.

Cheers,
Lars

L
L
Lars-Dominik Braun wrote on 26 Jan 2021 09:39
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 45712@debbugs.gnu.org)
YA/VOpukAbOMnxyM@noor.fritz.box
Hi Maxim,

Toggle quote (4 lines)
> 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
M
M
Maxim Cournoyer wrote on 28 Jan 2021 16:40
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 45712@debbugs.gnu.org)
874kj1ysr9.fsf_-_@gmail.com
Hi Lars,

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

Toggle quote (24 lines)
> 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
L
L
Lars-Dominik Braun wrote on 28 Jan 2021 17:18
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 45712@debbugs.gnu.org)
YBLj35oSVDaPQCYo@noor.fritz.box
Hi Maxim,

Toggle quote (3 lines)
> 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
M
M
Maxim Cournoyer wrote on 29 Jan 2021 15:14
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 45712@debbugs.gnu.org)
87k0rvygmn.fsf@gmail.com
Lars-Dominik Braun <lars@6xq.net> writes:

[...]

Toggle quote (38 lines)
>>From 9bc53a8e9706440668ec70d88db1ebc7d5e2c71d Mon Sep 17 00:00:00 2001
> From: Lars-Dominik Braun <lars@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).
M
M
Maxim Cournoyer wrote on 29 Jan 2021 15:26
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 45712@debbugs.gnu.org)
87h7mzyg39.fsf_-_@gmail.com
Hi Lars,

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

Toggle quote (13 lines)
> 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
L
L
Lars-Dominik Braun wrote on 1 Feb 2021 08:20
(name . Maxim Cournoyer)(address . maxim.cournoyer@gmail.com)(address . 45712@debbugs.gnu.org)
YBerzU9YrzFoCl9J@noor.fritz.box
Hi Maxim,

Toggle quote (7 lines)
> 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!

Toggle quote (3 lines)
> 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.

Toggle quote (2 lines)
> 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
M
M
Maxim Cournoyer wrote on 1 Feb 2021 18:02
(name . Lars-Dominik Braun)(address . lars@6xq.net)(address . 45712-done@debbugs.gnu.org)
87czxjsouj.fsf@gmail.com
Hi Lars,

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

Toggle quote (23 lines)
> 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
Closed
H
H
Hartmut Goebel wrote on 7 Feb 2021 17:59
6ace098f-4e4a-0f6f-ba09-f66728bdb564@goebel-consult.de
Hi Maxim,
Toggle quote (7 lines)
>
> 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@goebel-consult.de | www.goebel-consult.de |
Attachment: file
L
L
Lars-Dominik Braun wrote on 8 Feb 2021 09:02
(name . Hartmut Goebel)(address . hartmut@goebel-consult.de)
YCDwHTUjWZlT32ow@noor.fritz.box
Hi Hartmut,

Toggle quote (2 lines)
> 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
?
Your comment

This issue is archived.

To comment on this conversation send an email to 45712@debbugs.gnu.org

To respond to this issue using the mumi CLI, first switch to it
mumi current 45712
Then, you may apply the latest patchset in this issue (with sign off)
mumi am -- -s
Or, compose a reply to this issue
mumi compose
Or, send patches to this issue
mumi send-email *.patch