unofficial mirror of guix-devel@gnu.org 
 help / color / mirror / code / Atom feed
From: Lars-Dominik Braun <lars@6xq.net>
To: guix-devel@gnu.org
Subject: [RFC] Improve Python package quality
Date: Sun, 3 Jan 2021 11:00:56 +0100	[thread overview]
Message-ID: <X/GV2JXi/iOU2oAZ@noor.fritz.box> (raw)

[-- Attachment #1: Type: text/plain, Size: 1624 bytes --]

Hi,

I’d like to propose adding an additional phase to python-build-system to
improve Guix’ Python package quality.

Python is an interpreted language without any compile-time checks. Any
errors are only visible at run-time, including missing or wrong imports.
Additionally most Python packages use an additional packaging layer
through setuptools, which also declares mandatory and optional
dependencies.

This can result in buildable, but broken packages. An example of a
botched upgrade is fixed in commit
22e06297b1982f75aaadddba616b1052e506e4a0. The package python-httpx was
upgraded, but that new version declared a dependency to a newer
python-httpcore via setuptools, which wasn’t available and thus packages
depending on python-httpx were broken.

My proposal adds some build-time checks to guarentee three properties:

1) Depending on the package (distribution in Python language) via
setuptools works. This ensures dependencies are complete and have the
correct version as determined by setup.{py,cfg}.
2) Console entry points can be loaded. This ensures scripts installed to
bin/ actually work.
3) Top-level modules can be imported. This ensures the package is
actually usable and has no undeclared dependencies.

The attached patch implements all three. I’ve been rebuilding a few
Python packages using the patch and uncovered some issues already. I’m
sure it’s not perfect yet and thus I’m open to improvements. If this
idea is well received I’m willing to rebuild and fix *all* Python
packages broken by this new phase. I’m also aware this change needs to
go to core-updates.

Cheers,
Lars


[-- Attachment #2: 0001-WIP-build-system-python-Validate-installed-package.patch --]
[-- Type: text/x-diff, Size: 3449 bytes --]

From 919fd78b1aff6c277baca396ef962a5dcd4e23ae Mon Sep 17 00:00:00 2001
From: Lars-Dominik Braun <lars@6xq.net>
Date: Sun, 3 Jan 2021 10:30:29 +0100
Subject: [PATCH] [WIP] build-system/python: Validate installed package

* guix/build/python-build-system.scm (validate-loadable): New phase.
(%standard-phases): Use it.
---
 guix/build/python-build-system.scm | 51 ++++++++++++++++++++++++++++++
 1 file changed, 51 insertions(+)

diff --git a/guix/build/python-build-system.scm b/guix/build/python-build-system.scm
index 09bd8465c8..edb772a7a4 100644
--- a/guix/build/python-build-system.scm
+++ b/guix/build/python-build-system.scm
@@ -148,6 +148,56 @@
       (format #t "test suite not run~%"))
   #t)
 
+(define* (validate-loadable #:key tests? inputs outputs #:allow-other-keys)
+  "Ensure packages depending on this package via setuptools work properly,
+their advertised endpoints work and their top level modules are importable
+without errors."
+  (let ((script (string-join
+'(
+;; Python 2 support.
+"from __future__ import print_function"
+"import pkg_resources, sys, importlib"
+;; Only check site-packages installed by this package, but not dependencies
+;; (which pkg_resources.working_set would include). Path supplied via argv.
+"ws = pkg_resources.find_distributions (sys.argv[1])"
+"for dist in ws:"
+"    print ('validating', repr (dist.project_name), dist.location)"
+"    try:"
+"        req = str (dist.as_requirement ())"
+;; dist.activate() is not enough to actually check requirements, we have to
+;; .require() it.
+"        pkg_resources.require (req)"
+"    except Exception as e:"
+"        print (req, e)"
+"        sys.exit (1)"
+;; 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', }:"
+"           continue"
+"       for name, ep in v.items ():"
+"           print ('...trying to load endpoint', group, name)"
+"           ep.load ()"
+;; And finally try to load top level modules. This should not have any
+;; side-effects.
+"    for name in dist.get_metadata_lines ('top_level.txt'):"
+"        print ('...trying to load module', name)"
+"        try:"
+"            importlib.import_module (name)"
+;; Ignore non-existent modules, we only want to know if the existing ones work.
+"        except ModuleNotFoundError:"
+"            print ('......WARNING: module', name, 'not found, continuing')"
+"            continue")
+"\n")))
+    (add-installed-pythonpath inputs outputs)
+    ;; Make sure the working directory is empty (i.e. no Python modules in it)
+    (with-directory-excursion "/tmp"
+    ;; XXX: Cloak command run. Long and unreadable if it fails, provide an
+    ;; explanation instead.
+      (invoke "python" "-c" script (site-packages inputs outputs))))
+  #t)
+
 (define (python-version python)
   (let* ((version     (last (string-split python #\-)))
          (components  (string-split version #\.))
@@ -267,6 +317,7 @@ installed with setuptools."
     (replace 'install install)
     (add-after 'install 'check check)
     (add-after 'install 'wrap wrap)
+    (add-after 'check 'validate-loadable validate-loadable)
     (add-before 'strip 'rename-pth-file rename-pth-file)))
 
 (define* (python-build #:key inputs (phases %standard-phases)
-- 
2.26.2


             reply	other threads:[~2021-01-03 10:01 UTC|newest]

Thread overview: 9+ messages / expand[flat|nested]  mbox.gz  Atom feed  top
2021-01-03 10:00 Lars-Dominik Braun [this message]
2021-01-03 10:48 ` [RFC] Improve Python package quality Hartmut Goebel
2021-01-05  9:19   ` Lars-Dominik Braun
2021-01-03 15:04 ` Tobias Geerinckx-Rice
2021-01-05  9:31 ` Vincent Legoll
2021-01-05 10:20   ` Lars-Dominik Braun
2021-01-05 10:48     ` Vincent Legoll
2021-01-06 16:09       ` Hartmut Goebel
2021-01-07 13:31 ` Lars-Dominik Braun

Reply instructions:

You may reply publicly to this message via plain-text email
using any one of the following methods:

* Save the following mbox file, import it into your mail client,
  and reply-to-all from there: mbox

  Avoid top-posting and favor interleaved quoting:
  https://en.wikipedia.org/wiki/Posting_style#Interleaved_style

  List information: https://guix.gnu.org/

* Reply using the --to, --cc, and --in-reply-to
  switches of git-send-email(1):

  git send-email \
    --in-reply-to=X/GV2JXi/iOU2oAZ@noor.fritz.box \
    --to=lars@6xq.net \
    --cc=guix-devel@gnu.org \
    /path/to/YOUR_REPLY

  https://kernel.org/pub/software/scm/git/docs/git-send-email.html

* If your mail client supports setting the In-Reply-To header
  via mailto: links, try the mailto: link
Be sure your reply has a Subject: header at the top and a blank line before the message body.
Code repositories for project(s) associated with this public inbox

	https://git.savannah.gnu.org/cgit/guix.git

This is a public inbox, see mirroring instructions
for how to clone and mirror all data and code used for this inbox;
as well as URLs for read-only IMAP folder(s) and NNTP newsgroup(s).