[bug#45712] [PATCHES] Improve Python package quality

[Maxim Cournoyer <maxim.cournoyer@gmail.com>]

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

Hi Lars-Dominik,

Lars-Dominik Braun <lars@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

[-- Attachment #2: 0001-build-system-python-Add-a-sanity-check-phase.patch --]
[-- Type: text/x-patch, Size: 16346 bytes --]

From 2df41c3fb476822efac1aa8dac8368e91a0e360a 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] build-system/python: Add a sanity check phase.

Add a new phase validating the usability of installed Python packages.

* gnu/packages/aux-files/python/sanity-check.py: New file.
* Makefile.am (AUX_FILES): Register it.
* guix/build-system/python.scm (sanity-check.py): New variable.
(lower): Add the script as an implicit input.
* guix/build/python-build-system.scm: Remove trailing #t.
(sanity-check): New phase.
(%standard-phases): Use it.
* tests/builders.scm (test-build-package): New syntax.
("python-build-system: dummy-ok")
("python-build-system: dummy-fail-requirements")
("python-build-system: dummy-fail-import")
("python-build-system: dummy-fail-console-script"): Add tests.
---
 Makefile.am                                   |   1 +
 gnu/packages/aux-files/python/sanity-check.py |  85 ++++++++++++++
 guix/build-system/python.scm                  |   8 ++
 guix/build/python-build-system.scm            |  26 +++--
 tests/builders.scm                            | 106 +++++++++++++++++-
 5 files changed, 212 insertions(+), 14 deletions(-)
 create mode 100644 gnu/packages/aux-files/python/sanity-check.py

diff --git a/Makefile.am b/Makefile.am
index dc5cf9babc..dddae69ff1 100644
--- a/Makefile.am
+++ b/Makefile.am
@@ -380,6 +380,7 @@ AUX_FILES =						\
   gnu/packages/aux-files/linux-libre/4.4-i686.conf	\
   gnu/packages/aux-files/linux-libre/4.4-x86_64.conf	\
   gnu/packages/aux-files/pack-audit.c			\
+  gnu/packages/aux-files/python/sanity-check.py		\
   gnu/packages/aux-files/run-in-namespace.c
 
 # Templates, examples.
diff --git a/gnu/packages/aux-files/python/sanity-check.py b/gnu/packages/aux-files/python/sanity-check.py
new file mode 100644
index 0000000000..1ba912a931
--- /dev/null
+++ b/gnu/packages/aux-files/python/sanity-check.py
@@ -0,0 +1,85 @@
+# GNU Guix --- Functional package management for GNU
+# Copyright © 2021 Lars-Dominik Braun <lars@6xq.net>
+#
+# This file is part of GNU Guix.
+#
+# GNU Guix is free software; you can redistribute it and/or modify it
+# under the terms of the GNU General Public License as published by
+# the Free Software Foundation; either version 3 of the License, or (at
+# your option) any later version.
+#
+# GNU Guix is distributed in the hope that it will be useful, but
+# WITHOUT ANY WARRANTY; without even the implied warranty of
+# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE.  See the
+# GNU General Public License for more details.
+#
+# You should have received a copy of the GNU General Public License
+# along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
+
+from __future__ import print_function  # Python 2 support.
+import importlib
+import pkg_resources
+import sys
+import traceback
+
+try:
+    from importlib.machinery import PathFinder
+except ImportError:
+    PathFinder = None
+
+ret = 0
+
+# 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:
+        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)
+        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', }:
+            continue
+        for name, ep in v.items():
+            try:
+                print('...trying to load endpoint', group, name, end=': ')
+                ep.load()
+                print('OK')
+            except Exception:
+                print('ERROR:')
+                traceback.print_exc(file=sys.stdout)
+                ret = 1
+                continue
+
+    # 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'):
+        # Only available on Python 3.
+        if PathFinder and PathFinder.find_spec(name) is None:
+            # Ignore unavailable modules. Cannot use ModuleNotFoundError,
+            # because it is raised by failed imports too.
+            continue
+        try:
+            print('...trying to load module', name, end=': ')
+            importlib.import_module(name)
+            print('OK')
+        except Exception:
+            print('ERROR:')
+            traceback.print_exc(file=sys.stdout)
+            ret = 1
+            continue
+
+sys.exit(ret)
diff --git a/guix/build-system/python.scm b/guix/build-system/python.scm
index e39c06528e..2bb6fa87ca 100644
--- a/guix/build-system/python.scm
+++ b/guix/build-system/python.scm
@@ -2,6 +2,7 @@
 ;;; Copyright © 2013, 2014, 2015, 2016, 2017 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2013 Andreas Enge <andreas@enge.fr>
 ;;; Copyright © 2013 Nikita Karetnikov <nikita@karetnikov.org>
