Re: [PATCH] import: pypi: read requirements from wheels.

[ludo@gnu.org (Ludovic Courtès)]

Cyril Roelandt <tipecaml@gmail.com> skribis:

> * guix/import/pypi.scm (latest-wheel-release): New procedure.

Could you list the other changes (new procedures, new tests, changed
procedures, etc.) in the commit log?

Also, could you add a note in “Invoking guix import” to mention how
Wheels is used?  The ‘unzip’ requirement should also be mentioned in a
footnote or something.

> +(define (wheel-url->extracted-directory wheel-url)
> +  (string-append
> +   (string-join
> +    (list-head
> +     (string-split (last (string-split wheel-url #\/))  #\-) 2)
> +    "-")
> +   ".dist-info"))

I find it a bit hard to follow.  What about something along these lines
(untested):

     (match (string-split (basename wheel-url) #/-)
       ((name version _ ...)
        (string-append name "-" version ".dist-info")))

> +  (define (read-wheel-metadata wheel-archive)
> +    ;; Given WHEEL-ARCHIVE, a ZIP Python wheel archive, return the package's
> +    ;; requirements.
> +    (let* ((dirname (wheel-url->extracted-directory wheel-url))
> +           (json-file (string-append dirname "/metadata.json")))
> +      (and (system* "unzip" "-q" wheel-archive json-file)

Should be:

  (and (zero? (system* …)) …)

> +(test-assert "pypi->guix-package, wheels"
> +  ;; Replace network resources with sample data.
> +  (mock ((guix import utils) url-fetch
> +         (lambda (url file-name)
> +           (match url
> +             ("https://pypi.python.org/pypi/foo/json"
> +              (with-output-to-file file-name
> +                (lambda ()
> +                  (display test-json))))
> +             ("https://example.com/foo-1.0.0.tar.gz"
> +               (begin
> +                 (mkdir "foo-1.0.0")
> +                 (with-output-to-file "foo-1.0.0/requirements.txt"
> +                   (lambda ()
> +                     (display test-requirements)))
> +                 (system* "tar" "czvf" file-name "foo-1.0.0/")
> +                 (delete-file-recursively "foo-1.0.0")
> +                 (set! test-source-hash
> +                       (call-with-input-file file-name port-sha256))))
> +             ("https://example.com/foo-1.0.0-py2.py3-none-any.whl"
> +               (begin
> +                 (mkdir "foo-1.0.0.dist-info")
> +                 (with-output-to-file "foo-1.0.0.dist-info/metadata.json"
> +                   (lambda ()
> +                     (display test-metadata)))
> +                 (let ((zip-file (string-append file-name ".zip")))
> +		   ;; zip always adds a "zip" extension to the file it creates,
   ^
Please remove tabs.

> +                   ;; so we need to rename it.
> +                   (system* "zip" zip-file "foo-1.0.0.dist-info/metadata.json")
> +                   (rename-file zip-file file-name))

This will fail if ‘zip’ is unavailable.  If this command is really
needed, the test should be skipped when it’s missing, for instance by
adding something like this above the test

  (test-skip (if (which "zip") 0 1))

… using ‘which’ from (guix build utils).

It seems that all these files are left behind no?  Could you make sure
they are removed?  That probably means putting them in a separate
directory, and then having:

  (dynamic-wind
    (const #t)
    (lambda ()
      ;; the body
      )
    (lambda ()
      (delete-file-recursively temp-dir)))

Could you send an updated patch?

Thank you!  It’s good to see the importer getting smarter.  :-)

Ludo’.