bug#49168: ‘guix import pypi’ misses package dependencies

["Ludovic Courtès" <ludo@gnu.org>]

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

Hi,

Leo Prikler <leo.prikler@student.tugraz.at> skribis:

> Am Mittwoch, den 23.06.2021, 15:20 +0200 schrieb Ludovic Courtès:
>> Hi,
>> 
>> Leo Prikler <leo.prikler@student.tugraz.at> skribis:
>> 
>> > Am Dienstag, den 22.06.2021, 14:33 +0200 schrieb Ludovic Courtès:
>> 
>> [...]
>> 
>> > > Actually
>> > > <
>> > > https://files.pythonhosted.org/packages/fe/9d/4e15b2e74044ee051b6939c1b3ff716b0106e8f72d78eab8e08212eab44c/tablib-3.0.0.tar.gz
>> > > does not have a ‘requirements.txt’ file, and
>> > > <
>> > > https://files.pythonhosted.org/packages/16/85/078fc037b15aa1120d6a0287ec9d092d93d632ab01a0e7a3e69b4733da5e/tablib-3.0.0-py3-none-any.whl
>> > > doesn’t have much metadata, so I don’t even get where were get
>> > > that
>> > > info.
>> > It does, but it's well hidden in the src tree.  I peeked into the
>> > guix
>> > import code to find it.
>> 
>> Indeed.  The tarball above has ‘tests/requirements.txt’:
>> 
>> --8<---------------cut here---------------start------------->8---
>> pytest
>> pytest-cov
>> MarkupPy
>> odfpy
>> openpyxl>=2.6.0
>> pandas
>> pyyaml
>> tabulate
>> xlrd
>> xlwt
>> --8<---------------cut here---------------end--------------->8---
>> 
>> There are no optional dependencies in that file, though.  Or were you
>> looking at something else?
> The importer and I are looking at something else:
>
> $ tar xfv tablib-3.0.0.tar.gz tablib-3.0.0/src/tablib.egg-
> info/requires.txt | xargs cat
>
> [all]
> markuppy
> odfpy
> openpyxl>=2.6.0
> pandas
> pyyaml
> tabulate
> xlrd
> xlwt
>
> [cli]
> tabulate
>
> [html]
> markuppy
>
> [ods]
> odfpy
>
> [pandas]
> pandas
>
> [xls]
> xlrd
> xlwt
>
> [xlsx]
> openpyxl>=2.6.0
>
> [yaml]
> pyyaml

Oooh, I see.

So I came up with the following patch, which adds a flag for optional
dependencies, based on ‘requires.txt’.

Unfortunately, it has no effect for ‘guix import pypi tablib’ because we
only look at Wheel info in that case, as per:

  ;; 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))

AFAICS, wheels don’t provide that info, do they?

Why does the importer favor .whl in the first place?  Is it supposed to
be more accurate or more widespread or something?

Thoughts?

Thanks,
Ludo’.


