GNU bug report logs - #74582
[PATCH python-team 0/4] Fix edge case in pyproject-build-system

Previous Next

To reply to this bug, email your comments to 74582 AT debbugs.gnu.org.

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

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: guix-patches <at> gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH python-team 0/4] Fix edge case in pyproject-build-system
Date: Thu, 28 Nov 2024 17:05:19 +0900

Maxim Cournoyer (4):
  build/pyproject: Really merge directories in install phase.
  build/pyproject: Fix indentation.
  build/pyproject: Update PEP 427 reference URL in comment.
  build/pyproject: Resolve import warning.

 guix/build/pyproject-build-system.scm | 52 +++++++++++++++------------
 1 file changed, 29 insertions(+), 23 deletions(-)


base-commit: dd4b96e72c8fda4b025a75b47212e06e381e9ea1
-- 
2.46.0

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74582 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH python-team 1/4] build/pyproject: Really merge directories in
 install phase.
Date: Thu, 28 Nov 2024 21:16:39 +0900

Using rename-file, the destination had to be empty otherwise it would error
out.  By using copy-recursively, a directory can be copied onto a pre-existing
directory, really merging them.  This problem manifested itself attempting to
build the python-pyre package.

* guix/build/pyproject-build-system.scm (install)
<merge-directories>: Use copy-recursively instead of rename-file.

Change-Id: Iceb8609a86f29b17e5fbe6a9629339d0bc26e11f
---
 guix/build/pyproject-build-system.scm | 9 +++++++--
 1 file changed, 7 insertions(+), 2 deletions(-)