+;;; Copyright © 2021 Lars-Dominik Braun <lars@6xq.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -19,6 +20,8 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 (define-module (guix build-system python)
+  #:use-module ((gnu packages) #:select (search-auxiliary-file))
+  #:use-module (guix gexp)
   #:use-module (guix store)
   #:use-module (guix utils)
   #:use-module (guix memoization)
@@ -70,6 +73,10 @@ extension, such as '.tar.gz'."
   (let ((python (resolve-interface '(gnu packages python))))
     (module-ref python 'python-2)))
 
+(define sanity-check.py
+  ;; The script used to validate the installation of a Python package.
+  (search-auxiliary-file "python/sanity-check.py"))
+
 (define* (package-with-explicit-python python old-prefix new-prefix
                                        #:key variant-property)
   "Return a procedure of one argument, P.  The procedure creates a package with
@@ -156,6 +163,7 @@ pre-defined variants."
                         ;; Keep the standard inputs of 'gnu-build-system'.
                         ,@(standard-packages)))
          (build-inputs `(("python" ,python)
+                         ("sanity-check.py" ,(local-file sanity-check.py))
                          ,@native-inputs))
          (outputs outputs)
          (build python-build)
diff --git a/guix/build/python-build-system.scm b/guix/build/python-build-system.scm
index 7fef0b2278..1f11dd2b0a 100644
--- a/guix/build/python-build-system.scm
+++ b/guix/build/python-build-system.scm
@@ -9,6 +9,7 @@
 ;;; Copyright © 2019, 2020, 2021 Maxim Cournoyer <maxim.cournoyer@gmail.com>
 ;;; Copyright © 2020 Jakub Kądziołka <kuba@kadziolka.net>
 ;;; Copyright © 2020 Efraim Flashner <efraim@flashner.co.il>
+;;; Copyright © 2021 Lars-Dominik Braun <lars@6xq.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -134,6 +135,15 @@
              (apply invoke "python" "./setup.py" command params)))
       (error "no setup.py found")))
 
+(define* (sanity-check #: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 ((sanity-check.py (assoc-ref inputs "sanity-check.py")))
+    ;; Make sure the working directory is empty (i.e. no Python modules in it)
+    (with-directory-excursion "/tmp"
+      (invoke "python" sanity-check.py (site-packages inputs outputs)))))
+
 (define* (build #:key use-setuptools? #:allow-other-keys)
   "Build a given Python package."
   (call-setuppy "build" '() use-setuptools?)
@@ -225,8 +235,7 @@ useful when running checks after installing the package."
     ;; '--invalidation-mode' option, do not generate any.
     (unless <3.7?
       (invoke "python" "-m" "compileall" "--invalidation-mode=unchecked-hash"
-              out))
-    #t))
+              out))))
 
 (define* (wrap #:key inputs outputs #:allow-other-keys)
   (let ((pythonpath (guix-pythonpath inputs)))
@@ -262,8 +271,7 @@ installed with setuptools."
          (easy-install-pth (string-append site-packages "/easy-install.pth"))
          (new-pth (string-append site-packages "/" name ".pth")))
     (when (file-exists? easy-install-pth)
-      (rename-file easy-install-pth new-pth))
-    #t))
+      (rename-file easy-install-pth new-pth))))
 
 (define* (ensure-no-mtimes-pre-1980 #:rest _)
   "Ensure that there are no mtimes before 1980-01-02 in the source tree."
@@ -275,8 +283,7 @@ installed with setuptools."
     (ftw "." (lambda (file stat flag)
                (unless (<= early-1980 (stat:mtime stat))
                  (utime file early-1980 early-1980))
-               #t))
-    #t))
+               #t))))
 
 (define* (enable-bytecode-determinism #:rest _)
   "Improve determinism of pyc files."
@@ -284,8 +291,7 @@ installed with setuptools."
   (setenv "PYTHONHASHSEED" "0")
   ;; Prevent Python from creating .pyc files when loading modules (such as
   ;; when running a test suite).
-  (setenv "PYTHONDONTWRITEBYTECODE" "1")
-  #t)
+  (setenv "PYTHONDONTWRITEBYTECODE" "1"))
 
 (define* (ensure-no-cythonized-files #:rest _)
   "Check the source code for @code{.c} files which may have been pre-generated
@@ -296,8 +302,7 @@ by Cython."
               (string-append (string-drop-right file 3) "c")))
         (when (file-exists? generated-file)
           (format #t "Possible Cythonized file found: ~a~%" generated-file))))
-    (find-files "." "\\.pyx$"))
-  #t)
+    (find-files "." "\\.pyx$")))
 
 (define %standard-phases
   ;; The build phase only builds C extensions and copies the Python sources,
@@ -319,6 +324,7 @@ by Cython."
     (add-after 'install 'wrap wrap)
     (add-before 'check 'add-install-to-pythonpath add-install-to-pythonpath)
     (add-before 'check 'add-install-to-path add-install-to-path)
+    (add-after 'check 'sanity-check sanity-check)
     (add-before 'strip 'rename-pth-file rename-pth-file)))
 
 (define* (python-build #:key inputs (phases %standard-phases)
diff --git a/tests/builders.scm b/tests/builders.scm
index fdcf38ded3..c5528b2593 100644
--- a/tests/builders.scm
+++ b/tests/builders.scm
@@ -1,5 +1,6 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2012, 2013, 2014, 2015, 2019 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2021 Lars-Dominik Braun <lars@6xq.net>
 ;;;
 ;;; This file is part of GNU Guix.
 ;;;
@@ -17,19 +18,19 @@
 ;;; along with GNU Guix.  If not, see <http://www.gnu.org/licenses/>.
 
 
-(define-module (test-builders)
+(define-module (tests builders)
   #:use-module (guix download)
   #:use-module (guix build-system)
   #:use-module (guix build-system gnu)
+  #:use-module (guix build-system python)
   #:use-module (guix store)
+  #:use-module (guix monads)
   #:use-module (guix utils)
   #:use-module (guix base32)
   #:use-module (guix derivations)
   #:use-module (gcrypt hash)
   #:use-module (guix tests)
-  #:use-module ((guix packages)
-                #:select (package?
-                          package-derivation package-native-search-paths))
+  #:use-module (guix packages)
   #:use-module (gnu packages bootstrap)
   #:use-module (ice-9 match)
   #:use-module (srfi srfi-1)
@@ -78,4 +79,101 @@
 (test-assert "gnu-build-system"
   (build-system? gnu-build-system))
 
+\f
+;;;
+;;; Test the sanity-check phase of the Python build system.
+;;;
+
+(define-syntax-rule (test-build-package name expect-failure? package)
+  "Return a test named NAME, building PACKAGE in the external store."
+  (with-external-store store
+    (unless store (test-skip 1))
+    (let ((build (lambda (p)
+                   (build-derivations
+                    store (list (package-derivation store p))))))
+      (if expect-failure?
+          (test-error name
+                      (store-protocol-error?)
+                      (build package))
+          (test-assert name (build package))))))
+
+(test-build-package "python-build-system: dummy-ok" #f
+                    (dummy-package "python-dummy-ok"
+                      (build-system python-build-system)
+                      (arguments
+                       `(#:phases
+                         (modify-phases %standard-phases
+                           (replace 'unpack
+                             (lambda _
+                               (mkdir-p "dummy")
+                               (invoke "touch" "dummy/__init__.py")
+                               (with-output-to-file "setup.py"
+                                 (lambda _
+                                   (display "\
+from setuptools import setup
+setup(name='dummy-ok',
+      version='0',
+      packages=['dummy'])"))))))))))
+
+(test-build-package "python-build-system: dummy-fail-requirements" #t
+                    (dummy-package "python-dummy-fail-requirements"
+                      (build-system python-build-system)
+                      (arguments
+                       `(#:tests? #f
+                         #:phases
+                         (modify-phases %standard-phases
+                           (replace 'unpack
+                             (lambda _
+                               (mkdir-p "dummy")
+                               (invoke "touch" "dummy/__init__.py")
+                               (with-output-to-file "setup.py"
+                                 (lambda _
+                                   (display "\
+from setuptools import setup
+setup(name='dummy-fail-requirements',
+      version='0',
+      packages=['dummy'],
+      install_requires=['nonexistent'])"))))))))))
+
+(test-build-package "python-build-system: dummy-fail-import" #t
+                    (dummy-package "python-dummy-fail-import"
+                      (build-system python-build-system)
+                      (arguments
+                       `(#:tests? #f
+                         #:phases
+                         (modify-phases %standard-phases
+                           (replace 'unpack
+                             (lambda _
+                               (mkdir-p "dummy")
+                               (with-output-to-file "dummy/__init__.py"
+                                 (lambda _
+                                   (display "import nonexistent")))
+                               (with-output-to-file "setup.py"
+                                 (lambda _
+                                   (display "\
+from setuptools import setup
+setup(name='dummy-fail-import',
+      version='0',
+      packages=['dummy'])"))))))))))
+
+(test-build-package "python-build-system: dummy-fail-console-script" #f
+                    (dummy-package "python-dummy-fail-console-script"
+                      (build-system python-build-system)
+                      (arguments
+                       `(#:tests? #f
+                         #:phases
+                         (modify-phases %standard-phases
+                           (replace 'unpack
+                             (lambda _
+                               (mkdir-p "dummy")
+                               (invoke "touch" "dummy/__init__.py")
+                               (with-output-to-file "setup.py"
+                                 (lambda _
+                                   (display "\
+from setuptools import setup
+setup(name='dummy-fail-console-script',
+      version='0',
+      packages=['dummy'],
+      entry_points={'console_scripts': ['broken = dummy:nonexistent']})"))))))))))
+
 (test-end "builders")
-- 
2.30.0