Subject: Re: Should ‘guix import pypi’get dependencies from pyproject files?

From: Lars-Dominik Braun <[email-masked]>
Date: Sun, 15 Dec 2024 17:12:12 +0100

Hi,

> Should ‘guix import pypi’ attempt to get dependency information from
> ‘pyproject.toml’, in addition to ‘requirements.txt’ and wheel ‘METADATA’
> as it already does?

attached patches allow parsing the standardized pyproject.toml fields
for dependencies. This won’t work for poetry (we need a different
version parser for that), but it’s a start.

Lars

>From c2e7e07ad407613233edbb7ebfcc6f0c7c0bcc25 Mon Sep 17 00:00:00 2001
Message-ID: <[email-masked]>
From: Lars-Dominik Braun <[email-masked]>
Date: Sun, 15 Dec 2024 13:22:00 +0100
Subject: [PATCH 1/4] import: pypi: Support extracting dependencies from
 pyproject.toml.

* guix/import/pypi.scm (guess-requirements): Support extracting dependencies from pyproject.toml.
* tests/pypi.scm: ("pypi->guix-package, no requires.txt, but wheel."):
Renamed from "pypi->guix-package, wheels", remove requires.txt file,
because the current implementation cannot detect invalid files.
("pypi->guix-package, no usable requirement file, no wheel."): Renamed
from "pypi->guix-package, no usable requirement file.".
(test-pyproject.toml): New variable.
("pypi->guix-package, no wheel, no requires.txt, but pyproject.toml"): New test.
("pypi->guix-package, no wheel, but requires.txt and pyproject.toml"): Ditto.

Change-Id: Ib525750eb6ff4139a8209420042b28ae3c850764
---
 guix/import/pypi.scm |  74 +++++++++++++++++++++++--------
 tests/pypi.scm       | 101 ++++++++++++++++++++++++++++++++++++++++---
 2 files changed, 152 insertions(+), 23 deletions(-)

diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
index 7b9f54a200..7915d65d23 100644
--- a/guix/import/pypi.scm
+++ b/guix/import/pypi.scm
@@ -57,6 +57,7 @@ (define-module (guix import pypi)
   #:use-module (guix import utils)
   #:use-module (guix import json)
   #:use-module (json)
+  #:use-module (guix build toml)
   #:use-module (guix packages)
   #:use-module (guix upstream)
   #:use-module ((guix licenses) #:prefix license:)
@@ -386,7 +387,42 @@ (define (guess-requirements source-url wheel-url archive)
        (if wheel-url
            (and (url-fetch wheel-url temp)
                 (read-wheel-metadata temp))
-           #f))))
+           (list '() '())))))
+
+  (define (guess-requirements-from-pyproject.toml dir)
+    (let* ((pyproject.toml-files (find-files dir (lambda (abs-file-name _)
+                                          (string-match "/pyproject.toml$"
+                                          abs-file-name))))
+          (pyproject.toml (match pyproject.toml-files
+                            (()
+                              (warning (G_ "Cannot guess requirements from \
+pyproject.toml file, because it does not exist.~%"))
+                              '())
+                            (else (parse-toml-file (first pyproject.toml-files)))))
+          (pyproject-build-requirements
+           (or (recursive-assoc-ref pyproject.toml '("build-system" "requires")) '()))
+          (pyproject-dependencies
+           (or (recursive-assoc-ref pyproject.toml '("project" "dependencies")) '()))
+          ;; This is more of a convention, since optional-dependencies is a table of arbitrary values.
+          (pyproject-test-dependencies
+           (or (recursive-assoc-ref pyproject.toml '("project" "optional-dependencies" "test")) '())))
+      (if (null? pyproject.toml)
+        #f
+        (list (map specification->requirement-name pyproject-dependencies)
+              (map specification->requirement-name
+                   (append pyproject-build-requirements
+                           pyproject-test-dependencies))))))
+
+  (define (guess-requirements-from-requires.txt dir)
+    (let ((requires.txt-files (find-files dir (lambda (abs-file-name _)
+		                                          (string-match "\\.egg-info/requires.txt$"
+                                                  abs-file-name)))))
+     (match requires.txt-files
+       (()
+        (warning (G_ "Cannot guess requirements from source archive: \
+no requires.txt file found.~%"))
+        #f)
+       (else (parse-requires.txt (first requires.txt-files))))))
 
   (define (guess-requirements-from-source)
     ;; Return the package's requirements by guessing them from the source.
@@ -398,27 +434,29 @@ (define (guess-requirements source-url wheel-url archive)
              (if (string=? "zip" (file-extension source-url))
                  (invoke "unzip" archive "-d" dir)
                  (invoke "tar" "xf" archive "-C" dir)))
-           (let ((requires.txt-files
-                  (find-files dir (lambda (abs-file-name _)
-		                    (string-match "\\.egg-info/requires.txt$"
-                                                  abs-file-name)))))
-             (match requires.txt-files
-               (()
-                (warning (G_ "Cannot guess requirements from source archive:\
- no requires.txt file found.~%"))
-                (list '() '()))
-               (else (parse-requires.txt (first requires.txt-files)))))))
+               (list (guess-requirements-from-pyproject.toml dir)
+                     (guess-requirements-from-requires.txt dir))))
         (begin
           (warning (G_ "Unsupported archive format; \
 cannot determine package dependencies from source archive: ~a~%")
                    (basename source-url))
-          (list '() '()))))
-
-  ;; First, try to compute the requirements using the wheel, else, fallback to
-  ;; reading the "requires.txt" from the egg-info directory from the source
-  ;; archive.
-  (or (guess-requirements-from-wheel)
-      (guess-requirements-from-source)))
+          (list #f #f))))
+
+  (define (merge a b)
+    "Given lists A and B with two iteams each, combine A1 and B1, as well as A2 and B2."
+    (match (list a b)
+      (((first-propagated first-native) (second-propagated second-native))
+       (list (append first-propagated second-propagated) (append first-native second-native)))))
+
+  ;; requires.txt and the metadata of a wheel contain redundant information,
+  ;; so fetch only one of them, preferring requires.txt from the source
+  ;; distribution, which we always fetch, since the source tarball also
+  ;; contains pyproject.toml.
+  (match (guess-requirements-from-source)
+    ((from-pyproject.toml #f)
+      (merge (or from-pyproject.toml '(() ())) (or (guess-requirements-from-wheel) '(() ()))))
+    ((from-pyproject.toml from-requires.txt)
+      (merge (or from-pyproject.toml '(() ())) from-requires.txt))))
 
 (define (compute-inputs source-url wheel-url archive)
   "Given the SOURCE-URL and WHEEL-URL of an already downloaded ARCHIVE, return
diff --git a/tests/pypi.scm b/tests/pypi.scm
index c9aee34d8b..fe00e429b7 100644
--- a/tests/pypi.scm
+++ b/tests/pypi.scm
@@ -112,6 +112,20 @@ (define test-requires.txt-beaker "\
 coverage
 ")
 
+(define test-pyproject.toml "\
+[build-system]
+requires = [\"dummy-build-dep-a\", \"dummy-build-dep-b\"]
+
+[project]
+dependencies = [
+  \"dummy-dep-a\",
+  \"dummy-dep-b\",
+]
+
+[project.optional-dependencies]
+test = [\"dummy-test-dep-a\", \"dummy-test-dep-b\"]
+")
+
 (define test-metadata "\
 Classifier: Programming Language :: Python :: 3.7
 Requires-Dist: baz ~= 3
@@ -325,13 +339,90 @@ (define-syntax-rule (with-pypi responses body ...)
         (x
          (pk 'fail x #f))))))
 
+(test-assert "pypi->guix-package, no wheel, no requires.txt, but pyproject.toml"
+  (let ((tarball (pypi-tarball
+                  "foo-1.0.0"
+                  `(("pyproject.toml" ,test-pyproject.toml))))
+        (twice (lambda (lst) (append lst lst))))
+    (with-pypi (twice `(("/foo-1.0.0.tar.gz" 200 ,(file-dump tarball))
+                        ("/foo-1.0.0-py2.py3-none-any.whl" 404 "")
+                        ("/foo/json" 200 ,(lambda (port)
+                                            (display (foo-json) port)))))
+      ;; Not clearing the memoization cache here would mean returning the value
+      ;; computed in the previous test.
+      (invalidate-memoization! pypi->guix-package)
+      (match (pypi->guix-package "foo")
+        (`(package
+            (name "python-foo")
+            (version "1.0.0")
+            (source (origin
+                      (method url-fetch)
+                      (uri (pypi-uri "foo" version))
+                      (sha256
+                       (base32 ,(? string? hash)))))
+            (build-system pyproject-build-system)
+            (propagated-inputs (list python-dummy-dep-a python-dummy-dep-b))
+            (native-inputs (list python-dummy-build-dep-a python-dummy-build-dep-b
+                                 python-dummy-test-dep-a python-dummy-test-dep-b))
+            (home-page "http://example.com")
+            (synopsis "summary")
+            (description "summary.")
+            (license license:lgpl2.0))
+         (and (string=? default-sha256/base32 hash)
+              (equal? (pypi->guix-package "foo" #:version "1.0.0")
+                      (pypi->guix-package "foo"))
+              (guard (c ((error? c) #t))
+                (pypi->guix-package "foo" #:version "42"))))
+        (x
+         (pk 'fail x #f))))))
+
+(test-assert "pypi->guix-package, no wheel, but requires.txt and pyproject.toml"
+  (let ((tarball (pypi-tarball
+                  "foo-1.0.0"
+                  `(("foo-1.0.0/pyproject.toml" ,test-pyproject.toml)
+                    ("foo-1.0.0/bizarre.egg-info/requires.txt"
+                     ,test-requires.txt))))
+        (twice (lambda (lst) (append lst lst))))
+    (with-pypi (twice `(("/foo-1.0.0.tar.gz" 200 ,(file-dump tarball))
+                        ("/foo-1.0.0-py2.py3-none-any.whl" 404 "")
+                        ("/foo/json" 200 ,(lambda (port)
+                                            (display (foo-json) port)))))
+      ;; Not clearing the memoization cache here would mean returning the value
+      ;; computed in the previous test.
+      (invalidate-memoization! pypi->guix-package)
+      (match (pypi->guix-package "foo")
+        (`(package
+            (name "python-foo")
+            (version "1.0.0")
+            (source (origin
+                      (method url-fetch)
+                      (uri (pypi-uri "foo" version))
+                      (sha256
+                       (base32 ,(? string? hash)))))
+            (build-system pyproject-build-system)
+            ;; Information from requires.txt and pyproject.toml is combined.
+            (propagated-inputs (list python-bar python-dummy-dep-a python-dummy-dep-b
+                                     python-foo))
+            (native-inputs (list python-dummy-build-dep-a python-dummy-build-dep-b
+                                 python-dummy-test-dep-a python-dummy-test-dep-b
+                                 python-pytest))
+            (home-page "http://example.com")
+            (synopsis "summary")
+            (description "summary.")
+            (license license:lgpl2.0))
+         (and (string=? default-sha256/base32 hash)
+              (equal? (pypi->guix-package "foo" #:version "1.0.0")
+                      (pypi->guix-package "foo"))
+              (guard (c ((error? c) #t))
+                (pypi->guix-package "foo" #:version "42"))))
+        (x
+         (pk 'fail x #f))))))
+
 (test-skip (if (which "zip") 0 1))
-(test-assert "pypi->guix-package, wheels"
+(test-assert "pypi->guix-package, no requires.txt, but wheel."
   (let ((tarball (pypi-tarball
                   "foo-1.0.0"
-                  '(("foo-1.0.0/foo.egg-info/requires.txt"
-                     "wrong data \
-to make sure we're testing wheels"))))
+                  '(("foo-1.0.0/foo.egg-info/.empty" ""))))
         (wheel (wheel-file "foo-1.0.0"
                            `(("METADATA" ,test-metadata)))))
     (with-pypi `(("/foo-1.0.0.tar.gz" 200 ,(file-dump tarball))
@@ -362,7 +453,7 @@ (define-syntax-rule (with-pypi responses body ...)
         (x
          (pk 'fail x #f))))))
 
-(test-assert "pypi->guix-package, no usable requirement file."
+(test-assert "pypi->guix-package, no usable requirement file, no wheel."
   (let ((tarball (pypi-tarball "foo-1.0.0"
                                '(("foo.egg-info/.empty" "")))))
     (with-pypi `(("/foo-1.0.0.tar.gz" 200 ,(file-dump tarball))

base-commit: cfd4f56f75a20b6732d463180d211f796c9032e5
-- 
2.45.2

>From 0abdb392bf10a99291114fc7e162a3845f25c696 Mon Sep 17 00:00:00 2001
Message-ID: <[email-masked]>
In-Reply-To: <[email-masked]>
References: <[email-masked]>
From: Lars-Dominik Braun <[email-masked]>
Date: Sun, 15 Dec 2024 13:30:59 +0100
Subject: [PATCH 2/4] import: pypi: Add python-wheel to native inputs if
 setuptools is used.

* guix/import/pypi.scm (compute-inputs): Add missing python-wheel if
necessary.

Change-Id: Iedad213a6684856e48349289c4d9beba953f396b
---
 guix/import/pypi.scm | 8 +++++++-
 1 file changed, 7 insertions(+), 1 deletion(-)

diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
index 7915d65d23..52ec6e4ee6 100644
--- a/guix/import/pypi.scm
+++ b/guix/import/pypi.scm
@@ -470,12 +470,18 @@ (define (compute-inputs source-url wheel-url archive)
                          (type type))))
                 (sort deps string-ci<?)))
 
+  (define (add-missing-native-inputs inputs)
+    ;; setuptools cannot build wheels without the python-wheel.
+    (if (member "setuptools" inputs)
+      (cons "wheel" inputs)
+      inputs))
+
   ;; TODO: Record version number ranges in <upstream-input>.
   (let ((dependencies (guess-requirements source-url wheel-url archive)))
     (match dependencies
       ((propagated native)
        (append (requirements->upstream-inputs propagated 'propagated)
-               (requirements->upstream-inputs native 'native))))))
+               (requirements->upstream-inputs (add-missing-native-inputs native) 'native))))))
 
 (define* (pypi-package-inputs pypi-package #:optional version)
   "Return the list of <upstream-input> for PYPI-PACKAGE.  This procedure
-- 
2.45.2

>From 0c9708bf7b387f2100cdf375353982fbca9b364e Mon Sep 17 00:00:00 2001
Message-ID: <[email-masked]>
In-Reply-To: <[email-masked]>
References: <[email-masked]>
From: Lars-Dominik Braun <[email-masked]>
Date: Sun, 15 Dec 2024 16:56:53 +0100
Subject: [PATCH 3/4] import: pypi: Default to setuptools as build system
 input.

* guix/import/pypi.scm (guess-requirements): Default to setuptools if
pyproject.toml does not exist.

Change-Id: I600bd0a44342847878e3a2a7041bd7e7c7d30769
---
 guix/import/pypi.scm | 10 ++++++++--
 1 file changed, 8 insertions(+), 2 deletions(-)

diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
index 52ec6e4ee6..bba7361307 100644
--- a/guix/import/pypi.scm
+++ b/guix/import/pypi.scm
@@ -448,15 +448,21 @@ (define (guess-requirements source-url wheel-url archive)
       (((first-propagated first-native) (second-propagated second-native))
        (list (append first-propagated second-propagated) (append first-native second-native)))))
 
+  (define default-pyproject.toml-dependencies
+    ;; If there is no pyproject.toml, we assume it’s an old-style setuptools-based project.
+    '(() ("setuptools")))
+
   ;; requires.txt and the metadata of a wheel contain redundant information,
   ;; so fetch only one of them, preferring requires.txt from the source
   ;; distribution, which we always fetch, since the source tarball also
   ;; contains pyproject.toml.
   (match (guess-requirements-from-source)
     ((from-pyproject.toml #f)
-      (merge (or from-pyproject.toml '(() ())) (or (guess-requirements-from-wheel) '(() ()))))
+      (merge (or from-pyproject.toml default-pyproject.toml-dependencies)
+             (or (guess-requirements-from-wheel) '(() ()))))
     ((from-pyproject.toml from-requires.txt)
-      (merge (or from-pyproject.toml '(() ())) from-requires.txt))))
+      (merge (or from-pyproject.toml default-pyproject.toml-dependencies)
+             from-requires.txt))))
 
 (define (compute-inputs source-url wheel-url archive)
   "Given the SOURCE-URL and WHEEL-URL of an already downloaded ARCHIVE, return
-- 
2.45.2

>From 8ab434690c870deb95bfbf61adc60a6a38d084bb Mon Sep 17 00:00:00 2001
Message-ID: <[email-masked]>
In-Reply-To: <[email-masked]>
References: <[email-masked]>
From: Lars-Dominik Braun <[email-masked]>
Date: Sun, 15 Dec 2024 17:02:44 +0100
Subject: [PATCH 4/4] import: pypi: Move deduplication to final processing
 step.

* guix/import/pypi.scm (parse-requires.txt): Remove deduplication.
(parse-wheel-metadata): Remove deduplication.
(compute-inputs): Instead do it here on all the collected inputs.

Change-Id: I2504cc693e9bf2e4cc44fd37b5823904dbaaa925
---
 guix/import/pypi.scm | 16 ++++++----------
 1 file changed, 6 insertions(+), 10 deletions(-)

diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
index bba7361307..530b7d6879 100644
--- a/guix/import/pypi.scm
+++ b/guix/import/pypi.scm
@@ -283,12 +283,7 @@ (define (parse-requires.txt requires.txt)
         (let ((line (read-line port)))
           (cond
            ((eof-object? line)
-            ;; Duplicates can occur, since the same requirement can be
-            ;; listed multiple times with different conditional markers, e.g.
-            ;; pytest >= 3 ; python_version >= "3.3"
-            ;; pytest < 3 ; python_version < "3.3"
-            (map (compose reverse delete-duplicates)
-                 (list required-deps test-deps)))
+            (list required-deps test-deps))
            ((or (string-null? line) (comment? line))
             (loop required-deps test-deps inside-test-section? optional?))
            ((section-header? line)
@@ -342,8 +337,7 @@ (define (parse-wheel-metadata metadata)
         (let ((line (read-line port)))
           (cond
            ((eof-object? line)
-            (map (compose reverse delete-duplicates)
-                 (list required-deps test-deps)))
+            (list required-deps test-deps))
            ((and (requires-dist-header? line) (not (extra? line)))
             (loop (cons (specification->requirement-name
                          (requires-dist-value line))
@@ -486,8 +480,10 @@ (define (compute-inputs source-url wheel-url archive)
   (let ((dependencies (guess-requirements source-url wheel-url archive)))
     (match dependencies
       ((propagated native)
-       (append (requirements->upstream-inputs propagated 'propagated)
-               (requirements->upstream-inputs (add-missing-native-inputs native) 'native))))))
+       (append (requirements->upstream-inputs (delete-duplicates propagated)
+                                              'propagated)
+               (requirements->upstream-inputs (delete-duplicates (add-missing-native-inputs native))
+                                              'native))))))
 
 (define* (pypi-package-inputs pypi-package #:optional version)
   "Return the list of <upstream-input> for PYPI-PACKAGE.  This procedure
-- 
2.45.2