[bug#65010] [PATCH 0/8] Misc Python build system improvements

[Marius Bakke <marius@gnu.org>]

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

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

> Hi,
>
> the attached patches contain various Python build system improvements
> including an original TOML parser (which may be used by other build
> systems and importers in the future too) and fixes for #62781 and #63044.
> I’m also pushing the patches to the pyproject-toml branch. As always,
> a world rebuild is required.

Hi Lars!

> Lars-Dominik Braun (8):
>   build-system/pyproject: Use python-sans-pip-wrapper as default-python.
>   build-system/python: Ignore symlinks when changing mtime.
>   gnu: criu: Change file mtimes to fix build.
>   gnu: sssd: Change file mtimes to fix build.
>   guix: toml: Add TOML parser.
>   build-system/pyproject: Use TOML parser.
>   gnu: python-pytest-xdist: Disable failing test.
>   guix: pyproject-build-system: Default configure-flags to empty
>     dictionary.

I read through these patches and they LGTM overall.  Some individual
comments follow.

>>From 70baccd8068f35ff8dce0955eaea60f7ea9c240e Mon Sep 17 00:00:00 2001
> Message-ID: <70baccd8068f35ff8dce0955eaea60f7ea9c240e.1690972374.git.lars@6xq.net>
> In-Reply-To: <cover.1690972374.git.lars@6xq.net>
> References: <cover.1690972374.git.lars@6xq.net>
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Thu, 11 May 2023 08:33:02 +0200
> Subject: [PATCH 1/8] build-system/pyproject: Use python-sans-pip-wrapper as
>  default-python.
>
> Also adds python-setuptools and python-wheel to relevant packages,
> either to native-inputs or to propagated inputs if the pkg_resources
> Python module is loaded at runtime.
>
> * guix/build-system/pyproject.scm (default-python): Default to
> python-sans-pip-wrapper.

This change makes sense.  Using a 'python-toolchain' by default was a
mistake, a quite expensive one by the diff.  Sorry about that, and
thanks for fixing it.

As you noted elsewhere (and for those following along at home): this is
necessary to solve a cycle at the root of the Python package graph.  It
also is more "correct" in that there are no surprising or hidden inputs.

[...thinking while typing...]

About 'wheel': that package *is* actually required by the build system
(as opposed to pypa-build!): I think we should make it available by
default (e.g. with a #:wheel argument), or propagate it from the build
systems.  It makes no sense to add it to _all_ pyproject-build-system
consumers.

WDYT?

[...]

>>From f4697d0da0018e66ae759a9495a82f364cad5ccd Mon Sep 17 00:00:00 2001
> Message-ID: <f4697d0da0018e66ae759a9495a82f364cad5ccd.1690972374.git.lars@6xq.net>
> In-Reply-To: <cover.1690972374.git.lars@6xq.net>
> References: <cover.1690972374.git.lars@6xq.net>
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Sat, 13 May 2023 15:31:06 +0200
> Subject: [PATCH 2/8] build-system/python: Ignore symlinks when changing mtime.
>
> * guix/build/python-build-system.scm (ensure-no-mtimes-pre-1980): Ignore
> 'symlink.

LGTM.

>>From a0d4c891e6cf3522c6769e82d888d906445363f5 Mon Sep 17 00:00:00 2001
> Message-ID: <a0d4c891e6cf3522c6769e82d888d906445363f5.1690972374.git.lars@6xq.net>
> In-Reply-To: <cover.1690972374.git.lars@6xq.net>
> References: <cover.1690972374.git.lars@6xq.net>
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Sat, 13 May 2023 15:31:22 +0200
> Subject: [PATCH 3/8] gnu: criu: Change file mtimes to fix build.

This commit title is weird.  The build is already working, but using a
workaround that is no longer needed.

> * gnu/packages/virtualization.scm (criu)[arguments]: Add
> python-build-system to #:modules and #:imported modules, add phase
> 'ensure-no-mtimes-pre-1980.

LGTM.

>>From 824c6ea30573c3a02f33058bd1739be4cd980da2 Mon Sep 17 00:00:00 2001
> Message-ID: <824c6ea30573c3a02f33058bd1739be4cd980da2.1690972374.git.lars@6xq.net>
> In-Reply-To: <cover.1690972374.git.lars@6xq.net>
> References: <cover.1690972374.git.lars@6xq.net>
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Sat, 13 May 2023 16:20:23 +0200
> Subject: [PATCH 4/8] gnu: sssd: Change file mtimes to fix build.
>
> * gnu/packages/sssd.scm (sssd)[arguments]: Add
> python-build-system to #:modules and #:imported modules, add phase
> 'ensure-no-mtimes-pre-1980.

Ditto.

>>From 72d66e838ce9d3d2182c570ee3088063e372fcdd Mon Sep 17 00:00:00 2001
> Message-ID: <72d66e838ce9d3d2182c570ee3088063e372fcdd.1690972374.git.lars@6xq.net>
> In-Reply-To: <cover.1690972374.git.lars@6xq.net>
> References: <cover.1690972374.git.lars@6xq.net>
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Sun, 23 Jul 2023 11:20:03 +0200
> Subject: [PATCH 5/8] guix: toml: Add TOML parser.
>
> * guix/build/toml.scm: New file.
> * tests/toml.scm: New file.
> * Makefile.am: Register new files.
> ---
>  Makefile.am         |   2 +
>  guix/build/toml.scm | 478 ++++++++++++++++++++++++++++++++++++++++++++
>  tests/toml.scm      | 442 ++++++++++++++++++++++++++++++++++++++++

Woow, amazing work, and I'm surprised it's less than 500 lines!

I haven't studied it in detail, but on the surface LGTM.

>>From 8e079b48b7c07c07360db8eb6305e8044d86dd87 Mon Sep 17 00:00:00 2001
> Message-ID: <8e079b48b7c07c07360db8eb6305e8044d86dd87.1690972374.git.lars@6xq.net>
> In-Reply-To: <cover.1690972374.git.lars@6xq.net>
> References: <cover.1690972374.git.lars@6xq.net>
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Sun, 23 Jul 2023 11:22:03 +0200
> Subject: [PATCH 6/8] build-system/pyproject: Use TOML parser.
>
> More reliable than regular expressions.
>
> * guix/build-system/pyproject.scm (%pyproject-build-system-modules): Add (guix build toml).
> (pyproject-build): Add argument #:backend-path.
> * guix/build/pyproject-build-system.scm (build): Add support for
> auto-detected and override backend-path.
> * gnu/packages/python-build.scm (python-tomli)[arguments]: Remove
> 'add-self-to-path, because it is not necessary any more.
> (python-poetry-core): Same.
> (python-hatchling): Same.
> (python-pdm-backend): Same.
> ---
>  gnu/packages/python-build.scm         | 30 ++------------
>  guix/build-system/pyproject.scm       |  3 ++
>  guix/build/pyproject-build-system.scm | 56 +++++++++++++++------------
>  3 files changed, 39 insertions(+), 50 deletions(-)

LGTM...

[...]

> diff --git a/guix/build-system/pyproject.scm b/guix/build-system/pyproject.scm
> index 94b9d79692..585117cbf0 100644
> --- a/guix/build-system/pyproject.scm
> +++ b/guix/build-system/pyproject.scm
> @@ -46,6 +46,7 @@ (define %pyproject-build-system-modules
>    ;; Build-side modules imported by default.
>    `((guix build pyproject-build-system)
>      (guix build json)
> +    (guix build toml)
>      ,@%python-build-system-modules))
>  
>  (define (default-python)
> @@ -93,6 +94,7 @@ (define* (pyproject-build name inputs
>                            #:key source
>                            (tests? #t)
>                            (configure-flags ''())
> +                          (backend-path #f)
>                            (build-backend #f)
>                            (test-backend #f)
>                            (test-flags ''())
> @@ -116,6 +118,7 @@ (define* (pyproject-build name inputs
>                   #:source #+source
>                   #:configure-flags #$configure-flags
>                   #:system #$system
> +                 #:backend-path #$backend-path
>                   #:build-backend #$build-backend
>                   #:test-backend #$test-backend
>                   #:test-flags #$test-flags
> diff --git a/guix/build/pyproject-build-system.scm b/guix/build/pyproject-build-system.scm
> index c69ccc9d64..a1919eacf6 100644
> --- a/guix/build/pyproject-build-system.scm
> +++ b/guix/build/pyproject-build-system.scm
> @@ -21,11 +21,13 @@ (define-module (guix build pyproject-build-system)
>    #:use-module ((guix build python-build-system) #:prefix python:)
>    #:use-module (guix build utils)
>    #:use-module (guix build json)
> +  #:use-module (guix build toml)
>    #:use-module (ice-9 match)
>    #:use-module (ice-9 ftw)
>    #:use-module (ice-9 format)
>    #:use-module (ice-9 rdelim)
>    #:use-module (ice-9 regex)
> +  #:use-module (ice-9 textual-ports)

This import seems unused?

>    #:use-module (srfi srfi-1)
>    #:use-module (srfi srfi-26)
>    #:use-module (srfi srfi-34)
> @@ -60,8 +62,8 @@ (define-module (guix build pyproject-build-system)
>  ;;; wheel and expected to be created by the installing utility.
>  ;;; TODO: Add support for PEP-621 entry points.
>  ;;;
> -;;; Caveats:
> -;;; - There is no support for in-tree build backends.
> +;;; This module also supports in-tree build backends, which can be
> +;;; overridden by #:backend-path.

Perhaps this should also be documented in the manual.

>>From e987a9e28ec0d8d1b8ecdb2486eb12eae270a49d Mon Sep 17 00:00:00 2001
> Message-ID: <e987a9e28ec0d8d1b8ecdb2486eb12eae270a49d.1690972374.git.lars@6xq.net>
> In-Reply-To: <cover.1690972374.git.lars@6xq.net>
> References: <cover.1690972374.git.lars@6xq.net>
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Tue, 25 Jul 2023 18:26:58 +0200
> Subject: [PATCH 7/8] gnu: python-pytest-xdist: Disable failing test.
>
> * gnu/packages/check.scm (python-pytest-xdist)[arguments]: Skip failing
> test.

OK.

>>From b3726639df72aa3943d8e403e3c2b9a6cde05421 Mon Sep 17 00:00:00 2001
> Message-ID: <b3726639df72aa3943d8e403e3c2b9a6cde05421.1690972374.git.lars@6xq.net>
> In-Reply-To: <cover.1690972374.git.lars@6xq.net>
> References: <cover.1690972374.git.lars@6xq.net>
> From: Lars-Dominik Braun <lars@6xq.net>
> Date: Sun, 30 Jul 2023 13:36:37 +0200
> Subject: [PATCH 8/8] guix: pyproject-build-system: Default configure-flags to
>  empty dictionary.
>
> PEP 517 specifies it should be a dictionary and thus meson-python cannot
> handle an empty list.
>
> Fixes: <https://issues.guix.gnu.org/62781>
> ---
>  gnu/packages/build-tools.scm    | 10 ++--------
>  gnu/packages/python-science.scm |  5 -----
>  guix/build-system/pyproject.scm |  2 +-
>  3 files changed, 3 insertions(+), 14 deletions(-)

The commit message lacks a mention of the changed variables.

[...]

> diff --git a/guix/build-system/pyproject.scm b/guix/build-system/pyproject.scm
> index 585117cbf0..c0e089eac7 100644
> --- a/guix/build-system/pyproject.scm
> +++ b/guix/build-system/pyproject.scm
> @@ -93,7 +93,7 @@ (define* (lower name
>  (define* (pyproject-build name inputs
>                            #:key source
>                            (tests? #t)
> -                          (configure-flags ''())
> +                          (configure-flags ''(@))

I don't understand how the @ makes it a dictionary.  Can you enlighten
me?  Either here, or in a comment?  :-)

LGTM anyway!

Let's get these merged in the 'python-team' branch.  WDYT jgart?

[-- Attachment #2: signature.asc --]
[-- Type: application/pgp-signature, Size: 247 bytes --]