[-- Attachment #2: Type: text/x-patch, Size: 10848 bytes --]

diff --git a/guix/import/pypi.scm b/guix/import/pypi.scm
index 6731d50891..be01cc99d2 100644
--- a/guix/import/pypi.scm
+++ b/guix/import/pypi.scm
@@ -1,7 +1,7 @@
 ;;; GNU Guix --- Functional package management for GNU
 ;;; Copyright © 2014 David Thompson <davet@gnu.org>
 ;;; Copyright © 2015 Cyril Roelandt <tipecaml@gmail.com>
-;;; Copyright © 2015, 2016, 2017, 2019, 2020 Ludovic Courtès <ludo@gnu.org>
+;;; Copyright © 2015, 2016, 2017, 2019, 2020, 2021 Ludovic Courtès <ludo@gnu.org>
 ;;; Copyright © 2017 Mathieu Othacehe <m.othacehe@gmail.com>
 ;;; Copyright © 2018 Ricardo Wurmus <rekado@elephly.net>
 ;;; Copyright © 2019 Maxim Cournoyer <maxim.cournoyer@gmail.com>
@@ -232,10 +232,9 @@ the input field."
   "Given REQUIRES.TXT, a path to a Setuptools requires.txt file, return a list
 of lists of requirements.
 
-The first list contains the required dependencies while the second the
-optional test dependencies.  Note that currently, optional, non-test
-dependencies are omitted since these can be difficult or expensive to
-satisfy."
+The first list contains the required dependencies, the second contains test
+dependencies, and the third one contains optional dependencies.  Note that
+optional, non-test dependencies can be difficult or expensive to satisfy."
 
   (define (comment? line)
     ;; Return #t if the given LINE is a comment, #f otherwise.
@@ -249,6 +248,7 @@ satisfy."
     (lambda (port)
       (let loop ((required-deps '())
                  (test-deps '())
+                 (optional-deps '())
                  (inside-test-section? #f)
                  (optional? #f))
         (let ((line (read-line port)))
@@ -259,27 +259,33 @@ satisfy."
             ;; 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 optional-deps)))
            ((or (string-null? line) (comment? line))
-            (loop required-deps test-deps inside-test-section? optional?))
+            (loop required-deps test-deps optional-deps
+                  inside-test-section? optional?))
            ((section-header? line)
             ;; Encountering a section means that all the requirements
             ;; listed below are optional. Since we want to pick only the
             ;; test dependencies from the optional dependencies, we must
             ;; track those separately.
-            (loop required-deps test-deps (test-section? line) #t))
+            (loop required-deps test-deps optional-deps
+                  (test-section? line) #t))
            (inside-test-section?
             (loop required-deps
                   (cons (specification->requirement-name line)
                         test-deps)
+                  optional-deps
                   inside-test-section? optional?))
            ((not optional?)
             (loop (cons (specification->requirement-name line)
                         required-deps)
-                  test-deps inside-test-section? optional?))
+                  test-deps optional-deps
+                  inside-test-section? optional?))
            (optional?
-            ;; Skip optional items.
-            (loop required-deps test-deps inside-test-section? optional?))
+            (loop required-deps test-deps
+                  (cons (specification->requirement-name line)
+                        optional-deps)
+                  inside-test-section? optional?))
            (else
             (warning (G_ "parse-requires.txt reached an unexpected \
 condition on line ~a~%") line))))))))
@@ -314,7 +320,8 @@ returned value."
           (cond
            ((eof-object? line)
             (map (compose reverse delete-duplicates)
-                 (list required-deps test-deps)))
+                 (list required-deps test-deps
+                       '())))             ;XXX: no known optional dependencies
            ((and (requires-dist-header? line) (not (extra? line)))
             (loop (cons (specification->requirement-name
                          (requires-dist-value line))
@@ -392,7 +399,7 @@ cannot determine package dependencies from source archive: ~a~%")
   (or (guess-requirements-from-wheel)
       (guess-requirements-from-source)))
 
-(define (compute-inputs source-url wheel-url archive)
+(define* (compute-inputs source-url wheel-url archive)
   "Given the SOURCE-URL and WHEEL-URL of an already downloaded ARCHIVE, return
 a pair of lists, each consisting of a list of name/variable pairs, for the
 propagated inputs and the native inputs, respectively.  Also
@@ -419,17 +426,19 @@ return the unaltered list of upstream dependency names."
     (values (map process-requirements dependencies)
             (concatenate dependencies))))
 
-(define (make-pypi-sexp name version source-url wheel-url home-page synopsis
-                        description license)
+(define* (make-pypi-sexp name version source-url wheel-url home-page synopsis
+                         description license
+                         #:key optional-dependencies?)
   "Return the `package' s-expression for a python package with the given NAME,
-VERSION, SOURCE-URL, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
+VERSION, SOURCE-URL, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE.  When
+OPTIONAL-DEPENDENCIES? is true, include optional dependencies."
   (call-with-temporary-output-file
    (lambda (temp port)
      (and (url-fetch source-url temp)
           (receive (guix-dependencies upstream-dependencies)
               (compute-inputs source-url wheel-url temp)
             (match guix-dependencies
-              ((required-inputs native-inputs)
+              ((required-inputs native-inputs optional-inputs)
                (when (string-suffix? ".zip" source-url)
                  (set! native-inputs (cons
                                      '("unzip" ,unzip)
@@ -462,7 +471,12 @@ VERSION, SOURCE-URL, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
                        (base32
                         ,(guix-hash-url temp)))))
                    (build-system python-build-system)
-                   ,@(maybe-inputs required-inputs 'propagated-inputs)
+                   ,@(maybe-inputs
+                      (append required-inputs
+                              (if optional-dependencies?
+                                  optional-inputs ;TODO: emit a comment
+                                  '()))
+                      'propagated-inputs)
                    ,@(maybe-inputs native-inputs 'native-inputs)
                    (home-page ,home-page)
                    (synopsis ,synopsis)
@@ -472,9 +486,10 @@ VERSION, SOURCE-URL, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
 
 (define pypi->guix-package
   (memoize
-   (lambda* (package-name #:key repo version)
+   (lambda* (package-name #:key repo version optional-dependencies?)
      "Fetch the metadata for PACKAGE-NAME from pypi.org, and return the
-`package' s-expression corresponding to that package, or #f on failure."
+`package' s-expression corresponding to that package, or #f on failure.  When
+OPTIONAL-DEPENDENCIES? is true, include optional dependencies."
      (let* ((project (pypi-fetch package-name))
             (info    (and project (pypi-project-info project))))
        (and project
@@ -493,11 +508,17 @@ VERSION, SOURCE-URL, HOME-PAGE, SYNOPSIS, DESCRIPTION, and LICENSE."
                               (project-info-summary info)
                               (project-info-summary info)
                               (string->license
-                               (project-info-license info)))))))))
+                               (project-info-license info))
+                              #:optional-dependencies?
+                              optional-dependencies?)))))))
 
-(define (pypi-recursive-import package-name)
+(define* (pypi-recursive-import package-name #:key optional-dependencies?)
   (recursive-import package-name
-                    #:repo->guix-package pypi->guix-package
+                    #:repo->guix-package
+                    (lambda args
+                      (apply pypi->guix-package
+                             #:optional-dependencies? optional-dependencies?
+                             args))
                     #:guix-name python->package-name))
 
 (define (string->license str)
diff --git a/guix/scripts/import/pypi.scm b/guix/scripts/import/pypi.scm
index 33167174e2..58ae07f802 100644
--- a/guix/scripts/import/pypi.scm
+++ b/guix/scripts/import/pypi.scm
@@ -46,6 +46,8 @@ Import and convert the PyPI package for PACKAGE-NAME.\n"))
   (display (G_ "
   -r, --recursive        import packages recursively"))
   (display (G_ "
+  -O, --optional         include optional dependencies"))
+  (display (G_ "
   -V, --version          display version information and exit"))
   (newline)
   (show-bug-report-information))
@@ -62,6 +64,9 @@ Import and convert the PyPI package for PACKAGE-NAME.\n"))
          (option '(#\r "recursive") #f #f
                  (lambda (opt name arg result)
                    (alist-cons 'recursive #t result)))
+         (option '(#\O "optional") #f #f
+                 (lambda (opt name arg result)
+                   (alist-cons 'optional-dependencies? #t result)))
          %standard-import-options))
 
 \f
@@ -85,6 +90,9 @@ Import and convert the PyPI package for PACKAGE-NAME.\n"))
                              value)
                             (_ #f))
                            (reverse opts))))
+    (define optional-dependencies?
+      (assoc-ref opts 'optional-dependencies?))
+
     (match args
       ((package-name)
        (if (assoc-ref opts 'recursive)
@@ -94,9 +102,13 @@ Import and convert the PyPI package for PACKAGE-NAME.\n"))
                    `(define-public ,(string->symbol name)
                       ,pkg))
                   (_ #f))
-                (pypi-recursive-import package-name))
+                (pypi-recursive-import package-name
+                                       #:optional-dependencies?
+                                       optional-dependencies?))
            ;; Single import
-           (let ((sexp (pypi->guix-package package-name)))
+           (let ((sexp (pypi->guix-package package-name
+                                           #:optional-dependencies?
+                                           optional-dependencies?)))
              (unless sexp
                (leave (G_ "failed to download meta-data for package '~a'~%")
                       package-name))