diff --git a/guix/build/pyproject-build-system.scm b/guix/build/pyproject-build-system.scm
index c69ccc9d64..03992d915f 100644
--- a/guix/build/pyproject-build-system.scm
+++ b/guix/build/pyproject-build-system.scm
@@ -194,8 +194,13 @@ (define* (install #:key inputs outputs #:allow-other-keys)
                   (format #t "~a/~a -> ~a/~a~%"
                           source file destination file)
                   (mkdir-p destination)
-                  (rename-file (string-append source "/" file)
-                               (string-append destination "/" file))
+                  ;; Use 'copy-recursively' rather than 'rename-file' to guard
+                  ;; against the odd case where DESTINATION is a non-empty
+                  ;; directory, which may happen when using hybrid Python
+                  ;; build systems.
+                  (copy-recursively (string-append source "/" file)
+                                    (string-append destination "/" file))
+                  (delete-file-recursively (string-append source "/" file))
                   (when post-move
                     (post-move file)))
                 (scandir source
-- 
2.46.0

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74582 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH python-team 3/4] build/pyproject: Update PEP 427 reference URL
 in comment.
Date: Thu, 28 Nov 2024 21:16:41 +0900

* guix/build/pyproject-build-system.scm (install): Update reference URL.

Change-Id: Icf5dcc7254c33e8e466773ee66a2fd5648d583da
---
 guix/build/pyproject-build-system.scm | 5 +++--
 1 file changed, 3 insertions(+), 2 deletions(-)

diff --git a/guix/build/pyproject-build-system.scm b/guix/build/pyproject-build-system.scm
index d42577b259..314839c30f 100644
--- a/guix/build/pyproject-build-system.scm
+++ b/guix/build/pyproject-build-system.scm
@@ -172,8 +172,9 @@ (define* (check #:key tests? test-backend test-flags #:allow-other-keys)
       (format #t "test suite not run~%")))
 
 (define* (install #:key inputs outputs #:allow-other-keys)
-  "Install a wheel file according to PEP 427"
-  ;; See https://www.python.org/dev/peps/pep-0427/#installing-a-wheel-distribution-1-0-py32-none-any-whl
+  "Install a wheel file according to PEP 427."
+  ;; See <https://packaging.python.org/en/latest/specifications/\
+  ;; binary-distribution-format/#binary-distribution-format>.
   (let ((site-dir (site-packages inputs outputs))
         (python (assoc-ref inputs "python"))
         (out (assoc-ref outputs "out")))
-- 
2.46.0

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74582 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH python-team 2/4] build/pyproject: Fix indentation.
Date: Thu, 28 Nov 2024 21:16:40 +0900

* guix/build/pyproject-build-system.scm: Re-indent file with Emacs.

Change-Id: I15c89628190b81a71f799e4812c3b6a360f93bcb
---
 guix/build/pyproject-build-system.scm | 36 +++++++++++++--------------
 1 file changed, 18 insertions(+), 18 deletions(-)

diff --git a/guix/build/pyproject-build-system.scm b/guix/build/pyproject-build-system.scm
index 03992d915f..d42577b259 100644
--- a/guix/build/pyproject-build-system.scm
+++ b/guix/build/pyproject-build-system.scm
@@ -77,11 +77,11 @@ (define-condition-type &python-build-error &error python-build-error?)
 
 ;; Raised when 'check cannot find a valid test system in the inputs.
 (define-condition-type &test-system-not-found &python-build-error
-                       test-system-not-found?)
+  test-system-not-found?)
 
 ;; Raised when multiple wheels are created by 'build.
 (define-condition-type &cannot-extract-multiple-wheels &python-build-error
-                       cannot-extract-multiple-wheels?)
+  cannot-extract-multiple-wheels?)
 
 ;; Raised, when no wheel has been built by the build system.
 (define-condition-type &no-wheels-built &python-build-error no-wheels-built?)
@@ -93,8 +93,7 @@ (define* (build #:key outputs build-backend configure-flags #:allow-other-keys)
     "Look up the build backend in a pyproject.toml file."
     (call-with-input-file file
       (lambda (in)
-        (let loop
-          ((line (read-line in 'concat)))
+        (let loop ((line (read-line in 'concat)))
           (if (eof-object? line) #f
               (let ((m (string-match "build-backend = [\"'](.+)[\"']" line)))
                 (if m
@@ -122,18 +121,18 @@ (define* (build #:key outputs build-backend configure-flags #:allow-other-keys)
                                 auto-build-backend
                                 "setuptools.build_meta")))
     (format #t
-     "Using '~a' to build wheels, auto-detected '~a', override '~a'.~%"
-     use-build-backend auto-build-backend build-backend)
+            "Using '~a' to build wheels, auto-detected '~a', override '~a'.~%"
+            use-build-backend auto-build-backend build-backend)
     (mkdir-p wheel-dir)
     ;; Call the PEP 517 build function, which drops a .whl into wheel-dir.
     (invoke "python" "-c"
-     "import sys, importlib, json
+            "import sys, importlib, json
 config_settings = json.loads (sys.argv[3])
 builder = importlib.import_module(sys.argv[1])
 builder.build_wheel(sys.argv[2], config_settings=config_settings)"
-     use-build-backend
-     wheel-dir
-     config-settings)))
+            use-build-backend
+            wheel-dir
+            config-settings)))
 
 (define* (check #:key tests? test-backend test-flags #:allow-other-keys)
   "Run the test suite of a given Python package."
@@ -253,19 +252,20 @@ (define* (install #:key inputs outputs #:allow-other-keys)
                         (scandir wheel-dir
                                  (cut string-suffix? ".whl" <>)))))
       (cond
-        ((> (length wheels) 1)
-         ;; This code does not support multiple wheels yet, because their
-         ;; outputs would have to be merged properly.
-         (raise (condition (&cannot-extract-multiple-wheels))))
-        ((= (length wheels) 0)
-         (raise (condition (&no-wheels-built)))))
+       ((> (length wheels) 1)
+        ;; This code does not support multiple wheels yet, because their
+        ;; outputs would have to be merged properly.
+        (raise (condition (&cannot-extract-multiple-wheels))))
+       ((= (length wheels) 0)
+        (raise (condition (&no-wheels-built)))))
       (for-each extract wheels))
     (let ((datadirs (map (cut string-append site-dir "/" <>)
                          (list-directories site-dir
                                            (file-name-predicate "\\.data$")))))
       (for-each (lambda (directory)
                   (expand-data-directory directory)
-                  (rmdir directory)) datadirs))))
+                  (rmdir directory))
+                datadirs))))
 
 (define* (compile-bytecode #:key inputs outputs #:allow-other-keys)
   "Compile installed byte-code in site-packages."
@@ -341,7 +341,7 @@ (define* (create-entrypoints #:key inputs outputs #:allow-other-keys)
 import sys
 import ~a as mod
 sys.exit (mod.~a ())~%" interpreter module function)))
-        (chmod file-path #o755)))
+      (chmod file-path #o755)))
 
   (let* ((site-dir (site-packages inputs outputs))
          (out (assoc-ref outputs "out"))
-- 
2.46.0

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: 74582 <at> debbugs.gnu.org
Cc: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH python-team 4/4] build/pyproject: Resolve import warning.
Date: Thu, 28 Nov 2024 21:16:42 +0900

* guix/build/pyproject-build-system.scm: Hide the 'delete' symbol from
the imported (guix build utils) module to avoid a naming clash warning.

Change-Id: I7db9500b20c71c89e740c18c089f33c8569c4ffd
---
 guix/build/pyproject-build-system.scm | 2 +-
 1 file changed, 1 insertion(+), 1 deletion(-)

diff --git a/guix/build/pyproject-build-system.scm b/guix/build/pyproject-build-system.scm
index 314839c30f..beca4dc8ca 100644
--- a/guix/build/pyproject-build-system.scm
+++ b/guix/build/pyproject-build-system.scm
@@ -19,7 +19,7 @@
 
 (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 utils) #:hide (delete))
   #:use-module (guix build json)
   #:use-module (ice-9 match)
   #:use-module (ice-9 ftw)
-- 
2.46.0

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

From: "jgart" <jgart <at> dismail.de>
To: "Maxim Cournoyer" <maxim.cournoyer <at> gmail.com>, 74582 <at> debbugs.gnu.org
Cc: Sharlatan Hellseher <sharlatanus <at> gmail.com>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>,
 Munyoki Kilyungi <me <at> bonfacemunyoki.com>, Lars-Dominik Braun <lars <at> 6xq.net>,
 Marius Bakke <marius <at> gnu.org>, Tanguy Le Carrour <tanguy <at> bioneland.org>
Subject: Re: [bug#74582] [PATCH python-team 4/4] build/pyproject: Resolve
 import warning.
Date: Thu, 28 Nov 2024 18:44:54 +0000

[Message part 1 (text/plain, inline)]

Thanks!

Would anyone else like to review this? I won't be able to test and review it for up to a week.

LGTM, otherwise.

all best,

jgart

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

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Tanguy Le Carrour <tanguy <at> bioneland.org>,
 Munyoki Kilyungi <me <at> bonfacemunyoki.com>, 74582 <at> debbugs.gnu.org,
 jgart <jgart <at> dismail.de>, Marius Bakke <marius <at> gnu.org>,
 Sharlatan Hellseher <sharlatanus <at> gmail.com>
Subject: Re: [bug#74582] [PATCH python-team 1/4] build/pyproject: Really
 merge directories in install phase.
Date: Fri, 29 Nov 2024 08:23:59 +0100

Hi,

> +                  ;; Use 'copy-recursively' rather than 'rename-file' to guard
> +                  ;; against the odd case where DESTINATION is a non-empty
> +                  ;; directory, which may happen when using hybrid Python
> +                  ;; build systems.
> +                  (copy-recursively (string-append source "/" file)
> +                                    (string-append destination "/" file))
> +                  (delete-file-recursively (string-append source "/" file))

wouldn’t it be easier to remove this function entirely and move the
shebang-replacement via POST-MOVE into a separate function (perhaps
powered by FIND-FILES instead of SCANDIR)?

I believe with this patch we can also remove &cannot-extract-multiple-wheels
further down, since directories should be merged now, right?

Cheers,
Lars

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: Tanguy Le Carrour <tanguy <at> bioneland.org>,
 Munyoki Kilyungi <me <at> bonfacemunyoki.com>, 74582 <at> debbugs.gnu.org,
 jgart <jgart <at> dismail.de>, Marius Bakke <marius <at> gnu.org>,
 Sharlatan Hellseher <sharlatanus <at> gmail.com>
Subject: Re: [bug#74582] [PATCH python-team 1/4] build/pyproject: Really
 merge directories in install phase.
Date: Sun, 15 Dec 2024 00:12:58 +0900

Hi,

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

> Hi,
>
>> +                  ;; Use 'copy-recursively' rather than 'rename-file' to guard
>> +                  ;; against the odd case where DESTINATION is a non-empty
>> +                  ;; directory, which may happen when using hybrid Python
>> +                  ;; build systems.
>> +                  (copy-recursively (string-append source "/" file)
>> +                                    (string-append destination "/" file))
>> +                  (delete-file-recursively (string-append source "/" file))
>
> wouldn’t it be easier to remove this function entirely and move the
> shebang-replacement via POST-MOVE into a separate function (perhaps
> powered by FIND-FILES instead of SCANDIR)?

Yes, that could be nicer.  I'd like to keep it for a distinct commti
though, to keep this small and focus.

> I believe with this patch we can also remove &cannot-extract-multiple-wheels
> further down, since directories should be merged now, right?

Perhaps, though we'd want to verify that it indeed now works, and not
having seen that error once, I'm not too sure how to test it.  Do you
know of a package that could make use of this?

-- 
Thanks,
Maxim

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

From: Lars-Dominik Braun <lars <at> 6xq.net>
To: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Cc: Tanguy Le Carrour <tanguy <at> bioneland.org>,
 Munyoki Kilyungi <me <at> bonfacemunyoki.com>, 74582 <at> debbugs.gnu.org,
 jgart <jgart <at> dismail.de>, Marius Bakke <marius <at> gnu.org>,
 Sharlatan Hellseher <sharlatanus <at> gmail.com>
Subject: Re: [bug#74582] [PATCH python-team 1/4] build/pyproject: Really
 merge directories in install phase.
Date: Sun, 15 Dec 2024 17:25:29 +0100

Hi,

> Yes, that could be nicer.  I'd like to keep it for a distinct commti
> though, to keep this small and focus.

sure, fine.

> Perhaps, though we'd want to verify that it indeed now works, and not
> having seen that error once, I'm not too sure how to test it.  Do you
> know of a package that could make use of this?

Hm, I checked and it does not seem to be possible to build more than
one wheel with a single pyproject.toml[1]. Let’s keep the code as is then.

Lars

[1] https://peps.python.org/pep-0517/#build-wheel states that
    build_wheel() should return the basename of the .whl file it
    creates. That implies it can only be one.

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: Tanguy Le Carrour <tanguy <at> bioneland.org>,
 Munyoki Kilyungi <me <at> bonfacemunyoki.com>, 74582 <at> debbugs.gnu.org,
 jgart <jgart <at> dismail.de>, Marius Bakke <marius <at> gnu.org>,
 Sharlatan Hellseher <sharlatanus <at> gmail.com>
Subject: Re: [bug#74582] [PATCH python-team 1/4] build/pyproject: Really
 merge directories in install phase.
Date: Mon, 16 Dec 2024 16:09:12 +0900

Hi,

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

> Hi,
>
>> Yes, that could be nicer.  I'd like to keep it for a distinct commti
>> though, to keep this small and focus.
>
> sure, fine.
>
>> Perhaps, though we'd want to verify that it indeed now works, and not
>> having seen that error once, I'm not too sure how to test it.  Do you
>> know of a package that could make use of this?
>
> Hm, I checked and it does not seem to be possible to build more than
> one wheel with a single pyproject.toml[1]. Let’s keep the code as is then.

Yes.  I'd expect perhaps some kind of mono-repo holding multiple Python
packages could perhaps end up in such a situation where it'd have
mulitple .whl installed to the same prefix.

The odd case I had encountered where merging directories was failing was
an attempt to build a scikit package (I think it was python-libcst) that
used CMake for the extensions, installed that already, then did a
regular PEP 517 build and attempted to install more things to its
prefix, which already had things placed there by scikit/cmake, IIUC.

I ended up packaging libcst via cargo instead of scikit, since it's
authored in Rust.

-- 
Thanks,
Maxim

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Lars-Dominik Braun <lars <at> 6xq.net>
Cc: Tanguy Le Carrour <tanguy <at> bioneland.org>,
 Munyoki Kilyungi <me <at> bonfacemunyoki.com>, 74582 <at> debbugs.gnu.org,
 jgart <jgart <at> dismail.de>, Marius Bakke <marius <at> gnu.org>,
 Sharlatan Hellseher <sharlatanus <at> gmail.com>
Subject: Re: [bug#74582] [PATCH python-team 1/4] build/pyproject: Really
 merge directories in install phase.
Date: Wed, 18 Dec 2024 20:48:18 +0900

Hello,

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

[...]

> The odd case I had encountered where merging directories was failing was
> an attempt to build a scikit package (I think it was python-libcst)

Correction: it was python-pyre, see: bug#74581 for a package definition
reproducing it.

-- 
Thanks,
Maxim

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

From: Sharlatan Hellseher <sharlatanus <at> gmail.com>
To: 74582 <at> debbugs.gnu.org
Cc: Lars-Dominik Braun <lars <at> 6xq.net>,
 Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
Subject: [PATCH python-team 0/4] Fix edge case in pyproject-build-system
Date: Sun, 4 May 2025 20:12:23 +0100

[Message part 1 (text/plain, inline)]

Hi,

Which is consensus here?
 If it's stale feel free to close.

VCS: https://github.incerto.xyz/; https://git.sr.ht/~hellseher/
GPG: 9847 81DE 689C 21C2 6418 0867 76D7 27BF F62C D2B5

… наш разум - превосходная объяснительная машина которая способна найти
смысл почти в чем угодно, истолковать любой феномен, но совершенно не в
состоянии принять мысль о непредсказуемости.

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

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

From: Maxim Cournoyer <maxim.cournoyer <at> gmail.com>
To: Sharlatan Hellseher <sharlatanus <at> gmail.com>
Cc: 74582 <at> debbugs.gnu.org, Lars-Dominik Braun <lars <at> 6xq.net>
Subject: Re: [bug#74582] [PATCH python-team 0/4] Fix edge case in
 pyproject-build-system
Date: Wed, 07 May 2025 10:59:30 +0900

Hi Sharlatan,

Sharlatan Hellseher <sharlatanus <at> gmail.com> writes:

> Hi,
>
> Which is consensus here?
>  If it's stale feel free to close.

Where the value is limited since it's not a common case, the couple
things this series fixes are still valid.  If it still applies cleanly,
please merge it to the python-team branch.

-- 
Thanks,
Maxim

Previous Next

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